Skip to content

Commit 8ab57e1

Browse files
authored
RATIS-2278. Follower Fails to Append Entries Due to Index Validation Race Condition in NavigableIndices (#1248)
1 parent de58f05 commit 8ab57e1

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1694,7 +1694,10 @@ private CompletableFuture<Void> appendLog(ReferenceCountedObject<List<LogEntryPr
16941694
final List<ConsecutiveIndices> entriesTermIndices;
16951695
try(UncheckedAutoCloseableSupplier<List<LogEntryProto>> entries = entriesRef.retainAndReleaseOnClose()) {
16961696
entriesTermIndices = ConsecutiveIndices.convert(entries.get());
1697-
appendLogTermIndices.append(entriesTermIndices);
1697+
if (!appendLogTermIndices.append(entriesTermIndices)) {
1698+
// index already exists, return the last future
1699+
return appendLogFuture.get();
1700+
}
16981701
}
16991702

17001703
entriesRef.retain();

ratis-server/src/main/java/org/apache/ratis/server/impl/ServerImplUtils.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,20 @@ synchronized Long getTerm(long index) {
136136
return floorEntry.getValue().getTerm(index);
137137
}
138138

139-
synchronized void append(List<ConsecutiveIndices> entriesTermIndices) {
140-
for(ConsecutiveIndices indices : entriesTermIndices) {
141-
// validate startIndex
142-
final Map.Entry<Long, ConsecutiveIndices> lastEntry = map.lastEntry();
143-
if (lastEntry != null) {
144-
Preconditions.assertSame(lastEntry.getValue().getNextIndex(), indices.startIndex, "startIndex");
139+
synchronized boolean append(List<ConsecutiveIndices> entriesTermIndices) {
140+
for(int i = 0; i < entriesTermIndices.size(); i++) {
141+
final ConsecutiveIndices indices = entriesTermIndices.get(i);
142+
final ConsecutiveIndices previous = map.put(indices.startIndex, indices);
143+
if (previous != null) {
144+
// index already exists, revert this append
145+
map.put(previous.startIndex, previous);
146+
for(int j = 0; j < i; j++) {
147+
map.remove(entriesTermIndices.get(j).startIndex);
148+
}
149+
return false;
145150
}
146-
map.put(indices.startIndex, indices);
147151
}
152+
return true;
148153
}
149154

150155
synchronized void removeExisting(List<ConsecutiveIndices> entriesTermIndices) {

0 commit comments

Comments
 (0)