Log exceptions in TcpTransport at DEBUG level#51612
Merged
DaveCTurner merged 3 commits intoelastic:masterfrom Jan 31, 2020
Merged
Log exceptions in TcpTransport at DEBUG level#51612DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner merged 3 commits intoelastic:masterfrom
Conversation
When running Elasticsearch on a flaky network, we may see nodes leaving the cluster with reason `disconnected`. It may be useful to the cluster administrator to see the full exception that caused the disconnection, but this is only available with `TRACE` level logging which commingles the details of the problem with other messages that are not useful to end users. This commit promotes logging of exceptions in `TcpTransport` from `TRACE` to `DEBUG` to separate them from the truly `TRACE`-level messages.
Collaborator
|
Pinging @elastic/es-distributed (:Distributed/Network) |
original-brownbear
approved these changes
Jan 29, 2020
Contributor
original-brownbear
left a comment
There was a problem hiding this comment.
Makes sense IMO => LGTM
| Loggers.removeAppender(LogManager.getLogger(TcpTransport.class), appender); | ||
| appender.stop(); | ||
| IOUtils.close(tcpTransport); | ||
| testThreadPool.shutdown(); |
Contributor
There was a problem hiding this comment.
NIT: Maybe use org.elasticsearch.threadpool.ThreadPool#terminate(java.util.concurrent.ExecutorService, long, java.util.concurrent.TimeUnit) here so we don't get the leaked thread warning when running this one?
Tim-Brooks
requested changes
Jan 29, 2020
Contributor
Tim-Brooks
left a comment
There was a problem hiding this comment.
If we are going to implement these tests can we extract the exception handling to a separate class and test in a manner that does not require overriding the transport? See SecurityHttpExceptionHandler as a place where we extracted exception handling from the http transport.
Member
Author
|
Not sure of the value of a whole nother class for this, how about a static method? See a62032c. |
Member
Author
|
@elasticmachine please run elasticsearch-ci/default-distro |
DaveCTurner
added a commit
that referenced
this pull request
Jan 31, 2020
When running Elasticsearch on a flaky network, we may see nodes leaving the cluster with reason `disconnected`. It may be useful to the cluster administrator to see the full exception that caused the disconnection, but this is only available with `TRACE` level logging which commingles the details of the problem with other messages that are not useful to end users. This commit promotes logging of exceptions in `TcpTransport` from `TRACE` to `DEBUG` to separate them from the truly `TRACE`-level messages.
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Dec 15, 2021
In elastic#51612 we promoted certain log messages regarding unexpected network exceptions from `TRACE` to `DEBUG`. In fact it's often useful to see these exceptions by default, so with this commit we show the message (but not the stack trace) at `INFO` level. This commit also adds some commentary about what each of the exceptions means. Closes elastic#66473
DaveCTurner
added a commit
that referenced
this pull request
Dec 16, 2021
In #51612 we promoted certain log messages regarding unexpected network exceptions from `TRACE` to `DEBUG`. In fact it's often useful to see these exceptions by default, so with this commit we show the message (but not the stack trace) at `INFO` level. This commit also adds some commentary about what each of the exceptions means. Closes #66473
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running Elasticsearch on a flaky network, we may see nodes leaving the
cluster with reason
disconnected. It may be useful to the clusteradministrator to see the full exception that caused the disconnection, but this
is only available with
TRACElevel logging which commingles the details ofthe problem with other messages that are not useful to end users.
This commit promotes logging of exceptions in
TcpTransportfromTRACEtoDEBUGto separate them from the trulyTRACE-level messages.