-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"SEVERE: input future failed" spams stdout/stderr #2134
Comments
You say that you're seeing logging "if one future fails." Can you confirm that? My understanding of the code is that we log only if:
or
The logging is still not great, but as a starting point, let's figure out if there another bug on top of it :) |
Yes, it could be more than one error (i don't see the log every single time.) Perhaps I was misled by the wording of the error message? :) |
OK, let's start with the error message. What would you think of something like this?
|
@lukesandberg now, since I'll end up sending him the CL to make this change. Luke, what do you think of the message I propose in my previous post? (I will make it conditional so that we say something different for an (I'm ignoring the larger logging issue for the moment.) |
Sounds good. Just to know, what releae of guava this fix is it going to be On 21 August 2015 at 15:57, Chris Povirk notifications@github.com wrote:
Cheers, |
An improvement to the log message might be able to be included in release 19 if we want. The underlying problem of having too much logging requires more thought, so I wouldn't expect anything until at least release 20. Sorry :( |
No problem, thanks for adding this issue to your roadmap :) On 21 August 2015 at 18:08, Chris Povirk notifications@github.com wrote:
Cheers, |
#2134 (comment) ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=101240012
Hello, just following it up. This error mostly happens during stress tests. In that case it does appear that it's legitimate for multiple futures in a composite to fail. A specific stress test which wasn't failing before now fails because the inordinate amount of I/O generated by the logging message makes it slow down and time out. As a workaround, redirecting stderr to /dev/null fixes it. A similar level of stress could happen in production to an overloaded server. In that case, in practice, the solution (logging) will cause the problem to be worse (increase stress). Forking the repo just to comment that line out starts to look like an attractive alternative :( Can you change the code so it logs ONLY if a certain system property is set (and default to the old behaviour)? |
I ended up nobbling JUL altogether at runtime, following some of the advice found here: http://stackoverflow.com/questions/6077267/java-logging-api-disable-logging-to-standard-output |
In practice the second case will almost always be true if more than one Future fails. The Exceptions are collected in a ConcurrentHashSet. The problem is that almost no Exception implements equals() and hashCode() and it is highly unlikely that multiple threads throw the same instance of an Exception. My log gets spammed with SEVERE messages because Futures.allAsList(...).cancel(true) gets called and most of my interrupted threads throw InterruptedExceptions (by design). Guava thinks one InterruptedException is different from the other, resulting in log spam. |
|
(Some discussion about this in internal CL 246139348, including my theorizing that it might be nice to log exception only if they're of a different class than the original.) |
There is a chance that you can work around this with: whenAllSucceed(futures).callAsync(() -> allAsList(futures)); [edit: That probably doesn't actually work :(] We should still do better someday. |
Hello,
if one future fails in a combined future a large message with stacktrace is spit out via java.util.logging. This is inconvenient.
We know of lots cases where this behaviour cannot or shuold not be avoided (for instance, jobs failing because of timeouts).
The exception is propagated and handled correctly by the combined future so we have other means to find out what's wrong and handle the case gracefully (it has to be done anyway).
I can think of a few workarounds, which may even need to be applied in grops, which are equally pesky:
Can this be avoided? Forking and rebuilding from source just to comment out a logging line is a bit overkill.
The text was updated successfully, but these errors were encountered: