You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Sijie Guo <gu...@gmail.com> on 2015/02/06 08:22:43 UTC

Re: Review Request 28835: BOOKKEEPER-634: Provide admin tool to rename bookie identifier in ledger metadata

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



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java
<https://reviews.apache.org/r/28835/#comment117114>

    I don't like the idea to extend AbstractFuture. This is too tight with guava's internal implementation. I'd prefer using a variable for the settable future.
    
    class Read... implements GenericCallback {
    
    SettableFuture future = ...;
    
    public void operationComplete(..) {
    
    future.set(null); 
    // future.setException(..)
    
    }
    
    }


- Sijie Guo


On Jan. 12, 2015, 3:57 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28835/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 3:57 p.m.)
> 
> 
> Review request for bookkeeper, fpj, Ivan Kelly, and Sijie Guo.
> 
> 
> Bugs: BOKKEEPER-634
>     https://issues.apache.org/jira/browse/BOKKEEPER-634
> 
> 
> Repository: bookkeeper-git
> 
> 
> Description
> -------
> 
> admin tool for changing the bookie's identifier to IP/hostname
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java a1e4639 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28835/diff/
> 
> 
> Testing
> -------
> 
> Added few tests
> 
> 
> Thanks,
> 
> Rakesh R
> 
>


Re: Review Request 28835: BOOKKEEPER-634: Provide admin tool to rename bookie identifier in ledger metadata

Posted by Rakesh R <ra...@huawei.com>.

> On Feb. 6, 2015, 7:22 a.m., Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java, line 193
> > <https://reviews.apache.org/r/28835/diff/6/?file=816578#file816578line193>
> >
> >     I don't like the idea to extend AbstractFuture. This is too tight with guava's internal implementation. I'd prefer using a variable for the settable future.
> >     
> >     class Read... implements GenericCallback {
> >     
> >     SettableFuture future = ...;
> >     
> >     public void operationComplete(..) {
> >     
> >     future.set(null); 
> >     // future.setException(..)
> >     
> >     }
> >     
> >     }

Thanks Sijie for the comment. Since this is closed, I've raised BOOKKEEPER-837 to address this.


- Rakesh


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


On Jan. 12, 2015, 3:57 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28835/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 3:57 p.m.)
> 
> 
> Review request for bookkeeper, fpj, Ivan Kelly, and Sijie Guo.
> 
> 
> Bugs: BOKKEEPER-634
>     https://issues.apache.org/jira/browse/BOKKEEPER-634
> 
> 
> Repository: bookkeeper-git
> 
> 
> Description
> -------
> 
> admin tool for changing the bookie's identifier to IP/hostname
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java a1e4639 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28835/diff/
> 
> 
> Testing
> -------
> 
> Added few tests
> 
> 
> Thanks,
> 
> Rakesh R
> 
>