DM-17608: use transactions in gen2convert to avoid slow locking #125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is necessary to avoid poor performance on shared filesystems with SQLite's now-aggressive file locking (by making us lock the DB once instead of repeatedly locking and unlocking). It also seems desirable, in the absence of self-consistent finer-grained transactions on what is inserted into the converted repo (which we certainly did not have before).
We saw some warnings about accessing SQLite connections from different threads when testing PipelineTask execution with multiprocessing. Those seem to have come when the Python multiprocessing module ends a process and hence closes a connection, and didn't actually cause any problems that we could see. This change just disables that warning.
natelust
approved these changes
Feb 1, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes were originally committed to DM-17496, but are not really part of its primary goal, and are actually more important to get merged, as they fix a slowdown in ci_hsc on master. They have been tested with the stack being used for the demo later today (via the DM-17496 branch), and while we do not plan to use this branch for the demo, it would be desirable to get this into the weekly.
See commit messages for details.