-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5505 Prototype system overload retry loop for all operations #2497
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
4728868
PYTHON-5505 Prototype system overload retry loop for all operations
ShaneHarvey a83dea8
PYTHON-5505 Make cursor getMore retryable
ShaneHarvey c2ff971
PYTHON-5505 Use functools.wraps to fix test_comment
ShaneHarvey b6dd34d
PYTHON-5505 Add test that even non-retryable write ops will be retrie…
ShaneHarvey eb113f0
PYTHON-5505 Rename overloaded flag
ShaneHarvey b436647
PYTHON-5505 cleanup tests
ShaneHarvey f3be7f7
PYTHON-5505 Only apply backoff when SystemOverloaded label is present
ShaneHarvey 65fbd7e
PYTHON-5505 Fix _retry_overload handling of backoff too
ShaneHarvey 194863f
PYTHON-5505 Remove unused "no_retry" flag
ShaneHarvey 989fdfe
PYTHON-5505 Fix RetryableWriteError label check
ShaneHarvey 1f40e11
PYTHON-5505 Fix retry eligibility bug
ShaneHarvey 6114788
PYTHON-5505 Fix handling of ClientBulkWriteException
ShaneHarvey d2e3354
PYTHON-5505 Allow retry write after overload error even if sessions a…
ShaneHarvey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my own understanding: what's the purpose of this
overload
value? It'sTrue
when the error is an overload error?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's my understanding of it -- and then when
overload
is true and we've reachedmax_retries
, we raise the error, otherwise webackoff
and try again. (I think)And after this initial attempt, we only ever set
overload
toexc.error.has_error_label("Retryable")
, is there a reason why we cant just set it to that here?(side note: I like how
_overload
and_retrying
are the same length, but I think_overloaded
is a slightly more accurate var? i don't think this matters that much though since I feel like_overload
does get the point across for me)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
_overload
is only set when the error is a result of the server being overloaded. If so,_overloaded
is a clearer variable name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's all correct. I've renamed it
overloaded
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: this was renamed "always_retryable". "overloaded" is now based on the "SystemOverloaded" error label.