You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Ivan Kelly <iv...@apache.org> on 2012/03/01 13:07:02 UTC

Re: Review Request: BOOKKEEPER-112: Bookie Recovery on an open ledger will cause LedgerHandle#close on that ledger to fail

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3472/#review5494
-----------------------------------------------------------



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
<https://reviews.apache.org/r/3472/#comment11911>

    Why not use the SafeOrderedExecutor as BookKeeper does?



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
<https://reviews.apache.org/r/3472/#comment11912>

    bitwise &, I guess you meant && here.



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
<https://reviews.apache.org/r/3472/#comment11913>

    I dont like boolean flags like readForward being passed to the constructor here. I think it would be better to have a class called SingleFragmentCallbackWithForwardRead which inherits from SingleFragmentCallback. You can then overload processResult on that.


- Ivan


On 2012-02-28 11:09:19, Sijie Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3472/
> -----------------------------------------------------------
> 
> (Updated 2012-02-28 11:09:19)
> 
> 
> 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 a94a0e5 
>   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 29070eb 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java 99258ac 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java 015e4e4 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java dada67a 
> 
> Diff: https://reviews.apache.org/r/3472/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sijie
> 
>