poorbarcode commented on code in PR #20935:
URL: https://github.com/apache/pulsar/pull/20935#discussion_r1292950182
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -2151,7 +2151,13 @@ public void operationFailed(ManagedLedgerException
exception) {
mdEntry.triggerFailed(exception);
}
- });
+ };
+
+ if (State.NoLedger.equals(STATE_UPDATER.get(this))) {
+ persistPositionMetaStore(mdEntry, cb);
Review Comment:
> @hangc0276 @poorbarcode Actually, we have a throttling policy that can be
configured by managedLedgerDefaultMarkDeleteRateLimit. For improving this case,
it looks like we'd better have a separate one for the zookeeper. So that users
can persist to the cursor ledger for each second but persist to zookeeper for
every 30 seconds.
Good idea. Since this solution should change `broker.conf` and
`PersistencePolicies`, maybe a PIP is needed.
In the current PR, I've simpler this behavior to the following: persist to
ZK only hits these two scenarios
- the cursor ledger is not null, persist to ZK after persisting to BK
failed(the original behavior before this PR)
- the cursor ledger is null, and the
`cursor.markDeletedPosition==managedLedger.LAC`. Since the ledger can not be
created, no entry can be written now, so the variable `managedLedger.LAC` will
not increase anymore. For per subscription, persist to ZK will only be executed
once.
And in the future, we submit a PIP to add a rater limiter to further improve
the behavior.
@codelipenghui @hangc0276 What do you think abort?
--
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]
