> On Sept. 14, 2012, 6:18 a.m., Aniruddha Laud wrote: > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java, > > line 99 > > <https://reviews.apache.org/r/5086/diff/3/?file=154671#file154671line99> > > > > Wrap inside a synchronizedSet() ?
from 'http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#newSetFromMap(java.util.Map)', it indicates the returned set displays the same ordering, concurrency, and performance characteristics as the backing map. since I used ConcurrentHashMap, there is no need to synchronizedSet. > On Sept. 14, 2012, 6:18 a.m., Aniruddha Laud wrote: > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java, > > line 332 > > <https://reviews.apache.org/r/5086/diff/3/?file=154672#file154672line332> > > > > This is not necessarily true. The topic may or may not have moved. will remove 'TOPIC_MOVED' to make log statement simple and clearly. > On Sept. 14, 2012, 6:18 a.m., Aniruddha Laud wrote: > > hedwig-client/src/main/java/org/apache/hedwig/util/SubscriptionListener.java, > > line 24 > > <https://reviews.apache.org/r/5086/diff/3/?file=154673#file154673line24> > > > > Please add a note stating that no blocking operations should be done in > > processEvent() good point > On Sept. 14, 2012, 6:18 a.m., Aniruddha Laud wrote: > > hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/AbstractSubscriptionManager.java, > > line 260 > > <https://reviews.apache.org/r/5086/diff/3/?file=154680#file154680line260> > > > > remove topic from topic2sub2seq. Otherwise once we disconnect the > > subscription channel, a client-side subscription could succeed. I added a flag in ReleaseOp to control whether remove topic2sub2seq or not. since StubSubscriptionManager in testing would use it, we should not simply remove it. but I found that there is a bug of my code to stopDelivery in releaseTopic, since subscription states is removed from the map first, it would not be found in #finish() callback, so no subscribers found to stop. why it passed the test case I added? because in that test case, it uses StubSubscriptionManager, the states will not be removed. I will fix this one. - Sijie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5086/#review11516 ----------------------------------------------------------- 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 > >
