----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5086/#review11516 -----------------------------------------------------------
hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java <https://reviews.apache.org/r/5086/#comment24746> Wrap inside a synchronizedSet() ? hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java <https://reviews.apache.org/r/5086/#comment24747> This is not necessarily true. The topic may or may not have moved. hedwig-client/src/main/java/org/apache/hedwig/util/SubscriptionListener.java <https://reviews.apache.org/r/5086/#comment24748> Please add a note stating that no blocking operations should be done in processEvent() hedwig-client/src/main/java/org/apache/hedwig/util/SubscriptionListener.java <https://reviews.apache.org/r/5086/#comment24749> Also pass in the channel object that was disconnected. hedwig-protocol/src/main/protobuf/PubSubProtocol.proto <https://reviews.apache.org/r/5086/#comment24750> CHANNEL_DISCONNECTED = 3 and use this event as default hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/AbstractSubscriptionManager.java <https://reviews.apache.org/r/5086/#comment24754> remove topic from topic2sub2seq. Otherwise once we disconnect the subscription channel, a client-side subscription could succeed. - Aniruddha Laud On Sept. 14, 2012, 12:07 a.m., Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5086/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2012, 12:07 a.m.) > > > Review request for bookkeeper. > > > Description > ------- > > In some case, we need to hedwig-client as proxy server to provide messaging > service to other users. > > client -> proxy server 1 -> hedwig > \> proxy server 2 /> > > when client would connect to either proxy server to receive messages, the > proxy server would setup subscription channel to hedwig server. > > we just want client to be simple, so when the channel between client and > proxy server is broken, client will try to connect to proxy servers thru VIP. > it might connect to other proxy server. for example, first time client > connects to proxy server 1, but the client found the connection is broken, it > connects to proxy server 2. when proxy server 2 tried to setup subscription > channel to hedwig, hedwig found that this subscription has existed before > occupied by proxy server 1. > > the panic here is that proxy server 1 only disconnect old subscription > channel only when it detected the channel between client and itself is > broken. The detection might be delayed due to several reasons. so it might > increment the latency that messages are pushed to real client. > > so we try to introduce a subscription mode called CREATE_OR_ATTACH_OR_KILL > mode. > > when a subscriber use this subscription mode, it would kill old existed > subscription channel. when using this subscription mode, we would turn off > auto-reconnect functionality in hedwig client and just tell client about the > channel disconnected event so client could do its logic when channel is > detected. > > in order to provide some admin tool for admin guys to debug/operate, we > provide ADMIN mode. if a subscriber attach to a subscription using ADMIN > mode, its subscription channel would never be killed, then it is safe to > guarantee admin operations. > > > This addresses bug BOOKKEEPER-252. > https://issues.apache.org/jira/browse/BOOKKEEPER-252 > > > Diffs > ----- > > hedwig-client/src/main/cpp/inc/hedwig/callback.h 8d47018 > hedwig-client/src/main/cpp/inc/hedwig/subscribe.h 8ca5b2b > hedwig-client/src/main/cpp/lib/data.cpp 9c5fb37 > hedwig-client/src/main/cpp/lib/subscriberimpl.h beba547 > hedwig-client/src/main/cpp/lib/subscriberimpl.cpp 6d7506b > hedwig-client/src/main/cpp/test/subscribetest.cpp 6883867 > hedwig-client/src/main/cpp/test/util.h 45a6db3 > hedwig-client/src/main/java/org/apache/hedwig/client/api/Subscriber.java > 880fff7 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClientImpl.java > e3d46ca > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java > 5bcf786 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java > 075705a > > hedwig-client/src/main/java/org/apache/hedwig/util/SubscriptionListener.java > PRE-CREATION > > hedwig-protocol/src/main/java/org/apache/hedwig/protocol/PubSubProtocol.java > 723eb26 > hedwig-protocol/src/main/protobuf/PubSubProtocol.proto 7645c5e > > hedwig-server/src/main/java/org/apache/hedwig/admin/console/HedwigConsole.java > 7d8e22b > > hedwig-server/src/main/java/org/apache/hedwig/server/delivery/FIFODeliveryManager.java > b9125ca > > hedwig-server/src/main/java/org/apache/hedwig/server/handlers/SubscribeHandler.java > 1ad9ce1 > > hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java > 7220e23 > > hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/AbstractSubscriptionManager.java > 496817f > > hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/InMemorySubscriptionManager.java > ed04845 > > hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/MMSubscriptionManager.java > b7e22b5 > hedwig-server/src/test/java/org/apache/hedwig/client/TestPubSubClient.java > 2297c56 > > hedwig-server/src/test/java/org/apache/hedwig/server/PubSubServerStandAloneTestBase.java > 8a61223 > > hedwig-server/src/test/java/org/apache/hedwig/server/handlers/TestSubUnsubHandler.java > 5b53f24 > > hedwig-server/src/test/java/org/apache/hedwig/server/subscriptions/StubSubscriptionManager.java > a9df6a5 > > hedwig-server/src/test/java/org/apache/hedwig/server/subscriptions/TestMMSubscriptionManager.java > e06c473 > > Diff: https://reviews.apache.org/r/5086/diff/ > > > Testing > ------- > > > Thanks, > > Sijie Guo > >
