Skip to content
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

Recover from last log segment with invalid messages #3018

Merged
merged 3 commits into from
Mar 8, 2025

Conversation

justinlin-linkedin
Copy link
Collaborator

Summary

Since we don't ignore invalid log segment from this PR #2744, we run into lots of error replicas due to invalid messages in last log segment when power outage happens. This PR adds configuration and logic to deal with those errors.

When there are invalid messages in the log segment when persistent index is trying to recover index segment files, we check if the log segment is the last log segment and if the amount of remaining bytes are small, if so, we just ignore the invalid message and move on with the valid message.

Testing Done

Added unit tests.

@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 69.95%. Comparing base (52ba813) to head (743d07d).
Report is 187 commits behind head on master.

Files with missing lines Patch % Lines
.../github/ambry/messageformat/BlobStoreRecovery.java 66.66% 5 Missing and 2 partials ⚠️
...c/main/java/com/github/ambry/store/LogSegment.java 45.45% 3 Missing and 3 partials ⚠️
...main/java/com/github/ambry/store/StoreMetrics.java 0.00% 4 Missing ⚠️
...n/java/com/github/ambry/store/PersistentIndex.java 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3018      +/-   ##
============================================
+ Coverage     64.24%   69.95%   +5.70%     
- Complexity    10398    12205    +1807     
============================================
  Files           840      894      +54     
  Lines         71755    75615    +3860     
  Branches       8611     9035     +424     
============================================
+ Hits          46099    52893    +6794     
+ Misses        23004    19961    -3043     
- Partials       2652     2761     +109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@SophieGuo410 SophieGuo410 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just left one question.

*/
void truncateTo(long offset) throws IOException {
long currentFileSize = sizeInBytes();
if (currentFileSize < offset) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: in which scenario this will happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should never happen, adding it here just for sanity check.

@justinlin-linkedin justinlin-linkedin merged commit d27b50d into linkedin:master Mar 8, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants