[GitHub] [pulsar] codelipenghui commented on a change in pull request #10711: [Transaction] Fix transaction log handle managed ledger WriteFail state.

[GitBox <gi...@apache.org>]

codelipenghui commented on a change in pull request #10711:
URL: https://github.com/apache/pulsar/pull/10711#discussion_r639496319



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -215,7 +215,7 @@
     protected static final int DEFAULT_LEDGER_DELETE_RETRIES = 3;
     protected static final int DEFAULT_LEDGER_DELETE_BACKOFF_TIME_SEC = 60;
 
-    enum State {
+    public enum State {

Review comment:
       It's better do not to expose the internal state of the managed ledger, you can add a method to check if the managed ledger is closed?

##########
File path: pulsar-transaction/coordinator/src/test/java/org/apache/pulsar/transaction/coordinator/MLTransactionMetadataStoreTest.java
##########
@@ -367,6 +370,38 @@ public void testRecoverWhenDeleteFromCursor() throws Exception {
         Awaitility.await().until(transactionMetadataStore::checkIfReady);
     }
 
+    @Test
+    public void testManageLedgerWriteFailState() throws Exception {
+        ManagedLedgerFactoryConfig factoryConf = new ManagedLedgerFactoryConfig();
+        factoryConf.setMaxCacheSize(0);
+
+        @Cleanup("shutdown")
+        ManagedLedgerFactory factory = new ManagedLedgerFactoryImpl(metadataStore, bkc, factoryConf);
+        TransactionCoordinatorID transactionCoordinatorID = new TransactionCoordinatorID(1);
+        MLTransactionLogImpl mlTransactionLog = new MLTransactionLogImpl(transactionCoordinatorID, factory,
+                new ManagedLedgerConfig());
+        MLTransactionMetadataStore transactionMetadataStore =
+                new MLTransactionMetadataStore(transactionCoordinatorID, mlTransactionLog,
+                        new TransactionTimeoutTrackerImpl(), new TransactionRecoverTrackerImpl());
+
+        Awaitility.await().until(transactionMetadataStore::checkIfReady);
+        transactionMetadataStore.newTransaction(5000).get();
+        Field field = MLTransactionLogImpl.class.getDeclaredField("managedLedger");
+        field.setAccessible(true);
+        ManagedLedgerImpl managedLedger = (ManagedLedgerImpl) field.get(mlTransactionLog);
+        field = ManagedLedgerImpl.class.getDeclaredField("STATE_UPDATER");
+        field.setAccessible(true);
+        AtomicReferenceFieldUpdater state = (AtomicReferenceFieldUpdater) field.get(managedLedger);
+        state.set(managedLedger, WriteFailed);
+        try {
+            transactionMetadataStore.newTransaction(5000).get();

Review comment:
       Should add fail() here?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org