Re: [PR] [fix][ml] Prevent terminated managed ledger from transitioning back to writable state [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari commented on code in PR #25795:
URL: https://github.com/apache/pulsar/pull/25795#discussion_r3352091195


##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -1749,8 +1767,11 @@ public void operationComplete(Void v, Stat stat) {
                     synchronized (ManagedLedgerImpl.this) {
                         try {
                             State state = STATE_UPDATER.get(ManagedLedgerImpl.this);
-                            if (state == State.Closed || state.isFenced()) {
-                                log.debug().log("skip ledger update after create complete ledger is closed or fenced");
+                            if (state == State.Closed || state == State.Terminated || state.isFenced()) {

Review Comment:
   Unresolving this comment since there's very useful observations and analysis so far and it's useful to keep the thread visible until we agree on how to address the problems in short term and long term.
   
   I agree that the current ManagedLedgerImpl code path is quite complex. It's hard to reason about the correctness since it's a mixture of locks, synchronization and different executors. 
   
   Just wondering if terminate should be implemented in a way where the state is set to "Terminating" which would be used to prevent any new operations starting, but existing inflight operations would first be completed before the ManagedLedgerImpl could be transitioned into "Terminated" state? The transitioning from Terminating to Terminated would be handled after the metadata update has been successfully completed. If there's a crash in the metadata update, the ledger will continue to be operational. The client requesting the termination can retry if it doesn't receive a successful response to the termination request.
   WDYT?



-- 
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: commits-unsubscribe@pulsar.apache.org

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