hangc0276 commented on a change in pull request #13575:
URL: https://github.com/apache/pulsar/pull/13575#discussion_r807486500
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -146,6 +151,13 @@
protected static final int AsyncOperationTimeoutSeconds = 30;
+ protected static final String DELETABLE_LEDGER_MARKER_KEYWORD =
"pulsar.ml.deletable.ledgers";
Review comment:
For current implementation, we shouldn't introduce more overload for
current managedLedger Znode storage. we'd better reduce the prefix of keyword
or redesign the data structure for properties storage.
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -2547,21 +2582,15 @@ void internalTrimLedgers(boolean isTruncate,
CompletableFuture<?> promise) {
store.asyncUpdateLedgerIds(name, getManagedLedgerInfo(),
ledgersStat, new MetaStoreCallback<Void>() {
@Override
public void operationComplete(Void result, Stat stat) {
+ // perform actual deletion
+ removeAllDeletableLedgers();
Review comment:
The operationComplete will be called by `bookkeeper-ml-scheduler` thread
pool, If it executed by synchronously, it will block other operations. We can
use another new thread pool the execute it.
For the property map problem, could we use another thread safe data
structure the store it?
##########
File path:
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java
##########
@@ -2103,6 +2103,39 @@ public void testRetentionSize() throws Exception {
});
}
+ @Test
+ public void testTwoPhraseDeletion() throws Exception {
+ @Cleanup("shutdown")
+ ManagedLedgerFactory factory = new
ManagedLedgerFactoryImpl(metadataStore, bkc);
+ ManagedLedgerConfig config = new ManagedLedgerConfig();
+ config.setRetentionSizeInMB(0);
+ config.setMaxEntriesPerLedger(1);
+ config.setRetentionTime(1, TimeUnit.SECONDS);
+ config.setMaximumRolloverTime(1, TimeUnit.SECONDS);
+
+ ManagedLedgerImpl ml = (ManagedLedgerImpl)
factory.open("two_phrase_deletion", config);
+ ManagedCursor c1 = ml.openCursor("testCursor1");
+ ml.addEntry("m1".getBytes());
+ ml.addEntry("m2".getBytes());
+ c1.skipEntries(2, IndividualDeletedEntries.Exclude);
+ // let current ledger close
+ ml.rollCurrentLedgerIfFull();
+ // let retention expire
+ Thread.sleep(1500);
Review comment:
Avoiding to use sleep, it will introduce flaky test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
