[GitHub] [bookkeeper] horizonzy commented on a diff in pull request #3365: Fix race condition.

[GitBox <gi...@apache.org>]

horizonzy commented on code in PR #3365:
URL: https://github.com/apache/bookkeeper/pull/3365#discussion_r911624054


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java:
##########
@@ -402,6 +402,7 @@ public void registerLedgerMetadataListener(long ledgerId, LedgerMetadataListener
                 }
             }
             synchronized (listenerSet) {
+                listenerSet = listeners.computeIfAbsent(ledgerId, k -> new HashSet<>());

Review Comment:
   If we don't want second `computeIfAbsent` and still lock by ledgerId, we should define other object which is immutable. likes Map<String, Object> key: ledgerId, value: empty object. But it will introduce more object.
   The simplest way is make register and unregister synchronize, but it will reduce concurrency.
   
   In summary, I think second `computeIfAbsent` is the relatively good way.



-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org