----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3824/#review5119 -----------------------------------------------------------
thanks Ivan. most of this patch is good to me, except two things. one is, you didn't update message bound when someone unsubscribes. is there any decision on it? the other one is, if subscriber 1 subscribe topic with UNLIMITED bound, subscriber 2 subscribe topic with size-bound, you treat it as size-bound. but it would be better to set as UNLIMITED, right? hedwig-server/src/main/java/org/apache/hedwig/server/delivery/FIFODeliveryManager.java <https://reviews.apache.org/r/3824/#comment11219> I think DEBUG level is better for such kind of message. - Sijie On 2012-02-09 16:57:20, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3824/ > ----------------------------------------------------------- > > (Updated 2012-02-09 16:57:20) > > > Review request for bookkeeper. > > > Summary > ------- > > In hedwig, messages for a subscription will queue up forever if the > subscriber is offline. In some usecases, this is undesirable, as it will > eventually mean resource exhaustion. In this JIRA we propose an optional > change to the subscription contract, which allows the user to set a bound on > the number of messages which will be queued for its subscription while it is > offline. > > > This addresses bug BOOKKEEPER-168. > https://issues.apache.org/jira/browse/BOOKKEEPER-168 > > > Diffs > ----- > > hedwig-client/src/main/cpp/test/main.cpp 3290af3 > hedwig-client/src/main/cpp/test/messageboundtest.cpp PRE-CREATION > > hedwig-client/src/main/java/org/apache/hedwig/client/conf/ClientConfiguration.java > 66e049f > > hedwig-client/src/main/java/org/apache/hedwig/client/netty/HedwigSubscriber.java > 5fb1d88 > > hedwig-protocol/src/main/java/org/apache/hedwig/protocol/PubSubProtocol.java > f46868b > hedwig-client/src/main/cpp/inc/hedwig/client.h f37ef98 > hedwig-client/src/main/cpp/lib/client.cpp 6d70ad9 > hedwig-client/src/main/cpp/lib/data.h b4e2c15 > hedwig-client/src/main/cpp/lib/data.cpp a223120 > hedwig-client/src/main/cpp/lib/subscriberimpl.cpp 32075c4 > hedwig-client/src/main/cpp/scripts/tester.sh 5df613a > hedwig-client/src/main/cpp/test/Makefile.am 62902e6 > hedwig-protocol/src/main/protobuf/PubSubProtocol.proto e44d981 > > hedwig-server/src/main/java/org/apache/hedwig/server/delivery/FIFODeliveryManager.java > 2c9af4d > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/BookkeeperPersistenceManager.java > b76023c > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/LocalDBPersistenceManager.java > 02ec607 > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/PersistenceManager.java > 5c38ad9 > > hedwig-server/src/main/java/org/apache/hedwig/server/persistence/ReadAheadCache.java > 0261521 > > hedwig-server/src/main/java/org/apache/hedwig/server/subscriptions/AbstractSubscriptionManager.java > 714a631 > > hedwig-server/src/test/java/org/apache/hedwig/server/persistence/MessageBoundedPersistenceTest.java > PRE-CREATION > > hedwig-server/src/test/java/org/apache/hedwig/server/persistence/StubPersistenceManager.java > 84b866d > > Diff: https://reviews.apache.org/r/3824/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
