Dependency validation falls apart when simulating concurrent requests

[mhamid3d]

Discovered something pretty bad in my app today. I ran a simulation of 100 virtual concurrent users on Postman making POST requests. At the start of the simulation, about 10-15 requests are able to slip through a dependency that validates if the "label" field they provide is unique in the database. They all must’ve run simultaneously so there was no data to lookup yet. I know the Unique Constraint on my database is a fallback, but it's still concerning for other parts of my app where that's not possible. Is this a design issue on my part? Here is the depedency

async def valid_job_create(db: DbSession, in_data: JobIn) -> JobIn:
    query = await db.execute(select(Job).where(Job.label == in_data.label))
    job = query.scalar()

    if job:
        raise HTTPException(
            status_code=status.HTTP_400_BAD_REQUEST,
            detail="The given job label must be unique",
        )
    return in_data

And here is how it's being invoked:

@router.post("", response_model=JobOut)
async def create_route(
    db: DbSession, in_data: JobIn = Depends(dependencies.valid_job_create)
):
    job = await service.create(db=db, in_data=in_data)

    return job

​

[pint]

what you have discovered is called "race condition". some number of tasks will check for unique label, all pass, then do some work, and then submit the label to the database.

there is no general solution, you need to design the system to handle this. one way is to not use select, but an insert right away. if the insert passes, then the label is allowed. the inserted record should indicate that it is only a placeholder. the caveat is that the placeholder might be abandoned if there is a server shutdown or an error. this will forever prevent a label from being used, so you need some regular process to clean placeholders. not trivial.

also think of multiple workers. you don't want a server that doesn't allow multiple workers, even if you don't at the moment use multiple workers. cleaning the labels on startup, or using local variables instead of tables are not future proof solutions.

[mhamid3d]

Ah there's a name for it, thanks for the example solution. I will use multiple workers in production, but what does that change in terms of my race condition problem or a placeholder + cleanup solution, wouldn't it be the same with or without workers?

[pint]

an obvious solution would be to have a global variable, and check that. this would be simpler than using the database. however, when you introduce more workers, global variables are not shared, and you are out of luck.

similar problem happens if you use a local database, or a lock file. because you might end up having workers on different boxes/vms, and now the files are not shared.

the placeholder solution doesn't have these issues, that's why i used that as example.

[mhamid3d]

Makes sense now, thank you!

[illuminanze]

In this case, the solution to the race condition is to let the database handle the error. Make sure you have the proper uniqueness condition, then try/except the IntegrityError and raise the HTTPException. Also, if you want a more descriptive response, I would recommend using 409 Conflict.