Tron api returns 500 errors on jobruncollection mutations during reads - #1114
Open
CharlesPeacock wants to merge 5 commits into
Open
Conversation
…making them run bounded, not exclusive. Added additional locks and changed a test to reflect behavior
…read_only_endpoints_from_blocking_reactor merge with master branch.
… view_all_metrics
…ng generator iterations
This file contains hidden or 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
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.
This PR is a follow-up to PR-1113 and changes should be reviewed together (Or 1113 can be reviewed first, the branch for this PR is based off of 1113).
This PR adds fixes to a recently discovered race condition inside of JobRunCollection methods. When handling concurrent API requests, /api/jobs reads from a shared deque inside JobRunCollection, this deque can be mutated during reads by the reactor, resulting in replicable 500 errors with
RuntimeError: deque mutated during iteration.I fixed the issue by adding a snapshot of the deque to every method inside of JobRunCollection that reads from it. Since this operation is atomic, it ensures reads stay consistent and we don't read from a mutating deque.
Aside from passing tests, I made a small script that makes concurrent API calls to the Tron api, which I initially used to discover the issue.
After performing the change, there were no errors during API testing (for this issue specifically, errors would come out in phase 3 of this script).
Keep in mind that this script was made to test the previous changes, related to api GET endpoints moving from @exclusive to @bounded.
Sample output of script: