This repository has been archived by the owner on Nov 14, 2024. It is now read-only.
[PDS-127121] Log refreshed Cassandra hosts at info level #4917
Conversation
This file contains 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
Generate changelog in
|
jeremyk-91
approved these changes
Jul 27, 2020
| @@ -312,7 +312,7 @@ private void setServersInPoolTo(Set<InetSocketAddress> desiredServers) { | |||
| cassandra.refreshTokenRangesAndGetServers(); | |||
| } | |||
|
|
|||
| log.debug("Cassandra pool refresh added hosts {}, removed hosts {}.", | |||
| log.info("Cassandra pool refresh added hosts {}, removed hosts {}.", | |||
There was a problem hiding this comment.
nit: How valuable is an empty refresh line? Maybe we can have a refresh that did something at INFO and a refresh that did nothing at DEBUG.
jeremyk-91
approved these changes
Jul 27, 2020
| logRefreshedHosts(serversToAdd, serversToRemove); | ||
| } | ||
|
|
||
| private void logRefreshedHosts(Set<InetSocketAddress> serversToAdd, Set<InetSocketAddress> serversToRemove) { |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Goals (and why):
Expose more information about the state of the Cassandra pool in logs.
Implementation Description (bullets):
Changed
DEBUG->INFOon one log line (that details which hosts that have been added and removed).Testing (What was existing testing like? What have you done to improve it?):
N/A.
Concerns (what feedback would you like?):
I don't think this introduces much spam - it is unlikely that anyone refreshes their token ranges / servers more than once per 30 seconds (default is once per 120 seconds), so one log line per refresh is minimal.
As for whether this is enough - I believe so, as it would tell us what hosts that are being added/removed at each point so that we can have a reasonable picture of what the pool is. Naturally this doesn't tell us the actual current state of the pool, only the diff - but
debugLogStateOfPoolis a lot more heavy-handed, and possibly not necessary here.Where should we start reviewing?:
CassandraClientPoolImplPriority (whenever / two weeks / yesterday):
Whenever