-
Notifications
You must be signed in to change notification settings - Fork 429
RATIS-2282. LogAppender Restart Due to Premature Log Entry Access During Concurrent Write Processing #1249
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
Conversation
@szetszwo Plz review. |
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.
@RkGrit , the change looks good. Just two minor comments.
if (records.contains(index)) { | ||
return records.get(index); | ||
} |
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.
Just return without calling contain(..)
:
LogRecord getLogRecord(long index) {
return records.get(index);
}
boolean contains(long index) { | ||
return map.containsKey(index); | ||
} |
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.
contain(..) is not needed.
@szetszwo, thanks for your suggestion. It looks better. |
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.
+1 the change looks good.
…ing Concurrent Write Processing (apache#1249)
…ing Concurrent Write Processing (apache#1249)
…ing Concurrent Write Processing (apache#1249)
…ing Concurrent Write Processing (apache#1249)
…ing Concurrent Write Processing (apache#1249)
What changes were proposed in this pull request?
Put entry to cache before adding the record.
modify the getLogRecord function with contains.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2282
How was this patch tested?
unit tests