> On 2012-02-15 09:04:42, Sijie Guo wrote: > > 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?
> one is, you didn't update message bound when someone unsubscribes. is there > any decision on it? Well spotted, i've added this now > 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? Ah, there's a bug there also. I should break after finding a subscription with no message bound. > On 2012-02-15 09:04:42, Sijie Guo wrote: > > hedwig-server/src/main/java/org/apache/hedwig/server/delivery/FIFODeliveryManager.java, > > line 344 > > <https://reviews.apache.org/r/3824/diff/1/?file=73881#file73881line344> > > > > I think DEBUG level is better for such kind of message. actually, i only had that in for testing. removed now. - Ivan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3824/#review5119 ----------------------------------------------------------- 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 > >
