poorbarcode commented on code in PR #20935:
URL: https://github.com/apache/pulsar/pull/20935#discussion_r1287150163


##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java:
##########
@@ -229,6 +229,28 @@ void readTwice() throws Exception {
         entries.forEach(Entry::release);
     }
 
+    @Test
+    void testPersistentMarkDeleteIfCreateCursorLedgerFailed() throws Exception 
{

Review Comment:
   > I think it will not work because the old Ledger will not be removed if the 
new Ledger creation is failed. 
   
   Thank you for reminding me, this is an important point. Since the state of 
the cursor will change to `NoLedger` after the new Ledger creation is failed.  
Cursor will try to persist the information to ZK directly. So it will work. 
   
   > The test covered the case that the cursor didn't have a Ledger before. 
Could you please help also cover the case that the cursor had a Ledger before?
   
   Sure, I added the new 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]

Reply via email to