Say I have this endpoint on an express server:

app.get('/', async (req, res) => {
    var foo = await databaseGetFoo();
    if (foo == true) {
        foo = false;
        somethingThatShouldOnlyBeDoneOnce();
        await databaseSetFoo(foo);
    }

})

I think this creates a race condition if the endpoint is called twice simultaneously? If so how can I prevent this race condition from happening?

CodePudding user response：

OK, so based on the comments, I've got a little better understanding of what you want here.

Assuming that somethingThatShouldOnlyBeDoneOnce is doing something asynchronous (like writing to a database), you are correct that a user (or users) making multiple calls to that endpoint will potentially cause that operation to happen repeatedly.

Using your comment about allowing a single comment per user, and assuming you've got middleware earlier in the middleware stack that can uniquely identify a user by session or something, you could naively implement something like this that should keep you out of trouble (usual disclosures that this is untested, etc.):

let processingMap = {};

app.get('/', async (req, res, next) => {
    if (!processingMap[req.user.userId]) {
        // add the user to the processing map
        processingMap = {
            ...processingMap,
            [req.user.userId]: true
        };

        const hasUserAlreadySubmittedComment = await queryDBForCommentByUser(req.user.userId);

        if (!hasUserAlreadySubmittedComment) {
            // we now know we're the only comment in process
            // and the user hasn't previously submitted a comment,
            // so submit it now:

            await writeCommentToDB();

            delete processingMap[req.user.userId];

            res.send('Nice, comment submitted');
        } else {
            delete processingMap[req.user.userId];

            const err = new Error('Sorry, only one comment per user');
            err.statusCode = 400;

            next(err)
        }
    } else {
        delete processingMap[req.user.userId];

        const err = new Error('Request already in process for this user');
        err.statusCode = 400;

        next(err);
    }

})

Since insertion into the processingMap is all synchronous, and Node can only be doing one thing at a time, the first request for a user to hit this route handler will essentially lock for that user until the lock is removed when we're finished handling the request.

BUT... this is a naive solution and it breaks the rules for a 12 factor app. Specifically, rule 6, which is that your applications should be stateless processes. We've now introduced state into your application.

If you're sure you'll only ever run this as a single process, you're fine. However, the second you go to scale horizontally by deploying multiple nodes (via whatever method--PM2, Node's process.cluster, Docker, K8s, etc.), you're hosed with the above solution. Node Server 1 has no idea about the local state of Node Server 2 and so multiple requests hitting different instances of your multi-node application can't co-manage the state of the processing map.

The more robust solution would be to implement some kind of queue system, likely leveraging a separate piece of infrastructure like Redis. That way all of your nodes could use the same Redis instance to share state and now you can scale up to many, many instances of your application and all of them can share info.

I don't really have all the details on exactly how to go about building that out and it seems out of scope for this question anyway, but hopefully I've given you at least one solution and some idea of what to think about at a broader level.