You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/11/25 06:19:18 UTC
[GitHub] [accumulo] ctubbsii opened a new issue #1808: LoggingRunnable happily swallows OutOfMemoryErrors (and everything else)
ctubbsii opened a new issue #1808:
URL: https://github.com/apache/accumulo/issues/1808
**Describe the bug**
LoggingRunnable catches Throwable. This is very bad. I saw a compaction thread die due to OutOfMemoryError, and LoggingRunnable logged it, and the tserver kept on running... in a very bad state.
I get that maybe we want a best effort attempt to log when there's a problem in a LoggingRunnable thread... but we should still propagate the Throwable, *especially* if it's an Error type! The tserver (or any other server) can't be trusted to make good decisions after these kinds of problems occur and are ignored by the running system. We should let these be handled by the uncaught exception handler after logging by re-throwing them.
**Versions (OS, Maven, Java, and others, as appropriate):**
- Affected version(s) of this project: 2.1.0-SNAPSHOT (but probably affects 1.x also)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion closed issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
dlmarion closed issue #1808:
URL: https://github.com/apache/accumulo/issues/1808
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-759211941
> I think that is fine. I'm not sure we want to backport my change for this.
We definitely shouldn't backport something like this. These changes are huge, and could have hugely destabilizing effects on the stable 1.10 line. I like the more modest approach @brianloss took in #1865 for 1.10
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-733839847
> I wonder if this is related to - or just similar behavior to ...
@EdColeman I was thinking the same things. It's certainly similar in nature. This catching of Throwable, and/or not setting UncaughtExceptionHandlers on threads we spin off is really likely to bite us. It'd be great if we could take a comprehensive look at 1) all occurrences of us catching Throwable to verify their correctness, and 2) check all thread management code to ensure we properly handle uncaught exceptions.; The main problem is there's a *lot* (of both) of those. This issue is a specific find that was directly observed to be problematic (luckily, it occurred during testing and not on a production system), that can be fixed in a targeted way, regardless of whether we get a chance to do the comprehensive look at the rest.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
EdColeman commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-733831754
I wonder if this is related to - or just similar behavior to the issue in https://github.com/apache/accumulo/issues/1689 That was also triggered by low / OOM, but I do not know at this time if LoggingRunnable was also present in the logs. My recollection is that it is was not, but mentioning for completeness if there is a possibility of a correlation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] brianloss commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
brianloss commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-758725329
I see that #1818 fixes this issue for the 2.1 release. Is the desire for 1.10.2 to simply re-throw the original Throwable, or were people thinking of something more than that?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-735827639
This is related to https://issues.apache.org/jira/browse/ACCUMULO-3723 (though this issue is much more narrowly targeted).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on issue #1808: LoggingRunnable causes servers to happily ignore catastrophic failures
Posted by GitBox <gi...@apache.org>.
dlmarion commented on issue #1808:
URL: https://github.com/apache/accumulo/issues/1808#issuecomment-758772325
I think that is fine. I'm not sure we want to backport my change for this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org