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

Fix IntegrityError with unique field and get_or_create #302

Merged
merged 3 commits into from
Feb 29, 2020

Conversation

grigi
Copy link
Member

@grigi grigi commented Feb 28, 2020

Fixes #140

Had to introduce an extra lock for nested transactions for DB's that support it to prevent network errors with both PostgreSQL & MySQL.

@grigi
Copy link
Member Author

grigi commented Feb 28, 2020

@abondar based this on your original attempt to fix this. Please review.

@grigi grigi requested a review from abondar February 28, 2020 12:59
@coveralls
Copy link

coveralls commented Feb 28, 2020

Pull Request Test Coverage Report for Build 1338

  • 26 of 26 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 98.062%

Totals Coverage Status
Change from base Build 1324: -0.01%
Covered Lines: 3516
Relevant Lines: 3550

💛 - Coveralls

@grigi
Copy link
Member Author

grigi commented Feb 28, 2020

Wow, Py3.6 gets so confused with contexts 😭
I feel we may just have to remove it from CI altogether...
It has been deprecated for a few months now...

Unfortunately it would mean dropping pypy too.

I'll just conditionally skip the test for now.

@grigi
Copy link
Member Author

grigi commented Feb 28, 2020

Another option is to disable all connection pooling on py3.6?

@AEnterprise
Copy link
Collaborator

AEnterprise commented Feb 28, 2020

wow didn't expect pooling to be such a major pain on 3.6

but personally i'm more in favor for dropping only specific features on a version like pooling then dropping an entire version (prob best to log a warning on startup though so people are aware)

@grigi
Copy link
Member Author

grigi commented Feb 28, 2020

Py3.6 uses a contextvar backport, and unfortunately it has shortcomings. py3.7+ has extra state in their asyncio C-structs to keep track better.
I'm busy seeing what would be the lowest-impact change for disabling pooling for py3.6. It seems forcing a pool of 1 doesn't do it.
I suspect I would have to use explicit locking like I do for SQLite to get it working right.

@AEnterprise
Copy link
Collaborator

i think forcing a shared lock instead of making new locks for concurrent operations should do the trick

that and forcing a poolsize of 1 regardless since it can't use multiple so there is no reason to have any extras open

@grigi
Copy link
Member Author

grigi commented Feb 28, 2020

Well, in the meantime I'm getting hillarious errors, like "cannot rollback; the transaction is not yet started" but investigating further the cause is the dreaded "cannot perform operation: another operation is in progress".

And fixing race conditions fixes that.

@grigi
Copy link
Member Author

grigi commented Feb 29, 2020

@AEnterprise We are discussing the potential deprecation for py3.6 in #303
This PR fixes a bug, and should go in soon.
I'll merge this to master instead of develop, and cherry-pick the windows fix as well and release that. All the new feature work is going in as v0.16

@grigi grigi changed the base branch from develop to master February 29, 2020 17:39
@grigi grigi changed the base branch from master to develop February 29, 2020 17:40
@grigi
Copy link
Member Author

grigi commented Feb 29, 2020

I'll cherry pick the commits instead.

@grigi grigi merged commit a33d90d into develop Feb 29, 2020
@grigi grigi deleted the fix_get_or_create branch February 29, 2020 18:00
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.

IntegrityError with unique field and get_or_create
3 participants