[
https://issues.apache.org/jira/browse/BOOKKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13458341#comment-13458341
]
Sijie Guo commented on BOOKKEEPER-407:
--------------------------------------
{quote}
It doesn't hit the synchronized block every time. Before hitting the
synchronized block, we always check if the existing host is the same as the
channel's remote address. If it is, we just return. We will hit the
synchronized block only when a topic moves. And I don't think that will be too
frequent.
{quote}
The contention is not for topic2host, is for host2topics. The checking just for
same topic, but not for different topics. so different topics would compete for
the lock to access host2topics. the case would happened when a hub server is
down or at the start time of the client.
{quote}
Instead of using a ConcurrentLinkedQueue, we should use a Set. We don't want
any repeated values in host2Topics and a Set fits the use-case perfectly.
Collections.synchronizedSet() should give us the concurrency guarantees that
ConcurrentLinkedQueue does in terms of add, remove and get. We just have to
synchronize on the set while iterating over it.
{quote}
to clarify, using replace, it could prevent a repeated value being added to
same list. I agreed that a Set structure is better choice, and in my previous
patch for BOOKKEEPER-364, it did use set.
{quote}
We should use put() instead of replace().
{quote}
using put, you need to synchronization to prevent concurrent operations. while
using replace, you could leverage CAS to achieve fine granularity concurrency.
for two operation setting a topic ownership to different owners, we don't know
which one is correct, which one is wrong. so the differences between your
approach and my approach is that, your approach is let one succeed first, the
other one succeed after that and it would clear the first one. my approach is
to let only one succeed when compete the ownership. both the approaches works,
while my approach provides fine synchronization granularity.
another consideration I need to raise is that since host2topics and topic2host
are just cached structure, if there is wrong entry in them, the logic could
repair them after redirection. I prefer not to sacrifice the performance when
updating them.
> Hedwig client doesn't remove old topic2Host mapping in case of redirect
> -----------------------------------------------------------------------
>
> Key: BOOKKEEPER-407
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-407
> Project: Bookkeeper
> Issue Type: Bug
> Affects Versions: 4.2.0
> Reporter: Aniruddha
> Fix For: 4.2.0
>
> Attachments: BK-407.patch
>
>
> In the storeTopic2HostMapping function, there is a topic2Host.putIfAbsent().
> This doesn't seem correct as we should always update the mapping in case of a
> redirect.
> Reviewboard : https://reviews.apache.org/r/7139/
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
