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

ensure_for_user create IntegrityError race condition #6

Closed
awseeley opened this issue Feb 10, 2021 · 4 comments
Closed

ensure_for_user create IntegrityError race condition #6

awseeley opened this issue Feb 10, 2021 · 4 comments

Comments

@awseeley
Copy link

Hi - we are experiencing a race condition where concurrent requests can cause user.agentsettings = self.create(user=user) to throw an integrity error due to the user ID already existing in the database.

This shows up in our sentry error logs as follows:
duplicate key value violates unique constraint "django_agent_trust_agentsettings_user_id_key"
DETAIL: Key (user_id)=(1) already exists.

This was caused by the following commit which removed the get_or_create
71de1ac#diff-36c097cacb670d42993df6c72f8f2aae021eb52fb353e9cab14ca71f2afb54ac
#2

Timeline of this exception occurring as follows:

  1. Request A sent in no User<->Django Agent Trust relationship found exists on the request.user object so it attempts to create this relationship
  2. Request B sent in no User<->Django Agent Trust relationship found exists on the request.user object so it attempts to create this relationship
  3. Request A successfully creates this
  4. Request B fails to create as already done in step Bump django from 2.2.4 to 2.2.8 #3, throws an integrity error on user_id already existing.

I see two options to address this.

  1. Revert the problem commit, although it does have to attempt a retrive from the database first the performance overheads are very minimal in my view.
  2. Add a try catch on the create. If there is an IntegrityError exception then pass. As follows:

def ensure_for_user(self, user): """ Loads a user's AgentSettings instance, creating a default if necessary. """ try: user.agentsettings except self.model.DoesNotExist: try: user.agentsettings = self.create(user=user) except IntegrityError: pass

@psagers
Copy link
Member

psagers commented Feb 10, 2021

get_or_create actually has the same race condition; I think catching the IntegrityError is really the only thing one can do. As far as I know, the ORM doesn't provide the kind of clean ON CONFLICT semantics that some of the underlying SQL implementations provide.

@psagers
Copy link
Member

psagers commented Feb 10, 2021

What do you think about #7?

@awseeley
Copy link
Author

Agreed - yep #7 looks good to me!

@psagers
Copy link
Member

psagers commented Feb 12, 2021

Version 1.0.2

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

No branches or pull requests

2 participants