[jira] [Commented] (BOOKKEEPER-837) UpdateLedgerOp - Replace AbstractFuture with SettableFuture

["Sijie Guo (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/BOOKKEEPER-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14358214#comment-14358214 ] 

Sijie Guo commented on BOOKKEEPER-837:
--------------------------------------

the patch looks good to me. +1. committing

> UpdateLedgerOp - Replace AbstractFuture with SettableFuture
> -----------------------------------------------------------
>
>                 Key: BOOKKEEPER-837
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-837
>             Project: Bookkeeper
>          Issue Type: Sub-task
>          Components: bookkeeper-auto-recovery, bookkeeper-client, bookkeeper-server
>            Reporter: Rakesh R
>            Assignee: Rakesh R
>             Fix For: 4.4.0, 4.3.2
>
>         Attachments: 001-BOOKKEEPER-837.patch
>
>
> +Review comment :+ Using AbstractFuture is too tight with guava's internal implementation. Instead it would be good to use a variable for the settable future.
> {code}
>  class Read... implements GenericCallback {
>  SettableFuture future = ...;
>   public void operationComplete(..) {
>     future.set(null); 
>     // future.setException(..)
>   }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)