Change getHealth to compare optimistically confirmed slots

[steviez]

Problem

The current getHealth mechanism checks a local accounts hash slot vs. those of other nodes as specified by --known-validator. This is a very coarse comparison given that the default for this value is 100 slots. More so, any nodes using a value larger than the default (ie --incremental-snapshot-interval 500) will 100% see getHealth return status behind at some point.

Summary of Changes

Change the underlying mechanism of how health is computed. Instead of using the accounts hash slots published in gossip, use the latest optimistically confirmed slot from the cluster. The cluster confirmed slot can be compared against the latest optimistically confirmed bank to determine how far behind the node is. This new comparison is much more granular than the old approach, and also has the benefit of not going haywire if a known validator has issues (no domino effect).

Testing

With this branch, I observed a node reporting a diminishing "slots behind" value as it caught up from a process restart. To be sure, I also added a hack to stall ReplayStage for 30 seconds every 1000 slots. Stalling for 30 seconds should result in my node falling behind about 60-75 slots; this hack plus passing --health-check-slot-distance 25 allowed me to see the node fall behind (as reported by solana-validator monitor) immediately after the 30s sleep, but then quickly catch up and continue to report a healthy status.

Fixes #16957

[mvines]

Wait, this actually works? That is, a nodes knows the highest optimistically confirmed slot even if the highest bank it's currently replaying is older than that slot? 🤔

[steviez]

Wait, this actually works? That is, a nodes knows the highest optimistically confirmed slot even if the highest bank it's currently replaying is older than that slot? 🤔

Yessir!

[mvines]

Yessir!

via monitoring gossip votes?

[steviez]

Yessir!

via monitoring gossip votes?

Yep, here is a graph of the optimistic_slot datapoint really zoomed in (1 min) while the node was catching up:

This metric is reported whenever a new optimistic slot is recorded in the blockstore. Note how it is oscillating back and forth between two "bounds". The upper bound line corresponds to optimistic slots from gossip; the lower bound line corresponds to optimistic slots from replaying blocks.

solana/core/src/optimistic_confirmation_verifier.rs

Line 87 in 47540af

datapoint_info!("optimistic_slot", ("slot", new_optimistic_slot, i64),);

[codecov]

Codecov Report

Merging #33651 (fc3649e) into master (452fd5d) will increase coverage by 0.0%.
Report is 10 commits behind head on master.
The diff coverage is 97.1%.

@@           Coverage Diff           @@
##           master   #33651   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         806      806           
  Lines      217595   217574   -21     
=======================================
+ Hits       178065   178093   +28     
+ Misses      39530    39481   -49     

[jbiseda]

Thanks for improving this!

[t-nelson]

can we run a node with this change, minus the removal of the old version, that polls itself ever 5-10s and submits both the health distances via metrics?

[steviez]

can we run a node with this change, minus the removal of the old version, that polls itself ever 5-10s and submits both the health distances via metrics?

Sure. I tracked old and new where old is the existing mechanism and new is the mechanism added by this PR:

• my_slot: what slot the node reporting metrics is at
• cluster_slot: what slot the node's comparison is at
• distance: cluster_slot - my_slot
• healthy: whether the mechanism reported the node as healthy (1 is healthy, 0 is unknown/behind)

Lastly, I left incremental snapshot interval at 100 slots and dropped the health check slot distance to 50. I left solana-validator monitor running in a tmux session, which issues a getHealth request every 2 seconds. The node was restarted around 02:00 UTC (10/14) and finished init at 02:13:

[2023-10-14T02:13:57.583375572Z INFO  solana_metrics::metrics] datapoint: validator-new id="82DZWerqJrHMVYgvGH9RViWsUA9wVo9x66hdAwprqvC9" version="1.18.0 (src:00000000; feat:3079302975, client:SolanaLabs)" cluster_type=1i

The mechanism from this PR reported the node as healthy starting just before 02:26 whereas the old mechanism stabilized around 02:27:30

First off, here is a blow up of the legend to see which color is which trace:

And with that, here are the traces:

Some observations:

• In general, the old and new mechanisms track fairly well in terms of general slope of the traces
• The new mechanism shows a smooth ramp up whereas the old mechanism shows a "stair-step"; this is expected given that the old mechanism is dependent on account hashes which will be pushed every 100 slots by default
• The new mechanism is slightly "ahead" in terms of absolute slot; this is also expected given that the time to take an accounts hash and publish it introduce some latency. Given that we're comparing apples-to-apples, the latency is a non-issue (whereas if we compared the cluster optimistic slot to my node's local latest account hash, that would obviously be problematic)

Lastly, here are the distance plots; technically the same information as above just with a different way of viewing:

So, things are somewhat similar on capable machine. One instance where the new mechanism is night and day better is with an incremental snapshot interval. For example, here is the new vs. old healthy if I use an incremental snapshot (and thus accounts hash) interval of 250 slots:

The square wave is expected for the old mechanism. With the old mechanism, a node will view itself as healthy immediately after publishing a new accounts hash. But, once the comparison nodes publish another hash, the node will view itself as behind for the period until it publishes another one.

Technically, me using a smaller-than-default (50) health check distance lengthens the duration of "unhealthyness". However, the old health mechanism would still exhibit the toggling between healthy/unhealthy in this case. But, the new mechanism would allow tightening this limit significantly without being dependent on known validators.

[t-nelson]

awesome! let's get this in!