codelipenghui commented on code in PR #17164:
URL: https://github.com/apache/pulsar/pull/17164#discussion_r972607432
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -350,21 +356,48 @@ public void operationComplete(Void result, Stat stat) {
public void operationFailed(MetaStoreException e) {
log.error("[{}] Error while updating ledger cursor: {}
properties {}", ledger.getName(),
name, cursorProperties, e);
- updateCursorPropertiesResult.completeExceptionally(e);
+ // if resource is updated by other operate then we
will get bad-version exception
+ // so, retry the operation.
+ if (e instanceof
ManagedLedgerException.BadVersionException) {
+
asyncReadAndUpdateCursorProperties(updateFunction).whenComplete((__, ex) -> {
Review Comment:
I think I made a confused example before
> If we want to update the key A to value 1 (current is 0)
I want to say "we want to update the key A to value 1 based on the value is
0". If the key is absent, we don't need to do this operation.
More detailed explanation:
- It looks like we have data with versions 1, 2, 3
- 2 operations will introduce data versions 4 and 5 based on data version 3
- If we introduce retry logic here means the data from version 3 change to
version 5 is acceptable (although the retry logic will perform the update
operation based on data version actually)
- But the caller doesn't know. If the update operation has some precondition
based on version 3, the precondition has changed in data version 4. The caller
doesn't want the update to continue. If the caller can pass a function, it
should be ok, it is more like a compute operation to a Map. Of course, we can
also use lock for the get-check-set operation for a Map. But here is an async
API.
If we only retry put/remove here, I think it should be ok. It looks like we
can introduce a new API in the feature to support compute operation for cursor
properties. If we have a set operation, it will overwrite the whole cursor
properties, the incoming properties maybe just a new version based on the old
one. The retry will mess up the properties.
As @Jason918 said, it's a concurrency issue. If we have an internal retry,
the callers need to guarantee the concurrent behavior. If we don't have an
internal retry, the callers need to handle the update conflict. In my opinion,
the former is more challenging than the latter
--
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]
