feat(translator): report share-outcome counters at INFO so they can be read (#810) - #822
Merged
Merged
Conversation
…e read (#810) #810 separated the three refusal reasons and #811 added the recovered-work count, but all four counters were only ever readable through a `debug!`. The fleet runs at INFO, so nothing could read them. Measured on the fleet after the v1.11.33 roll, which is what exposed this: vm1 DEBUG lines in 30m: 0 INFO lines: 492 vm5 DEBUG lines in 30m: 0 INFO lines: 2213 So `grep 'SUPERSEDED target'` returned 0 on every node, and that zero meant "this line can never be emitted", not "no shares were accepted late". A check that cannot fail, guarding exactly the number #811 said was unquantified. A reporter task now writes all four counters every 300s, with per-interval deltas alongside the totals: the totals let two nodes be compared, the deltas show a node that has STARTED refusing, which a monotonically rising total hides. It reports unconditionally, including when everything is zero and unchanged. A reporter that only speaks when something happened cannot be told apart from one that died, and the all-zero line is itself the evidence that the submission path is quiet rather than unobserved. The formatting is a free function so the arithmetic is testable rather than buried in a spawned task. It uses `saturating_sub`: these counters are process-lived, so a report after a restart compares against a `prev` from before it, and plain subtraction would panic in debug and wrap to ~1.8e19 in release — a number that reads as catastrophic share loss. Controls: dropping a counter from the line fails all three tests; swapping `saturating_sub` for `wrapping_sub` fails only the reset test. Claude-Session: https://claude.ai/code/session_01Td1vvfowptTTnu88qG2iym
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
1 participant
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.
Follow-up to #810 and #811, found while trying to use them on the fleet.
The counters existed but could not be read
#810 separated the three refusal reasons; #811 added
shares_accepted_late. All four were only reachable through adebug!. The fleet runs at INFO, measured right after the v1.11.33 roll:So
grep 'SUPERSEDED target'returned 0 on every node — and that zero meant "this line can never be emitted", not "no shares were accepted late". I nearly reported it as a measurement. It is a check that cannot fail, sitting exactly on top of the number #811 explicitly flagged as unquantified.What this adds
A task reporting all four counters every 300s, totals and per-interval deltas:
Totals let two nodes be compared; deltas show a node that has started refusing, which a monotonically rising total hides.
It reports unconditionally, including all-zero-and-unchanged. A reporter that only speaks when something happened is indistinguishable from one that died, and the all-zero line is the evidence that the path is quiet rather than unobserved.
Testability
The formatting is a free function, so the arithmetic is tested rather than buried in a spawned task. It uses
saturating_subdeliberately: counters are process-lived, so a report after a restart compares against aprevfrom before it — plain subtraction panics in debug and wraps to ~1.8e19 in release, which reads in the logs as catastrophic share loss.saturating_sub→wrapping_suba_counter_reset_reports_zero_rather_than_wrappingfailscargo test -p translator_sv2 --lib: 84 passed, 0 failed. Clippy clean, fmt clean.⚠ While writing this I initially inserted the new items between
start()'s doc comment and its signature, orphaning that documentation onto a constant. Clippy'sdoc_lazy_continuationcaught it; fixed, andstart()'s docs are verified reattached.Why it matters now
Without this, #811's effect on the fleet cannot be measured at all — which was the open question left on that PR.
https://claude.ai/code/session_01Td1vvfowptTTnu88qG2iym