CASSANDRA-20978 JVMStabilityInspector does not call LoggingSupport#onShutdown() before killing the JVM

[alanwang67]

Added an additional parameter to JVM shut down so we can call the logger shutdown from inspectCommitLogError(), handleStartupFSError() and userFunctionTimeout().

patch by Alan Wang;

reviewed by for CASSANDRA-20978

Co-authored-by: Name1
Co-authored-by: Name2

[maedhroz]

LGTM...will start a CI run...

[belliottsmith]

Just an fyi that I tried to do this a little while ago and started hitting infrequent deadlocks with some stdout/err -> logger rerouting logic we have, that was much too awful to want to try to unpick at the time. We had to rollback the change.

Maybe you're already aware of this and have worked around it, but I just wanted to make sure it was known.

[maedhroz]

@belliottsmith The only out/err redirect issue I can remember seeing recently was CASSANDRA-20698. Do you happen to have any more details? The main thing I tried to make sure we avoided here was the deadlock on OOM issue if we try to shutdown/flush the logging apparatus with no available memory...

[belliottsmith]

I'm afraid I cannot find a reference to it. It was a problem only on an internal branch, but the problem was caused by LogbackStatusListener. It would periodically deadlock on shutdown and cause tests to timeout. I am sure it is solvable, but it wasn't frequent enough to prevent us committing the change initially, so you may not have spotted it (or, perhaps, it is somehow not a problem upstream).

[maedhroz]

Gotcha. I think we've not seen anything like that in our CI runs so far (CC@alanwang67).

[alanwang67]

I also haven't seen anything like that in our CI runs, though let me take a look at the code path that LogbackStatusListener runs through.