Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/issues/221
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
