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

Sijie Guo commented on BOOKKEEPER-580:
--------------------------------------

> but I wonder why not return OK rather than LedgerClosedException. 

I just want to make the return code clearly to tell what happened.

> I'm not sure why you have removed the call to close in BookieRecoveryTest.

since the ledger handle is already closed. we don't need to close it again.

> In fact, I thought this was the case with TestFencing, so I'm wondering why 
> it is correct to replace the exception like that.

since closeLedgers is closed before recovering, so the ledger handles are 
already closed. closing them again is supposed to return LedgerClosedException. 
we don't get metadata version exception anymore. It is based on the new 
behavior to change that exception.

> One other suggestion is to add comments describing why we need to check if 
> the ledger is closed in those two places.

I could do it if the above comments make sense for you.
                
> improve close logic
> -------------------
>
>                 Key: BOOKKEEPER-580
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-580
>             Project: Bookkeeper
>          Issue Type: Bug
>          Components: bookkeeper-client
>            Reporter: Sijie Guo
>            Assignee: Sijie Guo
>             Fix For: 4.3.0
>
>         Attachments: BOOKKEEPER-580.diff, BOOKKEEPER-580.diff
>
>
> currently, bookkeeper client still write ledger metadata to metadata storage 
> even the metadata is already closed or undergoing closing. which would cause 
> lots of bad version metadata update encountering unrecoverable errors in 
> ledger handle. e.g. NotEnoughtBookiesException.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to