Skip to content

Commit ad7589f

Browse files
oneby-wanglhotari
authored andcommitted
[fix][broker] Fix ManagedLedgerImpl.advanceCursorsIfNecessary() method may lose non-durable cursor properties in race condition (#25796)
(cherry picked from commit cbe716121068357de367b20e9b6f5bf08e869cb4)
1 parent 2e79ee3 commit ad7589f

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3149,7 +3149,7 @@ void advanceCursorsIfNecessary(List<LedgerInfo> ledgersToDelete) throws LedgerNo
31493149
&& highestPositionToDelete.compareTo(cursor.getManagedLedger()
31503150
.getLastConfirmedEntry()) <= 0 && !(!cursor.isDurable() && cursor instanceof NonDurableCursorImpl
31513151
&& ((NonDurableCursorImpl) cursor).isReadCompacted())) {
3152-
cursor.asyncMarkDelete(highestPositionToDelete, cursor.getProperties(), new MarkDeleteCallback() {
3152+
cursor.asyncMarkDelete(highestPositionToDelete, null, new MarkDeleteCallback() {
31533153
@Override
31543154
public void markDeleteComplete(Object ctx) {
31553155
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static org.mockito.ArgumentMatchers.anyMap;
2727
import static org.mockito.ArgumentMatchers.anyString;
2828
import static org.mockito.ArgumentMatchers.eq;
29+
import static org.mockito.ArgumentMatchers.nullable;
2930
import static org.mockito.Mockito.atLeast;
3031
import static org.mockito.Mockito.doAnswer;
3132
import static org.mockito.Mockito.doNothing;
@@ -5213,4 +5214,65 @@ public void addFailed(ManagedLedgerException exception, Object ctx) {
52135214
// Verify properties are preserved after cursor reset
52145215
assertEquals(cursor.getProperties(), expectedProperties);
52155216
}
5217+
5218+
@Test
5219+
@SuppressWarnings("unchecked")
5220+
public void testAdvanceCursorsIfNecessaryNeverLoseMarkDeleteProperties() throws Exception {
5221+
ManagedLedgerConfig config = new ManagedLedgerConfig();
5222+
config.setMaxEntriesPerLedger(1);
5223+
config.setRetentionTime(0, TimeUnit.SECONDS);
5224+
config.setRetentionSizeInMB(0);
5225+
5226+
@Cleanup
5227+
ManagedLedgerImpl ledger =
5228+
(ManagedLedgerImpl) factory.open("testAdvanceCursorsIfNecessaryNeverLoseMarkDeleteProperties", config);
5229+
@Cleanup
5230+
ManagedCursorImpl durableCursor = (ManagedCursorImpl) ledger.openCursor("durableCursor1");
5231+
@Cleanup
5232+
NonDurableCursorImpl realNonDurableCursor =
5233+
(NonDurableCursorImpl) ledger.newNonDurableCursor(PositionFactory.EARLIEST);
5234+
NonDurableCursorImpl nonDurableCursor = spy(realNonDurableCursor);
5235+
5236+
ledger.getCursors().removeCursor(realNonDurableCursor.getName());
5237+
ledger.getCursors().add(nonDurableCursor, null);
5238+
5239+
CountDownLatch advanceCursorsMarkDeleteEnteredLatch = new CountDownLatch(1);
5240+
CountDownLatch nonDurableCursorsMarkDeleteCompletedLatch = new CountDownLatch(1);
5241+
CountDownLatch advanceCursorsMarkDeleteCompletedLatch = new CountDownLatch(1);
5242+
5243+
doAnswer(invocation -> {
5244+
Map<String, Long> invocationProperties = invocation.getArgument(1);
5245+
// Pause the advanceCursorsIfNecessary mark-delete so the nonDurableCursor markDelete() can complete first.
5246+
if (invocationProperties == null || invocationProperties.isEmpty()) {
5247+
advanceCursorsMarkDeleteEnteredLatch.countDown();
5248+
assertTrue(nonDurableCursorsMarkDeleteCompletedLatch.await(5, TimeUnit.SECONDS));
5249+
try {
5250+
return invocation.callRealMethod();
5251+
} finally {
5252+
advanceCursorsMarkDeleteCompletedLatch.countDown();
5253+
}
5254+
}
5255+
5256+
return invocation.callRealMethod();
5257+
}).when(nonDurableCursor)
5258+
.internalAsyncMarkDelete(any(Position.class), nullable(Map.class), any(MarkDeleteCallback.class),
5259+
nullable(Object.class), nullable(Runnable.class));
5260+
5261+
ledger.addEntry("entry-1".getBytes(Encoding));
5262+
Position pos2 = ledger.addEntry("entry-2".getBytes(Encoding));
5263+
5264+
// Mark-delete the durable cursor to trigger trimming, which advances non-durable cursors.
5265+
durableCursor.markDelete(pos2);
5266+
assertTrue(advanceCursorsMarkDeleteEnteredLatch.await(5, TimeUnit.SECONDS));
5267+
5268+
String propertyKey = "test-property";
5269+
Map<String, Long> properties = new HashMap<>();
5270+
properties.put(propertyKey, 1L);
5271+
nonDurableCursor.markDelete(pos2, properties);
5272+
nonDurableCursorsMarkDeleteCompletedLatch.countDown();
5273+
5274+
assertTrue(advanceCursorsMarkDeleteCompletedLatch.await(5, TimeUnit.SECONDS));
5275+
assertEquals(nonDurableCursor.getMarkDeletedPosition(), pos2);
5276+
assertEquals(nonDurableCursor.getProperties(), properties);
5277+
}
52165278
}

0 commit comments

Comments
 (0)