-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fixes for when getMappedRange cannot parse as tuple #6665
Conversation
AWS CodeBuild CI Report for macOS BigSur 11.5.2
|
Doxense CI Report for Windows 10
|
AWS CodeBuild CI Report for Linux CentOS 7
|
flow/error_definitions.h
Outdated
@@ -178,6 +178,10 @@ ERROR( blob_granule_not_materialized, 2037, "Blob Granule Read was not materiali | |||
ERROR( get_mapped_key_values_has_more, 2038, "getMappedRange does not support continuation for now" ) | |||
ERROR( get_mapped_range_reads_your_writes, 2039, "getMappedRange tries to read data that were previously written in the transaction" ) | |||
ERROR( checkpoint_not_found, 2040, "Checkpoint not found" ) | |||
ERROR( key_not_tuple, 2041, "The key cannot be parsed as a tuple" ); | |||
ERROR( value_not_tuple, 2042, "The key cannot be parsed as a tuple" ); |
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 message needs update - it says 'key'
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.
Good catch! Thanks!
fdbserver/storageserver.actor.cpp
Outdated
try { | ||
keyTuple = Tuple::unpack(keyValue->key); | ||
} catch (Error& e) { | ||
TraceEvent(SevError, "KeyNotTuple").detail("Key", keyValue->key.printable()); |
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.
Is SevError
the right severity? This basically means we are in a very wrong state.
Also, do we want to log the error e
here as well?
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.
Fixed.
fe0a06e
to
7f913f6
Compare
AWS CodeBuild CI Report for Linux CentOS 7
|
Doxense CI Report for Windows 10
|
Throws specific exception instead of a more general error type.
Log the violating bytes
Make SS not retry/hang on this kind of errors
Add and fix tests
Code-Reviewer Section
The general guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branch
ormaster
if this is the youngest branch)