-
Notifications
You must be signed in to change notification settings - Fork 112
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
topology: skip invalid peers instead of failing #1045
Conversation
Err(err) => { | ||
warn!( | ||
"system.peers or system.local has an invalid row, skipping it: {}", | ||
err |
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 really would like to see #1021 here. IIUC this would most always return system.peers or system.local has invalid column type
, no?
Either way, this LGTM as it addresses the skip part.
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.
The warn!
prints a pretty message contained by err
, so we should get the deepest possible explanation of what went wrong.
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.
LGTM
Let's wait with merging until CI issues are fixed (and then rebase) |
You can mark #235 as fixed by this. |
fa2c2ed
to
915feba
Compare
Rebased on main. |
It appears that in some clusters there sometimes emerges a transient state that there are some incomplete/invalid peer entries in system tables. Under such circumstances, the driver would throw an error and fail the metadata refresh altogether, eventually leading to critical conditions such as no hosts to query. To fix this in line with how other drivers approach this, a warning is emitted upon invalid peer entry read, and the entry is skipped with no error returned.
915feba
to
a282396
Compare
|
Do you plan to make a new tag any time soon after merging this bugfix? |
We are thinking of introducing some paging API changes soon and then releasing. |
…peer topology: skip invalid peers instead of failing (cherry picked from commit a0e7856)
Motivation
It appears that in some clusters there sometimes emerges a transient state that there are some incomplete/invalid peer entries in system tables. Under such circumstances, the driver would throw an error and fail the metadata refresh altogether, eventually leading to critical conditions such as no hosts to query.
Solution
To fix this in line with how other drivers approach this, a warning is emitted upon invalid peer entry read, and the entry is skipped with no error returned.
Fixes: #1023
Fixes: #235
Pre-review checklist
[ ] I have provided docstrings for the public items that I want to introduce.[ ] I have adjusted the documentation in./docs/source/
.Fixes:
annotations to PR description.