----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2594/#review3113 -----------------------------------------------------------
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java <https://reviews.apache.org/r/2594/#comment6922> Can we pl get some javadoc here that specifies the precise semantics of the two new flags. Looking at the code actually makes me understand the semantics, but it will be better if we can create a javadoc here, especially since this is a public api, isn't it? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java <https://reviews.apache.org/r/2594/#comment6924> I have a major question here: suppose client A is executing a Bookie.addEntry(), it first checked that handle is not fencedout and is about to go ahead to write the entry into the log. It is about to execute handle.addEntry(). Now another client B issued a Bookie.addEntry(enableFencing=true). This client sets the fecing flag in the handle and then finishes doing the handle.addEntry() for its own entry. It now returns to the client and client B thinks that it has successfully fenced out other clients. Now the first thread that was processing the request for client A wakes up and finishes the handle.addEntry() call. Can this happen? If so, then we have a problem. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java <https://reviews.apache.org/r/2594/#comment6925> is there a use-case why we need the enableFencing call for both addEntry() and readEntry()? Isn't it enough to have this flag only for readEntry() call? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java <https://reviews.apache.org/r/2594/#comment6926> Is it easy to print which client (client identifier, ip address, etc) has fenced off? This helps debugging. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java <https://reviews.apache.org/r/2594/#comment6923> maybe we should use the volatile keyword here? bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java <https://reviews.apache.org/r/2594/#comment6927> Some java doc here would be really realyl helpful bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java <https://reviews.apache.org/r/2594/#comment6928> some javadocs on the meaning of "options" would be nice. - Dhruba On 2011-11-04 17:40:53, Ivan Kelly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2594/ > ----------------------------------------------------------- > > (Updated 2011-11-04 17:40:53) > > > Review request for bookkeeper. > > > Summary > ------- > > BookKeeper is designed for use as a Write ahead log. In systems with a > primary/backup architecture, the primary will write state updates to the WAL. > If the primary dies the backup comes online, reads the WAL to get the latest > state and starts serving requests. However, if the primary was only > partitioned from the network, or stuck in a long GC, a split brain occurs. > Both primary and backup can service client requests. > > Fencing(http://en.wikipedia.org/wiki/Fencing_%28computing%29) ensures that > this cannot happen. With fencing, the backup can close the WAL of the > primary, and cause any subsequent attempt by the primary to write to the WAL > to give an error. > > > This addresses bug BOOKKEEPER-101. > https://issues.apache.org/jira/browse/BOOKKEEPER-101 > > > Diffs > ----- > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java > d651894 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java > 292617e > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java > 024cac3 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java > d7c8f67 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java > a1fbab7 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java > b3eb5b9 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java > 6f72e47 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java > 8c2a54f > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java > 328c7ca > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java > a68856c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > 7465c52 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java > eddd760 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 385b16c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java > c2d4cee > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java > d70ae27 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java > 873dafe > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java > f1b3ad9 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java > 2cd4de8 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java > 07639aa > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestProtoVersions.java > f6cd8c9 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java > 6bac569 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java > 97dc2ab > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java > ac54d9a > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConcurrentLedgerTest.java > ebb17d2 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LoopbackClient.java > 85822bf > > hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookieBenchmark.java > 18319d7 > > Diff: https://reviews.apache.org/r/2594/diff > > > Testing > ------- > > > Thanks, > > Ivan > >
