You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by gi...@git.apache.org on 2017/06/30 07:43:32 UTC
[GitHub] reddycharan commented on issue #221: TestAuth#testCloseMethodCalledOnAuthProvider is failing

reddycharan commented on issue #221: TestAuth#testCloseMethodCalledOnAuthProvider is failing 
URL: https://github.com/apache/bookkeeper/issues/221#issuecomment-312199104
 
 
   yes, with this change org.apache.bookkeeper.auth.TestAuth$LogCloseCallsBookieAuthProviderFactory.close is called twice.
   
   With or without this change BookieServer.shutdown will be called twice because of the following callstacks:
   -------------
       org.apache.bookkeeper.proto.BookieServer.shutdown(BookieServer.java:165)
       org.apache.bookkeeper.test.BookKeeperClusterTestCase.restartBookies(BookKeeperClusterTestCase.java:424)
       org.apache.bookkeeper.test.BookKeeperClusterTestCase.restartBookies(BookKeeperClusterTestCase.java:373)
       org.apache.bookkeeper.auth.TestAuth.entryCount(TestAuth.java:91)
       org.apache.bookkeeper.auth.TestAuth.testCloseMethodCalledOnAuthProvider(TestAuth.java:148)
       sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   -------------
   
   and 
   
       org.apache.bookkeeper.proto.BookieServer.shutdown(BookieServer.java:165)
       org.apache.bookkeeper.proto.BookieServer$DeathWatcher.run(BookieServer.java:229)
   
   -------------
   
   
   now because of this change, for the second BookieServer.shutdown call, though 'running' value in BookieServer.shutdown method is false, it will call  BookieNettyServer.shutdown, which will in turn call TestAuth$LogCloseCallsBookieAuthProviderFactory.close for the second time. But before this change if 'running' value is false in BookieServer.shutdown method, then it wont call BookieServer.shutdown.
   
   As i mentioned earlier relying on 'running' is not reliable way of handling shutdown logic, especially in the case of partial successful start or partial successful shutdown. Also, all the shutdown/close methods should be idempotent operations. for eg: https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ThreadPoolExecutor.html#shutdown()  https://docs.oracle.com/javase/7/docs/api/java/io/BufferedReader.html#close() .Similarly here calling BookieNettyServer.shutdown on second BookieServer.shutdown should be ok and it should be idempotent operation.
   
   
   So fix for this testcase should be to change the expected value to '2' instead of '1' for this failure.
   
   @eolivelli 
   
    
    
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services