heesung-sn commented on code in PR #23515:
URL: https://github.com/apache/pulsar/pull/23515#discussion_r1826880722
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/OwnershipCache.java:
##########
@@ -170,7 +170,17 @@ public CompletableFuture<Optional<NamespaceEphemeralData>>
getOwnerAsync(Namespa
// If we're not the owner, we need to check if anybody else is
String path = ServiceUnitUtils.path(suName);
- return lockManager.readLock(path);
+ return lockManager.readLock(path).thenCompose(owner -> {
+ //If the current broker is the owner, we must try to acquire
ownership to avoid cache loss.
Review Comment:
Seems like we are adding a caching logic in this getter func. Seems like we
are doing more work here although the original intention of this func is to
simply fetch the lock info from the metadata store.
I think we need to confirm
1. if there is any side effect from adding this logic
2. Change the func name and doc that clarifies it also fills the cache.
Usually, we fetches any metadata info from the cache which refreshes and
does async load if not already cached. I am wondering why we are not following
this practice here for this ownership check, or we missed anything that the
original author intended here.
--
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]
