Repository navigation
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bnjoroge1
force-pushed
the
Bnjoroge/queue-gauges
branch
from
October 6, 2026 19:50
4663364 to
51c2192
Compare
Bnjoroge1
force-pushed
the
Bnjoroge/control-migrations
branch
from
October 7, 2026 04:33
dfff47d to
f67e4f7
Compare
Bnjoroge1
force-pushed
the
Bnjoroge/queue-gauges
branch
5 times, most recently
from
October 7, 2026 05:55
c2fc877 to
2facb70
Compare
Bnjoroge1
force-pushed
the
Bnjoroge/queue-gauges
branch
2 times, most recently
from
October 7, 2026 09:24
d821148 to
8b4d963
Compare
…d runs_on The queue-depth/front gauges read the ready set in the global dispatch order (priority DESC, run_order, job_order; the cancel and status reads append the run_id, job_id tie-breakers). The per-pool `jobs_ready` index leads with pool_key, so every global gauge call sorted the whole ready set. A new partial index, `jobs_ready_global`, feeds that order on both backends; the claim path keeps reading `jobs_ready` (its order is that index's key, unchanged). The pg `queue_gauges` front read now takes `LIMIT 1` — only the first row was ever used — and every gauge parse (queue_gauges, the cancel path, ready_front_labels, queue_stats) tolerates a stored `runs_on` that is not a JSON string list instead of failing the command. Lite already read `LIMIT 1` and parsed tolerantly; it gets the matching index. Schema: the index ships as migration `V2026100506__jobs_ready_global_index` in both dialects (Refinery embedded, forward-only) plus `schema.sql`, `lite/schema.sql`, `docs/control-schema.sql` and the chartdb catalog. No schema-version bump here: the migration runner lands with the control-migrations branch and this change stacks after it. Verified with EXPLAIN on populated, analyzed tables: Postgres `Index Scan using jobs_ready_global` with no Sort node, and the claim order still `Index Scan using jobs_ready`; SQLite `SCAN jobs USING INDEX jobs_ready_global` with no temp b-tree. Tests: shared suite front-label test (both backends), lite plan + in-place migration + malformed runs_on, pg plan + non-list runs_on.
The rebase onto main moved the command-outcome gauges to the current contract: `SubmitOutcome` reports `queued_jobs` (there is no `queue_depth`), and `CancelOutcome` carries `queue_nonempty: bool` (the global ready set, computed after the transition) plus `next_runs_on`. The suite test, the lite malformed-`runs_on` test and the pg one asserted fields that no longer exist, so the crate did not compile in any test build. Assert what exists: `queued_jobs` after submit, `queue_nonempty` true while the surviving front job is ready and false once the run is drained, with the front labels still asserted in the global dispatch order (the point of the index). The Postgres malformed-`runs_on` case now pins the *gauge* contract directly (`ready_front_labels` returns no labels instead of failing): the cancel command also runs the promotion sweep, which scans the ready set for admission, and tolerating a non-list `runs_on` there is a scheduling decision this change does not make — the lite twin keeps covering the command path end to end.
The rust-lint job runs `cargo fmt --all -- --check`; the new gauge tests and the migration const list landed unformatted.
Bnjoroge1
force-pushed
the
Bnjoroge/queue-gauges
branch
from
October 7, 2026 15:02
8b4d963 to
df57951
Compare
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.
Stacked on #372 (control migrations). Fixes the PR #347 queue-gauge report.
What changed
pg/lifecycle.rs::queue_gaugesnowLIMIT 1viaquery_opt(only the first row was ever used) and parsesruns_ontolerantly (unwrap_or_default— bad JSON no longer fails the gauge). Same treatment for the live gauge paths:dispatch.rs::ready_front_labels_on,dispatch.rs::cancel_outcome_gauges,lookups.rs::queue_stats.next_ready_labels/queue_statsalreadyLIMIT 1+ tolerant; the missing piece was the index.jobs_ready_global ON jobs(priority DESC, run_order, job_order, run_id, job_id) WHERE queue_state = ready— added to both schema.sql copies, docs, and migrationV2026100506(registered inMIGRATIONSby this branch). The per-pooljobs_readyindex is untouched (claim path keeps using it).queue_gauges_report_the_global_front(both backends); lite EXPLAIN asserts index-fed, no temp B-tree, plus in-place migration + malformedruns_on; pg EXPLAIN (Index Scan using jobs_ready_global, no Sort) + non-listruns_on. Verified ad hoc against disposable PG and SQLite with 100k rows; the repo test suite is the CI gate.Notes
2026100505is reserved for a pending artifact-catalog migration; the ledger intentionally jumps 0504 → 0506. Refinery applies in ascending order, so 0505 landing later is applied normally.pg::queue_gaugeshas no callers on the base (dead code); it was fixed as requested — the live gauge paths are the three above. Decide separately whether to wire it in or drop it.Summary by cubic
Makes the global queue-gauge reads index-fed instead of sorting the whole ready queue on every call, and stops the gauges from failing on a
runs_onthat isn't a JSON string list.The
jobs_ready_globalpartial index serves the global dispatch order (priority DESC, run_order, job_orderplus run/job tie-breakers) on both backends, while the per-pool claim path keepsjobs_readyunchanged. The pg front read now takesLIMIT 1, and a badruns_onreports the depth with no front labels rather than erroring. Tests were ported to the currentCancelOutcomecontract and verified via EXPLAIN that the gauge plans are index-fed with no Sort node.Migration
V2026100506in both dialects; the ledger jumps 0504 → 0506, with 0505 reserved for a pending artifact-catalog migration.pg::queue_gaugeshas no callers on the base (dead code): it was fixed as requested, but decide separately whether to wire it in or drop it.Written for commit df57951. Summary will update on new commits.