Benedict commented on CASSANDRA-15170:

Thanks.  It's looking good, just a few more minor(ish) items:

* {{NanoTimeToCurrentTimeMillis}}: {{updater}} should be {{final}} to ensure 
visibility, but better still would be to probably use {{InfiniteLoopExecutor}}. 
 I have a number of question marks around the pre-existing code there, though 
(like why it’s using an {{Object}} for waiting for instance), so up to you 
exactly what you do there.
* {{blockingIOThreead}} probably also needs to be {{volatile}}
* I may have lost track of changes here, but any reason why you got rid of the 
{{Feature}} enum in preference for a long?
* {{ColumnFamilyStore.shutdownExecutorsAndWait}}: use {{ImmutableList.of}}?
* Should we introduce {{ExecutorUtils.shutdownAndWait}} since we do it 
* Some of the places you’ve switched {{shutdown}} to {{shutdownNow]} it might 
also be good to switch to {{ExecutorUtils.awaitTermination}} so that we can be 
informed if we haven’t shutdown/

Could you elaborate on what you saw with the single threaded executor?  It 
shouldn’t have had any impact on how long the threads were around for, and it 
might be indicative of some other problem?

For future reference, it helps for review to update existing branches with new 
commits, without rebasing, instead of creating new ones with a clean commit 
history.  We tend to rebase and clean-up just prior to commit, so that the 
reviewer can easily see what has changed between versions.  It's admittedly 
painful working with multiple branches, for all involved.

> Reduce the time needed to release in-JVM dtest cluster resources after close
> ----------------------------------------------------------------------------
>                 Key: CASSANDRA-15170
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15170
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Test/dtest
>            Reporter: Jon Meredith
>            Assignee: Jon Meredith
>            Priority: Normal
> There are a few issues that slow the in-JVM dtests from reclaiming metaspace 
> once the cluster is closed.
> IsolatedExecutor issues the shutdown on a SingleExecutorThreadPool, sometimes 
> this thread was still running 10s after the dtest cluster was closed.  
> Instead, switch to a ThreadPoolExecutor with a core pool size of 0 so that 
> the thread executing the class loader close executes sooner.
> If an OutboundTcpConnection is waiting to connect() and the endpoint is not 
> answering, it has to wait for a timeout before it exits. Instead it should 
> check the isShutdown flag and terminate early if shutdown has been requested.
> In 3.0 and above, HintsCatalog.load uses java.nio.Files.list outside of a 
> try-with-resources construct and leaks a file handle for the directory.  This 
> doesn't matter for normal usage, it leaks a file handle for each dtest 
> Instance created.
> On trunk, Netty global event executor threads are still running and delay GC 
> for the instance class loader.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to