Warn if vaildator appears to be repairing excessively

[jbiseda]

Problem

It is possible for validators to stay in sync with a cluster solely using repair. This is substantially less efficient than receiving data using turbine and most likely indicates network related misconfiguration. Misconfigured nodes generating substantial repair traffic skews metric data.

Summary of Changes

Maintain slot repair history and generate warnings if a node appears to be excessively repairing slots while indicating that it is "caught up" with the cluster.

Fixes #

[codecov]

Codecov Report

Merging #33320 (df3e5c2) into master (982d29c) will decrease coverage by 0.1%.
Report is 3 commits behind head on master.
The diff coverage is 86.4%.

@@            Coverage Diff            @@
##           master   #33320     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         806      806             
  Lines      217499   217660    +161     
=========================================
+ Hits       178019   178138    +119     
- Misses      39480    39522     +42     

[t-nelson]

mind breaking the plumbing out to a separate commit to ease review? same pr is fine, just wanna see the functional changes in isolation to i don't miss anything

[behzadnouri]

The change seems overly intrusive (34 files, 347 loc) just to emit a warning, and most operators do not monitor logs anyways.
On our side, we already have the ability to identify nodes repairing excessively from the existing metrics as in #33166, which is more convenient than inspecting logs.

[jbiseda]

The change seems overly intrusive (34 files, 347 loc) just to emit a warning, and most operators do not monitor logs anyways. On our side, we already have the ability to identify nodes repairing excessively from the existing metrics as in #33166, which is more convenient than inspecting logs.

Granted it touches lots of files, but it's pretty basic refactoring. Yes, we have the tools to investigate as in #33166, but it still requires an investigation. Inspecting logs is inconvenient for us, but more convenient for operators. If this reduces instances where we have to investigate I think it would be a win.

[jbiseda]

Granted it touches lots of files, but it's pretty basic refactoring. Yes, we have the tools to investigate as in #33166, but it still requires an investigation. Inspecting logs is inconvenient for us, but more convenient for operators. If this reduces instances where we have to investigate I think it would be a win.

Do operators look at warnings in the log? @t-nelson ping @behzadnouri

[t-nelson]

Do operators look at warnings in the log? @t-nelson ping @behzadnouri

does posting screenshots on discord constitute "looking"?

[jbiseda]

Do operators look at warnings in the log? @t-nelson ping @behzadnouri

does posting screenshots on discord constitute "looking"?

I guess I'm asking if it's worth trying to help operators with log messages? 🤷

[behzadnouri]

Granted it touches lots of files, but it's pretty basic refactoring. Yes, we have the tools to investigate as in #33166, but it still requires an investigation. Inspecting logs is inconvenient for us, but more convenient for operators. If this reduces instances where we have to investigate I think it would be a win.

Do operators look at warnings in the log? @t-nelson ping @behzadnouri

If the code change was small then adding some more logs was probably fine.
But in this case, my view is that we are not really getting enough value out of the added log message to justify the extra code or the maintenance burden incurred when changing the adjacent code in the future.

[t-nelson]

considering only the second commit, it's not so bad. the first is big 'cause fmt wrapped a bunch of function calls

[jbiseda]

The health status calculation is being changed by: #33651. I'll update this PR following that change.

[t-nelson]

how many nodes are typically in a state that would be logging with this? how frequently?

we can already observe these nodes from metrics, right?

[jbiseda]

how many nodes are typically in a state that would be logging with this? how frequently?

Not a huge amount, maybe ~1 per week in the top 100. I'll look across the full set of validators.

we can already observe these nodes from metrics, right?

Yeah, we see a bump in repair, then find the outliers. I was hoping this would help eliminate some noise.