Skip to content

Commit 9b70ba3

Browse files
authored
[fix][ml] Fix NoSuchElementException in EntryCountEstimator caused by a race condition (apache#25177)
1 parent 45def39 commit 9b70ba3

File tree

2 files changed

+74
-11
lines changed

2 files changed

+74
-11
lines changed

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCountEstimator.java

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Collection;
2323
import java.util.Map;
2424
import java.util.NavigableMap;
25+
import java.util.NoSuchElementException;
2526
import org.apache.bookkeeper.client.LedgerHandle;
2627
import org.apache.bookkeeper.mledger.Position;
2728
import org.apache.bookkeeper.mledger.PositionFactory;
@@ -82,17 +83,15 @@ static int internalEstimateEntryCountByBytesSize(int maxEntries, long maxSizeByt
8283
return maxEntries;
8384
}
8485

85-
// Adjust the read position to ensure it falls within the valid range of available ledgers.
86-
// This handles special cases such as EARLIEST and LATEST positions by resetting them
87-
// to the first available ledger or the last active ledger, respectively.
88-
if (lastLedgerId != null && readPosition.getLedgerId() > lastLedgerId.longValue()) {
89-
readPosition = PositionFactory.create(lastLedgerId, Math.max(lastLedgerTotalEntries - 1, 0));
90-
} else if (lastLedgerId == null && readPosition.getLedgerId() > ledgersInfo.lastKey()) {
91-
Map.Entry<Long, MLDataFormats.ManagedLedgerInfo.LedgerInfo> lastEntry = ledgersInfo.lastEntry();
92-
readPosition =
93-
PositionFactory.create(lastEntry.getKey(), Math.max(lastEntry.getValue().getEntries() - 1, 0));
94-
} else if (readPosition.getLedgerId() < ledgersInfo.firstKey()) {
95-
readPosition = PositionFactory.create(ledgersInfo.firstKey(), 0);
86+
if (ledgersInfo.isEmpty()) {
87+
return 1;
88+
}
89+
90+
try {
91+
readPosition = adjustReadPosition(readPosition, ledgersInfo, lastLedgerId, lastLedgerTotalEntries);
92+
} catch (NoSuchElementException e) {
93+
// there was a race condition where ledgersInfo became empty just before adjustReadPosition was called
94+
return 1;
9695
}
9796

9897
long estimatedEntryCount = 0;
@@ -183,4 +182,28 @@ static int internalEstimateEntryCountByBytesSize(int maxEntries, long maxSizeByt
183182
// Ensure at least one entry is always returned as the result
184183
return Math.max((int) Math.min(estimatedEntryCount, maxEntries), 1);
185184
}
185+
186+
private static Position adjustReadPosition(Position readPosition,
187+
NavigableMap<Long, MLDataFormats.ManagedLedgerInfo.LedgerInfo>
188+
ledgersInfo,
189+
Long lastLedgerId, long lastLedgerTotalEntries) {
190+
// Adjust the read position to ensure it falls within the valid range of available ledgers.
191+
// This handles special cases such as EARLIEST and LATEST positions by resetting them
192+
// to the first available ledger or the last active ledger, respectively.
193+
if (lastLedgerId != null && readPosition.getLedgerId() > lastLedgerId.longValue()) {
194+
return PositionFactory.create(lastLedgerId, Math.max(lastLedgerTotalEntries - 1, 0));
195+
}
196+
long lastKey = ledgersInfo.lastKey();
197+
if (lastLedgerId == null && readPosition.getLedgerId() > lastKey) {
198+
Map.Entry<Long, MLDataFormats.ManagedLedgerInfo.LedgerInfo> lastEntry = ledgersInfo.lastEntry();
199+
if (lastEntry != null && lastEntry.getKey() == lastKey) {
200+
return PositionFactory.create(lastEntry.getKey(), Math.max(lastEntry.getValue().getEntries() - 1, 0));
201+
}
202+
}
203+
long firstKey = ledgersInfo.firstKey();
204+
if (readPosition.getLedgerId() < firstKey) {
205+
return PositionFactory.create(firstKey, 0);
206+
}
207+
return readPosition;
208+
}
186209
}

managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/EntryCountEstimatorTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
package org.apache.bookkeeper.mledger.impl;
2020

2121
import static org.apache.bookkeeper.mledger.impl.cache.RangeEntryCacheImpl.BOOKKEEPER_READ_OVERHEAD_PER_ENTRY;
22+
import static org.mockito.Mockito.mock;
23+
import static org.mockito.Mockito.when;
2224
import static org.testng.Assert.assertEquals;
2325
import java.util.HashSet;
2426
import java.util.NavigableMap;
27+
import java.util.NoSuchElementException;
2528
import java.util.Set;
2629
import java.util.TreeMap;
2730
import org.apache.bookkeeper.mledger.Position;
@@ -289,4 +292,41 @@ public void testMaxSizeIsLongMAX_VALUE() {
289292
int result = estimateEntryCountByBytesSize(Long.MAX_VALUE);
290293
assertEquals(result, maxEntries);
291294
}
295+
296+
@Test
297+
public void testNoLedgers() {
298+
readPosition = PositionFactory.EARLIEST;
299+
// remove all ledgers from ledgersInfo
300+
ledgersInfo.clear();
301+
int result = estimateEntryCountByBytesSize(5_000_000);
302+
// expect that result is 1 because the estimation couldn't be done
303+
assertEquals(result, 1);
304+
}
305+
306+
@Test
307+
public void testNoLedgersRaceFirstKey() {
308+
readPosition = PositionFactory.EARLIEST;
309+
// remove all ledgers from ledgersInfo
310+
ledgersInfo = mock(NavigableMap.class);
311+
when(ledgersInfo.isEmpty()).thenReturn(false);
312+
when(ledgersInfo.firstKey()).thenThrow(NoSuchElementException.class);
313+
when(ledgersInfo.lastKey()).thenReturn(1L);
314+
int result = estimateEntryCountByBytesSize(5_000_000);
315+
// expect that result is 1 because the estimation couldn't be done
316+
assertEquals(result, 1);
317+
}
318+
319+
@Test
320+
public void testNoLedgersRaceLastKey() {
321+
readPosition = PositionFactory.EARLIEST;
322+
// remove all ledgers from ledgersInfo
323+
ledgersInfo = mock(NavigableMap.class);
324+
lastLedgerId = null;
325+
when(ledgersInfo.isEmpty()).thenReturn(false);
326+
when(ledgersInfo.firstKey()).thenReturn(1L);
327+
when(ledgersInfo.lastKey()).thenThrow(NoSuchElementException.class);
328+
int result = estimateEntryCountByBytesSize(5_000_000);
329+
// expect that result is 1 because the estimation couldn't be done
330+
assertEquals(result, 1);
331+
}
292332
}

0 commit comments

Comments
 (0)