[GitHub] [cassandra] smiklosovic commented on a diff in pull request #1563: CASSANDRA-17493 the second take

[GitBox <gi...@apache.org>]

smiklosovic commented on code in PR #1563:
URL: https://github.com/apache/cassandra/pull/1563#discussion_r849237370


##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -5287,12 +5287,16 @@ protected synchronized void drain(boolean isFinalShutdown) throws IOException, I
             CommitLog.instance.shutdownBlocking();
 
             // wait for miscellaneous tasks like sstable and commitlog segment deletion
-            ScheduledExecutors.nonPeriodicTasks.shutdown();
-            if (!ScheduledExecutors.nonPeriodicTasks.awaitTermination(1, MINUTES))
-                logger.warn("Unable to terminate non-periodic tasks within 1 minute.");

Review Comment:
   Yes. With one caveat and that is that instead of waiting 1 minute _for each executor_ (here we had just one), we are waiting 1 minute _for all of them_ to be shutdown / terminated. I consider this to be the flaw of the previous solution. They were not covering all other executors and I think that if whoever did that was taking other executors into account, he would end up with same solution.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org