> On 2012-03-29 12:44:18, Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java, > > line 444 > > <https://reviews.apache.org/r/3472/diff/7/?file=94988#file94988line444> > > > > Perhaps we should close lh here. It's a noop really, as a non recovery > > lh is a ReadOnlyLedgerHandle, where noop is just a stub. We should close it > > anyhow, just in case it changes to not being a noop in the future.
otherwise the patch looks good to me. +1 - Ivan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3472/#review6515 ----------------------------------------------------------- On 2012-03-23 15:30:15, Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3472/ > ----------------------------------------------------------- > > (Updated 2012-03-23 15:30:15) > > > Review request for bookkeeper. > > > Summary > ------- > > Bookie recovery updates the ledger metadata in zookeeper. LedgerHandle will > not get notified of this update, so it will try to write out its own ledger > metadata, only to fail with KeeperException.BadVersion. This effectively > fences all write operations on the LedgerHandle (close and addEntry). close > will fail for obvious reasons. addEntry will fail once it gets to the failed > bookie in the schedule, tries to write, fails, selects a new bookie and tries > to update ledger metadata. > > Update Line 605, testSyncBookieRecoveryToRandomBookiesCheckForDupes(), when > done > Also, uncomment addEntry in > TestFencing#testFencingInteractionWithBookieRecovery() > > > This addresses bug BOOKKEEPER-112. > https://issues.apache.org/jira/browse/BOOKKEEPER-112 > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java > 37623dc > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java > f71e53f > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java > b403aa1 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > c67a79c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 539d6b2 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > b8923e8 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java > 7de1c10 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java > 0b882c6 > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/CloseTest.java > e28d32c > > Diff: https://reviews.apache.org/r/3472/diff > > > Testing > ------- > > > Thanks, > > Sijie > >
