1

I'm using MongoDB with NodeJS. Therefore I use mongoose.

I'm developing a multi player real time game. So I receive many requests from many players sometimes at the very same time.

I can simplify it by saying that I have a house collection, that looks like this:

{
    "_id" : 1,
    "items": [item1, item2, item3]
}

I have a static function, called after each request is received:

house.statics.addItem = function(id, item, callback){
    var HouseModel = this;
    HouseModel.findById(id, function(err, house){
        if (err) throw err;
        //make some calculations such as:
        if (house.items.length < 4){
            HouseModel.findByIdAndUpdate(id, {$push: {items: item}}, cb);
        }
    });
}

In this example, I coded so that the house document can never have more than 4 items. But what happens is that when I receive several request at the very same time, this function is executed twice by both requests and since it is asynchronous, they both push a new item to the items field and then my house has 5 items.

I am doing something wrong? How can I avoid that behavior in the future?

alexislg
  • 1,018
  • 13
  • 20

4 Answers4

2

yes, you need better locking on the houseModel, to indicate that an addItem is in progress.

The problem is that multiple requests can call findById and see the same house.items.length, then each determine based on that (outdated) snapshot that it is ok to add one more item. The nodejs boundary of atomicity is the callback; between an async call and its callback, other requests can run.

One easy fix is to track not just the number of items in the house but the number of intended addItems as well. On entry into addItem, bump the "want to add more" count, and test that.

Andras
  • 2,995
  • 11
  • 17
  • Thanks for your answer. If I get it you mean that the variable "intended add items" would be stored in RAM? – alexislg Feb 07 '15 at 21:46
  • yes, indexed by the house _id, maybe as a property on house.statistics. That should work, unless you're running multiple servers and/or a node cluster. For multiple processes you'd have to implement a mutexed counter in some external shared store like memcache or mongodb itself. – Andras Feb 07 '15 at 23:04
  • Thanks! this will definitely work. Do you know if this is a common way to handle this kind of situations? My app will run in multi processes so thank you for your extra explanation – alexislg Feb 08 '15 at 21:08
  • I don't really know, I'm coming from a mysql background and am kind of new to mongodb. I'm now using nodejs with mongodb, and the asych calls and limited atomicity make for... interesting locking. So far I've been able to improvise. – Andras Feb 08 '15 at 23:13
  • Even if the “nodejs boundary of atomicity is the callback”, who says you mayn’t have multiple nodejs processes accessing the same MongoDB instance? – binki Jun 23 '17 at 01:07
1

One possible approach since the release of Mongoose 4.10.8 is writing a plugin which makes save() fail if the document has been modified since you loaded it. A partial example is referenced in #4004:

@vkarpov15 said:

8b4870c should give you the general direction of how one would write a plugin for this

Since Mongoose 4.10.8, plugins now have access to this.$where. For documents which have been loaded from the database (i.e., are not this.isNew), the plugin can add conditions which will be evaluated by MongoDB during the update which can prevent the update from actually happening. Also, if a schema’s saveErrorIfNotFound option is enabled, the save() will return an error instead of succeeding if the document failed to save.

By writing such a plugin and changing some property (such as a version number) on every update to the document, you can implement “optimistic concurrency” (as #4004 is titled). I.e., you can write code that roughly does findOne(), do some modification logic, save(), if (ex) retry(). If all you care about is a document remaining self-consistent and ensuring that Mongoose’s validators run and your document is not highly contentious, this lets you write code that is simple (no need to use something which bypasses Mongoose’s validators like .update()) without sacrificing safety (i.e., you can reject save()s if the document was modified in the meantime and avoid overwriting committed changes).

Sorry, I do not have a code example yet nor do I know if there is a package on npm which implements this pattern as a plugin yet.

binki
  • 7,754
  • 5
  • 64
  • 110
1

I am also building a multiplayer game and ran into the same issue. I believe I have solved it my implementing a queue-like structure:

class NpcSaveQueue {
    constructor() {
        this.queue = new Map();
        this.runQueue();
    }

    addToQueue(unitId, obj) {
        if (!this.queue.has(unitId)) {
            this.queue.set(String(unitId), obj);
        } else {
            this.queue.set(String(unitId), {
                ...this.queue.get(unitId),
                ...obj,
            })
        }
    }

    emptyUnitQueue(unitId) {
        this.queue.delete(unitId);
    }

    async executeUnitQueue(unitId) {
        await NPC.findByIdAndUpdate(unitId, this.queue.get(unitId));
        this.emptyUnitQueue(unitId);
    }

    runQueue() {
        setInterval(() => {
            this.queue.forEach((value, key) => {
                this.executeUnitQueue(key);
            })
        }, 1000)
    }
}

Then when I want to update an NPC, instead of interacting with Mongoose directly, I run:

npcSaveQueue.addToQueue(unit._id, {
                "location.x": newLocation.x,
                "location.y": newLocation.y,
            });

That way, every second, the SaveQueue just executes all code for every NPC that requires updating.

David
  • 944
  • 1
  • 13
  • 20
-2

This function never executes twice, because update operation is atomic on a level of single document. More info in official manual: http://docs.mongodb.org/manual/core/write-operations-atomicity/#atomicity-and-transactions

cehmja
  • 130
  • 6
  • 1
    the update itself may be atomic, but the call to it and the test before it are not. Between the find and its callback, and between the find-and-update and its callback, any number of other calls can run and see the stale, un-updated data. See my answer below. – Andras Feb 07 '15 at 20:57