Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass upsert=True to find_one_and_replace() on Document.save() method. #2581

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

puentesarrin
Copy link

@puentesarrin puentesarrin commented Oct 28, 2021

Following the discussion found in #1944 (comment).

In my company, we have faced an edge case where we might have concurrent "save()" calls failing, please imagine the following situation:

If we have at least two processes running in parallel that tries to save a document with the same new id in the same collection, both processes will get None when invoking PyMongo's find_one_and_replace in this line because the document with such _id doesn't exist in the collection, and then MongoEngine will invoke to PyMongo's insert_one here, so one of these two processes will get the document inserted and the other will obtain a NotUniqueError exception raised.

Instead, if we pass upsert=True to find_one_and_replace method, we will make sure that the document is saved and won't fail, even more because conceptually save() should never fail with such exception, at least not for the unique index on _id field. Also, note that we can ignore the returning value from find_one_and_replace since all what we need to return is the doc["_id"] that we already have in that trace.

Note: I've tried to reproduce it in unit tests using a thread pool but it's quite hard :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant