codelipenghui commented on code in PR #24522:
URL: https://github.com/apache/pulsar/pull/24522#discussion_r2226276517
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java:
##########
@@ -132,8 +132,16 @@ public void setCloseWhenDone(boolean closeWhenDone) {
public void initiate() {
if (STATE_UPDATER.compareAndSet(OpAddEntry.this, State.OPEN,
State.INITIATED)) {
- ByteBuf duplicateBuffer = data.retainedDuplicate();
+ // Fail the add operation if the managed ledger is in a state that
prevents adding entries.
+ ManagedLedgerException exbw = ml.interceptorException;
+ if (exbw != null) {
+ ml.pendingAddEntries.remove(this);
+ this.failed(exbw);
+ // Don't recycle the object here, see:
https://lists.apache.org/thread/po08w0tkhc7q8gc5khpdft6stxnr1v2y
+ return;
+ }
Review Comment:
> because there is more than one place to call initiate(), so I check the
interceptor exception here.
But the OpAddEntry is first added to pendingAddEntries and only removed
later, correct? If we mark the managed ledger as not ready to accept new
messages, shouldn’t we also prevent adding new items to pendingAddEntries?
Also, the new exception effectively introduces a new state for the managed
ledger—rejecting all new entries when an interceptor exception is present.
We’re now using a different mechanism to represent this state, which adds
complexity (managing two conditions instead of one).
Note that the current implementation already checks isFenced() before adding
entries to pendingAddEntries.
Using a dedicated state to represent this behavior would make the code
easier to reason about. If the managed ledger enters a fenced state, it rejects
all new requests. With the current state + exception approach, the ledger
rejects new requests either when it’s fenced or when the interceptor exception
is not null—effectively two ways of modeling one concept.
--
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]
