----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7129/#review11689 -----------------------------------------------------------
hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelManager.java <https://reviews.apache.org/r/7129/#comment25268> rename to submitOpAfterDelay hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelManager.java <https://reviews.apache.org/r/7129/#comment25266> rename to redirectToHost hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelManager.java <https://reviews.apache.org/r/7129/#comment25265> This shouldn't be part of this interface. It should be part of AbstractHChannelManager as it's only ever called from DefaultServerChannel. hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClientImpl.java <https://reviews.apache.org/r/7129/#comment25267> You're breaking your interface here by having getHChannelManager return an AbstractHChannelManager rather than just a HChannelManager. It means the HChannelManager interface is redundant because every implementation would have to extend AbstractHChannelManager to be useful. In effect, AbstractHChannelManager is now the interface. The simple fix is to move the getSubscribeResponseHandler, startDelivery & stopDelivery, asyncCloseSubscription, getSubscriptionEventEmitter from AbstractHChannelManager to the interface. This isn't nice though, because it conflates the concepts of channel management and subscription management, but it's better to have a messy interface, than one that you are bypassing all the time. - Ivan Kelly On Sept. 17, 2012, 12:13 p.m., Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7129/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2012, 12:13 p.m.) > > > Review request for bookkeeper. > > > Description > ------- > > Currently Hedwig client is coupled with the design of one subscription per > channel. In order to multiplex multiple subscription into one channel, it > would be better to refine the Hedwig java client code to provide better > interface to support both mode. > > > This addresses bug BOOKKEEPER-364. > https://issues.apache.org/jira/browse/BOOKKEEPER-364 > > > Diffs > ----- > > > hedwig-client/src/main/java/org/apache/hedwig/client/data/MessageConsumeData.java > ccb9903 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/AbstractResponseHandler.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/MessageConsumeCallback.java > d8f9324 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/PublishResponseHandler.java > d83078e > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeReconnectCallback.java > f746803 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/SubscribeResponseHandler.java > ac209d1 > > hedwig-client/src/main/java/org/apache/hedwig/client/handlers/UnsubscribeResponseHandler.java > f77417f > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/AbstractHChannelManager.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/CleanupChannelMap.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ClientChannelPipelineFactory.java > ca6d706 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ConnectCallback.java > 68867a3 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/DefaultServerChannel.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/FilterableMessageHandler.java > a97bab9 > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannel.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelHandler.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelImpl.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HChannelManager.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigClientImpl.java > 39d9a86 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigPublisher.java > 0891945 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java > 81af1dd > hedwig-client/src/main/java/org/apache/hedwig/client/netty/NetUtils.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/NonSubscriptionChannelPipelineFactory.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/ResponseHandler.java > 127cfb8 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/SubscriptionEventEmitter.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/WriteCallback.java > 41a19d2 > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/package-info.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/simple/SimpleHChannelManager.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/simple/SimpleSubscribeResponseHandler.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/simple/SimpleSubscriptionChannelPipelineFactory.java > PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/simple/SubscribeReconnectCallback.java > PRE-CREATION > hedwig-client/src/main/java/org/apache/hedwig/util/VarArgs.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/7129/diff/ > > > Testing > ------- > > > Thanks, > > Sijie Guo > >
