315157973 commented on a change in pull request #9900:
URL: https://github.com/apache/pulsar/pull/9900#discussion_r594399795
##########
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/BaseResources.java
##########
@@ -97,6 +98,28 @@ public void set(String path, Function<T, T> modifyFunction)
throws MetadataStore
}
}
+ public void set(String path, OrderedExecutor executor, Function<T, T>
modifyFunction) throws MetadataStoreException {
+ CompletableFuture<Void> callback = new CompletableFuture<>();
+ // Use orderedExecutor to ensure that
+ // concurrent updates in each path are handled by a single thread
+ executor.executeOrdered(path, () ->
Review comment:
The version of zk is not stored with Policies, but a version of one
broker, which can only ensure that it is safe to update ZK concurrently between
different brokers.
t1: Thread-1 writes to zk, updates the local version to v1
t2: Thread-2 writes to zk(use v1) and updates the local version to v2
Local thread safety cannot be guaranteed.
I mentioned a plan in my previous PR, to save a copy of Zk verision to
Policies, so that it becomes an optimistic lock. But there are too many changes
in this plan.
zk is the bottleneck, I don't think CAS zk is a good way.
In addition, if we have other MetaData implementations in the future without
the zk version, wouldn't it be necessary to refactor again? So it would be
better to solve thread safety inside the broker.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
