Benedict commented on CASSANDRA-15170:

Thanks, looks good.

bq. The shutdown->shutdownNow change on the InfiniteLoopExecutor was just to 
match trunk naming, there's no functional change.

It was an unrelated point that I noticed, at the places you made the 
modification, we were invoking {{Executor.awaitTermination}} instead of 
{{ExecutorUtils.awaitTermination}}, the latter of which actually reports a 
failure to terminate as an exception.  We probably have other places in which 
we are inconsistent, I simply noticed these places while looking at the changes.

It looks like there's a related bad merge to trunk for 
{{shutdownReferenceReaper}}.  The {{EXEC}} and {{STRONG_LEAK_DETECTOR}} are 
being shutdown/awaited separately?

bq.  Sadly I can't reproduce it now and have lost the heap dump showing an 

The problem is that there's no good reason for this to have any impact.  If the 
shutdown task exits, the thread pool will be shutdown, so there's no apparent 
reason to need to depend on the instant core thread timeout.  It's not terribly 
important, since the change should only confer a very minor difference in code 
clarity, but it may cause people to scratch their heads later.  Anyway, it's 
probably not worth further investigation.

bq. The ColumnFamilyStore.shutdownExecutorsAndWait uses the builder so it can 
add the perDiskflushExecutors

My mistake, thanks for clarifying.

bq. We could introduce the shutdown and wait, but would mean 4 variants of it, 
I don't mind it as it is at the moment.

I don't mind terribly either way, either.

> 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