> On 2012-02-01 15:01:34, Ivan Kelly wrote: > > I had a quick look over this. The exclude bookies stuff needs to be removed > > since BOOKKEEPER-23 gets rid of the need for it & it exposes internal > > details through the public api which is bad. The other fix in here looks > > ok, but I found it hard to pick it out with the excludeBookies stuff in > > there also.
if the failed bookie existed in the quorum set which maxAddConfirmed entry belongs to, readLastConfirmed could not succeed, either open and openNoRecovery would fail. (this issue has been reported in BOOKKEEPER-152, but it seems that it is hard to separate into two patches. so I put them in this patch.) so recovery tool needs failed bookies information as hints to tell readLastConfirmed to skip failed bookies. I think to change excludedBookies related api to protected, not expose to public. what is your opinion? - Sijie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3472/#review4745 ----------------------------------------------------------- On 2012-01-29 09:58:30, Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3472/ > ----------------------------------------------------------- > > (Updated 2012-01-29 09:58:30) > > > 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/BookKeeper.java > 5bb37c3 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java > 37623dc > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java > 547e240 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java > b403aa1 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java > 56186ab > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > 4625bbb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java > 29070eb > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java > 43e999d > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java > 8526db5 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java > 015e4e4 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerOpenTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/3472/diff > > > Testing > ------- > > > Thanks, > > Sijie > >
