fix(metrics): count inbound sync stream shedding by reason (#1307) - #1317
Open
MavenRain wants to merge 6 commits into
Open
fix(metrics): count inbound sync stream shedding by reason (#1307)#1317MavenRain wants to merge 6 commits into
MavenRain wants to merge 6 commits into
Conversation
Both shed arms in WorkerNetwork::shed_inbound_sync_stream refuse a
stream before any request frame is read, so no counter saw them and
the only trace was a debug! line hidden by the default info filter.
Add tn_worker.sync_streams_shed_total{worker, reason} with reasons
denied (admission caps hit, deny task spawned) and budget_exhausted
(shed budget full, stream dropped with no reply), recorded at the
admission decision through admit_shed_and_record, before the
best-effort deny write. The pair is the signal: a nonzero
budget_exhausted rate while denied stays low is the signature of
shed slots pinned by peers that never read their deny reply.
Extend the WorkerMetrics unit test and add a wiring test for the
per-arm reason; both confirmed by mutation.
Closes #1307.
Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…-metric Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…-metric Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…-metric Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…-metric Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
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
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.
Closes #1307.
Problem
WorkerNetwork::shed_inbound_sync_streamrefuses inbound sync streams in two ways, both on stream open before any request frame is read: when the admission caps hit it spawns a shed task that writesDeny(AtCapacity), and when the shed budget (MAX_CONCURRENT_SHED_TASKS, #1265) is exhausted it drops the stream with no reply. Neither path reaches a counter, and the only trace is adebug!line the defaultinfofilter hides. On the requester side the budget-exhausted drop reads as a genericfailed to read sync ack frameI/O error, so backpressure is indistinguishable from a transport fault.Fix
crates/consensus/worker/src/metrics.rs:SyncShedReason { Denied, BudgetExhausted }with aconst fn label, andWorkerMetrics::record_sync_stream_shedrecordingtn_worker.sync_streams_shed_total{worker, reason}(same labeled-counter pattern asForwardDropReason/record_forward_dropped).crates/consensus/worker/src/network/mod.rs:WorkerNetworkgains ametrics: WorkerMetricsfield built from the worker id innew. No signature change: the registry keys series by name and labels, so this instance and theRequestHandler's record into the same per-worker series.admit_shed_and_recordwrapstry_admit_shed: a reserved slot recordsdenied, no slot recordsbudget_exhausted.shed_inbound_sync_streamcalls it at the decision point, before the spawn, so the count stays exact when the best-effort,SYNC_REQUEST_READ_TIMEOUT-bounded deny write later fails or the task never runs.budget_exhaustedrate whiledeniedstays low is the signature of shed slots pinned by peers that never read their deny reply (the squatting case from the issue). The pair, not the sum, is the signal. The series carry no peer label; attribution needs the per-peerdebug!lines.Threat model: the counter adds one labeled increment per refused stream, the same cost as the existing
quorum_failures_total/forwarded_txns_dropped_totalcounters, and only on a path that already dropped the stream, so a flooding peer cannot make the responder do more work than before. No admission bound changes:MAX_CONCURRENT_BATCH_STREAMS,MAX_PENDING_REQUESTS_PER_PEER, andMAX_CONCURRENT_SHED_TASKSare untouched. The primary'sPrimaryMetricsmirror is out of scope, per the issue.Testing
gateledger run -- cargo +nightly fmt -p tn-worker -- --check: green.metrics::tests::test_metrics_register_and_updateextended: twoDeniedand oneBudgetExhaustedrefusal produce one series per reason under the worker label with counts 2 and 1. Confirmed by mutation (collapsing thebudget_exhaustedlabel intodeniedfails the test).network::tests::shed_admit_records_reason_per_arm: fills the shed budget throughadmit_shed_and_record(every slot recordsdenied), then one more call is refused and recordsbudget_exhausted. Confirmed by mutation (swapping the two reasons inadmit_shed_and_recordfails the test).gateledger run -- cargo +1.94 test -p tn-worker --libin an isolated target dir: green.cargo +1.94attest runs on the second machine.