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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
