[VQueues] Track per-entry wait stats and error counts#4894
Conversation
Reworks how scheduler wait-time accounting is persisted, replacing the two ad-hoc EMA fields with the full `WaitStats` breakdown: - `VQueueStatistics` now keeps an EMA over all seven `WaitStats` buckets (`avg_wait_stats`), applied only on scheduler-driven moves so zero samples from non-scheduler transitions no longer dilute the averages. - `EntryStatistics` records `latest_attempt_wait_stats` and a saturating `total_wait_stats` across all attempts, plus a new `num_errors` counter: yields caused by transient errors now count as errors instead of yields. - `MoveMetrics` carries `Option<WaitStats>` instead of two loose `u32`s. - `WaitStats` switches to fixed encoding and gains `ema_apply` / saturating `Add`. Surfaced in SQL: `sys_vqueues.num_errors`, and `sys_vqueue_meta.avg_blocked_on_lock` / `avg_blocked_on_invoker_concurrency`. Entry `Status` display is now kebab-case (`backing-off`) to match `sys_invocation` conventions. Note: breaks the persisted codec for vqueue entries, meta, and pending merge operands (tag renumbering/type changes). Acceptable while vqueues remain experimental and fresh-cluster only.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ba0bafc23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $this.$attr = if $this.$attr == 0 { | ||
| $other.$attr |
There was a problem hiding this comment.
When a wait bucket has only seen zero-valued scheduler samples, this branch keeps treating the bucket as uninitialized and replaces it with the first nonzero sample instead of blending it with the earlier zero samples. For example, after many successful runs with blocked_on_lock_ms == 0, the first locked run will make sys_vqueue_meta.avg_blocked_on_lock jump to the full wait time; VQueueMeta::apply_update calls ema_apply for each scheduler-provided sample on Running transitions, so those zero attempts should affect the average rather than being ignored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
In fact, this is intentional to be reflect positive changes from zero quicker than the decay, otherwise it'll take long until users start observing.
1 participant
Reworks how scheduler wait-time accounting is persisted, replacing the two
ad-hoc EMA fields with the full
WaitStatsbreakdown:VQueueStatisticsnow keeps an EMA over all sevenWaitStatsbuckets(
avg_wait_stats), applied only on scheduler-driven moves so zero samplesfrom non-scheduler transitions no longer dilute the averages.
EntryStatisticsrecordslatest_attempt_wait_statsand a saturatingtotal_wait_statsacross all attempts, plus a newnum_errorscounter:yields caused by transient errors now count as errors instead of yields.
MoveMetricscarriesOption<WaitStats>instead of two looseu32s.WaitStatsswitches to fixed encoding and gainsema_apply/ saturatingAdd.Surfaced in SQL:
sys_vqueues.num_errors, andsys_vqueue_meta.avg_blocked_on_lock/avg_blocked_on_invoker_concurrency.Entry
Statusdisplay is now kebab-case (backing-off) to matchsys_invocationconventions.Note: breaks the persisted codec for vqueue entries, meta, and pending merge
operands (tag renumbering/type changes). Acceptable while vqueues remain
experimental and fresh-cluster only.