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

Sijie Guo commented on BOOKKEEPER-351:
--------------------------------------

thanks Matteo. the fix is clear, but I had some comments on the test case.

1. It is not a good idea to put the test in TestReadTimeout. from the name of 
'TestReadTimeout', it was used to test read timeout cases. it would be better 
to put the test case in other test cases like BookKeeperTest.

2. 

{code}
+    @Test
+    void testAsyncReadWithError() throws Exception {
+        LedgerHandle lh = bkc.createLedger(3, 3, DigestType.CRC32, 
"testPasswd".getBytes());
+        stopBKCluster();
{code}

I think the runtime exception is thrown due the sheduler is closed when closing 
BookKeeper client. so actually you could just close BookKeeper client only 
instead of stopping bookkeeper cluster.
                
> asyncAddEntry should not throw an exception
> -------------------------------------------
>
>                 Key: BOOKKEEPER-351
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-351
>             Project: Bookkeeper
>          Issue Type: Bug
>    Affects Versions: 4.2.0
>            Reporter: Matteo Merli
>            Assignee: Matteo Merli
>            Priority: Minor
>             Fix For: 4.2.0
>
>         Attachments: 
> 0001-BOOKKEEPER-351-asyncAddEntry-should-not-throw-an-exc.patch
>
>
> There are cases where LedgerHandle.asyncAddEntry() fails with a 
> RuntimeException that is thrown by executor.submit(). 
> It should better invoke the callback with a failure result.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to