RkGrit commented on code in PR #1248:
URL: https://github.com/apache/ratis/pull/1248#discussion_r2051425599
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerImplUtils.java:
##########
@@ -138,19 +138,14 @@ synchronized Long getTerm(long index) {
synchronized void append(List<ConsecutiveIndices> entriesTermIndices) {
for(ConsecutiveIndices indices : entriesTermIndices) {
- // validate startIndex
- final Map.Entry<Long, ConsecutiveIndices> lastEntry = map.lastEntry();
- if (lastEntry != null) {
- Preconditions.assertSame(lastEntry.getValue().getNextIndex(),
indices.startIndex, "startIndex");
- }
map.put(indices.startIndex, indices);
Review Comment:
I think the object can be not null in the following situation.
When INCONSISTENCY occurs, two AppendEntriesRequest arrive with the same
startIndex.
Then we call map.put for the second, but this function will return the
first, which is not null.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerImplUtils.java:
##########
@@ -138,19 +138,14 @@ synchronized Long getTerm(long index) {
synchronized void append(List<ConsecutiveIndices> entriesTermIndices) {
for(ConsecutiveIndices indices : entriesTermIndices) {
- // validate startIndex
- final Map.Entry<Long, ConsecutiveIndices> lastEntry = map.lastEntry();
- if (lastEntry != null) {
- Preconditions.assertSame(lastEntry.getValue().getNextIndex(),
indices.startIndex, "startIndex");
- }
map.put(indices.startIndex, indices);
}
}
synchronized void removeExisting(List<ConsecutiveIndices>
entriesTermIndices) {
for(ConsecutiveIndices indices : entriesTermIndices) {
final ConsecutiveIndices removed = map.remove(indices.startIndex);
- Preconditions.assertSame(indices, removed, "removed");
+ Preconditions.assertSame(indices.startIndex, removed.startIndex,
"removed");
Review Comment:
I think the object can be different in the following situation.
When INCONSISTENCY occurs, two AppendEntriesRequest arrive with the same
startIndex, both are put into the map. (the second will replace the first)
Then we call map.remove to remove the first, but actually we get the second
in the map.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@ratis.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
