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 concurrency in imagery store #59

Merged
merged 1 commit into from
Jun 4, 2019
Merged

Fix concurrency in imagery store #59

merged 1 commit into from
Jun 4, 2019

Conversation

oldmud0
Copy link
Member

@oldmud0 oldmud0 commented Jun 4, 2019

Apparently, connection pools in SQLite are still too dumb to synchronize properly without triggering SQLITE_TIMEOUT, so I just decided to limit the connection pool to 1 to force mutual exclusion. However, I may have accidentally discovered a deadlock condition, given that the connection pool caused a timeout. I hope this is not the case.

Even with and without this fix, the tests for everything - not just the store - are failing once in a blue moon, which doesn't seem to be a good sign. What's worse is that the errors do not point to anything in particular - for instance, one error was about rolling back a transaction while a transaction was not underway. I checked the code path and there isn't a path where this could ever happen. Another error was a Z CAM E1 backend test saying that a dummy function was called twice when it should have been called three times. These are some very peculiar synchronization problems.

Apparently, connection pools in SQLite are still too dumb to synchronize
properly without triggering SQLITE_TIMEOUT, so I just decided to limit
the connection pool to 1 to force mutual exclusion. However, I may have
accidentally discovered a deadlock condition, given that the connection
pool caused a timeout. I hope this is not the case.
Copy link
Member

@bbridges bbridges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limiting the maximum write to just one works fine. Test flakiness should probably be looked into at a later date.

@bbridges bbridges merged commit c86a9b5 into master Jun 4, 2019
@bbridges bbridges deleted the imagery-concurrent branch June 4, 2019 03:23
@senkevinli senkevinli mentioned this pull request Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants