[
https://issues.apache.org/jira/browse/BOOKKEEPER-362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484607#comment-13484607
]
Sijie Guo commented on BOOKKEEPER-362:
--------------------------------------
Thanks Yixue for the new patch. Several minor comments.
1) seems that NoOpRemoteSubscriptionManager.java is used for testing only. so
why not put it in test package?
2) TestZkRemoteSubManager.java. I don't think you need a separated test case
for it. We already have
hedwig-server/src/test/java/org/apache/hedwig/server/meta/TestMetadataManager.java
for testing metadata manager itself. You could add test case in it.
3) About RemoteSubscriptionManager. I am just thinking to make the interface
protected under conditional write/remove, like TopicPersistenceInfoManager. You
could take a look at TopicPersistenceInfoManager.
the interface may be like setTopicUnsubscribedToRegion(topic, region, version,
cb, ctx).
{code}
+ /**
+ * Hash map for hub-hostname and topic to Zk-node version
+ */
+ private final ConcurrentHashMap<RegionTopicPair, ZkVersion>
regiontopic2version = new ConcurrentHashMap<RegionTopicPair, ZkVersion>();
{code}
so the structure regiontopic2version could be common across different region
subscription manager implementations, "ConcurrentHashMap<RegionTopicPair,
Version>" and put it in RegionManager. so ZKRemoteSubscriptionManager just
takes the responsibility of writing/reading/deleting metadata entries.
Besides that, is there any consideration why not put
ZkRemoteSubscriptionManager under ZkMetadataManagerFactory, or at least in meta
package?
(4) ZooKeeperServiceDownException. I agreed it would be a helpful exception,
but it seems not right in this context. Since RemoteSubscriptionManager is
already an interface, there might be different metadata storage used for this
manager. So when RegionManager interactives with RemoteSubscriptionManager, the
exception what it would expect is an exception indicating it comes from
metadata service down. Suppose RegionManager needs to process the exception
from RemoteSubscriptionManager, it might just handle metadata service down
exception rather handle a specific exception like
ZooKeeperServiceDownException, because ZooKeeperServiceDownException would not
be thrown if using a different metadata storage.
(5) About region name or region address. As my previous comment, I would prefer
region name rather region address. since the region address might be changed.
> Local subscriptions fail if remote region is down
> -------------------------------------------------
>
> Key: BOOKKEEPER-362
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-362
> Project: Bookkeeper
> Issue Type: Bug
> Components: hedwig-server
> Affects Versions: 4.2.0
> Reporter: Aniruddha
> Assignee: Yixue (Andrew) Zhu
> Priority: Critical
> Labels: hedwig
> Fix For: 4.2.0
>
> Attachments: rebase_remoteregion.patch
>
>
> Currently, local subscriptions fail if the remote region hubs are down, even
> if the local hub has subscribed to the remote topic previously. Because of
> this, one region cannot function independent of the other.
> A more detailed discussion related to this can be found here
> http://mail-archives.apache.org/mod_mbox/zookeeper-bookkeeper-dev/201208.mbox/%3cCAOLhyDQSOF+Y+pvnyrd-HJRq1YEr=c8ok_b3_mr81r1g-9m...@mail.gmail.com%3e
--
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
