Schema agreement error handling #1479
Draft
Conversation
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
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves schema agreement error handling by introducing logic to distinguish between transient and non-transient errors, allowing the schema agreement loop to terminate early when encountering serious issues rather than waiting for the full timeout.
Key Changes:
- Added
classify_schema_check_errorfunction to categorize errors as transient (continue retrying) or non-transient (return immediately) - Refactored
await_schema_agreement_with_required_nodeto useControlFlowfor early termination on non-transient errors - Added documentation noting the misnamed
TracesEventsIntoRowsResultErrorvariant (to be fixed in v2.0)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scylla/src/errors.rs | Added documentation explaining that TracesEventsIntoRowsResultError is misnamed and will be renamed in v2.0 |
| scylla/src/client/session.rs | Introduced error classification logic to enable early termination for non-transient schema agreement errors, restructured timeout handling accordingly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SyntaxError will cause schema agreement process to fail immediately after next commits (that will add error classification there).
Lorak-mmk
force-pushed
the
schema-agreement-error-handling
branch
from
0c8ed93 to
31bafbb
Compare
This changes the logic of schema agreement to allow some error to end the process immediately, without waiting until the timeout. For now the error clasification is not done with a lot of effort, just to have something that doesn't treat transient errors as non-transient, but also classify some errors as non-transient.
Lorak-mmk
force-pushed
the
schema-agreement-error-handling
branch
3 times, most recently
from
ad492eb to
b101c79
Compare
8 tasks
After changing schema agreement logic, those tests became slow, as they had to wait until schema agreement timeout in each case, which is by default 60s.
Lorak-mmk
force-pushed
the
schema-agreement-error-handling
branch
from
b101c79 to
8bb3718
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
In #1451 we are going to more carefully handle errors.
In order to make this easier by only having to work on deciding how to handle each error, this PR introduces changes to schema agreement logic that
allow it to end early for certain errors.
Pre-review checklist
I added relevant tests for new features and bug fixes.I have provided docstrings for the public items that I want to introduce.I have adjusted the documentation in./docs/source/.I added appropriateFixes:annotations to PR description.