Reduce verbosity of ping message #894
Conversation
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: integration-tests (20.04), integration-tests (22.04), and 2 discussions need to be resolved
a discussion (no related file):
Please rebase on latest main.
nativelink-service/src/worker_api_server.rs line 262 at r1 (raw file):
err, ret(level = Level::INFO), level = Level::WARN,
I don't think this is the one you are looking for. I think the one you want is the INFO message of when it sends/receives the ping. I suggest moving it down to DEBUG level.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: integration-tests (20.04), integration-tests (22.04), and 2 discussions need to be resolved
nativelink-service/src/worker_api_server.rs line 262 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
I don't think this is the one you are looking for. I think the one you want is the
INFOmessage of when it sends/receives the ping. I suggest moving it down toDEBUGlevel.
iirc the INFO level includes ping backs, should we move those log messages to debug ?
aleksdmladenovic
force-pushed
the
reduce-keepalive-log-message-level
branch
from
dcded09 to
c3126db
Compare
|
|
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and 2 discussions need to be resolved
nativelink-service/src/worker_api_server.rs line 262 at r1 (raw file):
Previously, adam-singer (Adam Singer) wrote…
iirc the INFO level includes ping backs, should we move those log messages to debug ?
Yeah, lets use INFO.
aleksdmladenovic
force-pushed
the
reduce-keepalive-log-message-level
branch
from
c3126db to
be72f81
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 2 discussions need to be resolved
nativelink-service/src/worker_api_server.rs line 262 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
Yeah, lets use
INFO.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and 1 discussions need to be resolved
I have reduced the keep alive log message level from `ERROR` to `INFO`.
aleksdmladenovic
force-pushed
the
reduce-keepalive-log-message-level
branch
from
be72f81 to
98e4f6a
Compare
Description
I improved the flexibility by reducing the warning level of the KEEPALIVE log message.
So when we look at the log output in the k8s examples, there are a lot of ping error messages for KEEPALIVE, but I tracked down the right one and reduced it to a lower log level(Warning) should make that output more readable
Fixes #(764)
Type of change
How Has This Been Tested?
Please also list any relevant details for your test configuration
Checklist
bazel test //...passes locallygit amendsee some docsThis change is