Honor higher authorized priority for queued shared checks #2

Merged

qwolff merged 2 commits from fix/dedup-priority-promotion into main 2026-10-15 18:51:02 +00:00

Conversation 1 Commits 2 Files changed 4 +828 -10

qwolff commented 2026-10-15 18:28:39 +00:00

Owner

Copy link

Problem

start_run authorizes/normalizes the requested priority, but an identical-input follower only subscribes to the in-flight result. Its priority is discarded. An authorized stabilizer can therefore wait behind ordinary traffic for a job still queued at sweep priority, despite explicitly requesting the stabilizer class. This is priority inversion, not evidence that a large measured memory estimate is wrong. Promotion cannot guarantee capacity or preempt running jobs.

Change

• Associate each shared check with a scheduler-owned demand handle. Remember higher demand before enqueue and bind grouped checks to the exact queued job ID.
• Raise that waiter's priority monotonically under the scheduler lock, then apply the existing dispatcher. Do not match display labels, promote an entire run, duplicate execution, or cancel anything.
• Keep original session/principal accounting and limits, arrival, resource estimates, capabilities, trust, locality and normal equal-class fairness. Existing authentication, isolation, uncached and test-filter sharing rules are unchanged.
• Ignore attachments to running/removed jobs, including adopted worker jobs and their replacement after worker loss. Preserve legitimate queued promotions across grouped retries.

Regression evidence

Before implementation, cargo test dedup_priority_promotes_only_the_original_same_input_job -- --nocapture failed: the original queued job remained Sweep instead of Stabilizer.

After implementation, CODEX_THREAD_ID=dedup-priority cargo test --bin t dedup_priority -- --nocapture: 8 passed. Covers exact same-input attachment, no new job, unrelated inputs and jobs, grouped checks, attachment before enqueue, monotonicity, unchanged original constraints/principal limits, running/stale handles, adoption loss and retry, denied unauthorized stabilizer requests and Sweep normalization.

cargo fmt --check and git diff --check pass. All compilation went through the cargo wrapper to rcargo.

Full final suite: 280 passed, 6 failed; rerunning only these six also fails on clean base a5b0fe3:

• sandbox::tests::checks_downloading_the_same_crates_at_once_share_cargos_lock: cargo unavailable in sandbox.
• sandbox::tests::checks_with_a_network_of_their_own_cannot_reach_the_nodes_loopback: pasta PID-file permission denied.
• server::tests::logs_are_found_by_check_across_runs_by_run_and_by_the_start_of_their_hash: disk warning replaces expected log.
• sources::tests::a_working_copy_travels_as_its_commit_and_what_differs: commit assertion.
• sweep::tests::a_check_that_never_finishes_holds_up_no_new_commit: timeout.
• sweep::tests::a_restarted_server_follows_the_head_run_it_resumed_without_waiting_for_it: timeout.

Clippy could not run: remote stable toolchain lacks cargo-clippy.

Rollout: separate approval required

No production deployment, restart, attachment or reprioritization has been performed. Runtime reports 462afb1385; this patch is based on latest main a5b0fe3. Existing uncommitted scheduler/node-placement and queue-reporting work in other checkouts was inspected and left untouched.

After approved merge, let the normal main publication produce the image and verify t main/t log publish-image identify the merged commit and immutable image digest. Submit a separate infrastructure change pinning only apps/t/statefulset.yaml to that digest; no worker rollout is required for this server-side change. The current t/t StatefulSet has OnDelete, so syncing that pin alone does not activate it.

After explicit rollout approval and the deployment owner's safe restart window:

kubectl -n t get statefulset t -o jsonpath='{.spec.template.spec.containers[?(@.name=="t")].image}{"\n"}'
# Verify the approved merged-commit digest above before proceeding.
kubectl -n t delete pod t-0
kubectl -n t wait --for=condition=Ready pod/t-0 --timeout=180s
t status --json

Verify the replacement server version includes this merge. Then, under a principal with run and prioritize, use a verified existing qualification checkout whose complete relevant inputs match the queued job (not an arbitrary newer checkout):

t jobs --json
T_TOKEN="$(< "$HOME/.config/t/integrator-token")" t -C "$QUALIFICATION_CHECKOUT" test sdk-web sdk-web-size sdk-linux --priority stabilizer --detach --json
t jobs --json

Before attachment, establish exact input identity and that these jobs are still queued; stop if identity is uncertain or any selected check is uncached/isolated. Verify the original owner run/job remains, its queued priority alone rises and its memory estimate is unchanged, with no new execution. Follow the returned subscriber run with t follow <run> --json. Do not use --force, cancel a run, lower memory, relax OOM handling, or pause unrelated sessions. Cached/running work needs no promotion; t follow alone is read-only and does not request a new priority. These commands are an approval-gated procedure, not executed actions.

## Problem `start_run` authorizes/normalizes the requested priority, but an identical-input follower only subscribes to the in-flight result. Its priority is discarded. An authorized stabilizer can therefore wait behind ordinary traffic for a job still queued at sweep priority, despite explicitly requesting the stabilizer class. This is priority inversion, not evidence that a large measured memory estimate is wrong. Promotion cannot guarantee capacity or preempt running jobs. ## Change - Associate each shared check with a scheduler-owned demand handle. Remember higher demand before enqueue and bind grouped checks to the exact queued job ID. - Raise that waiter's priority monotonically under the scheduler lock, then apply the existing dispatcher. Do not match display labels, promote an entire run, duplicate execution, or cancel anything. - Keep original session/principal accounting and limits, arrival, resource estimates, capabilities, trust, locality and normal equal-class fairness. Existing authentication, isolation, uncached and test-filter sharing rules are unchanged. - Ignore attachments to running/removed jobs, including adopted worker jobs and their replacement after worker loss. Preserve legitimate queued promotions across grouped retries. ## Regression evidence Before implementation, `cargo test dedup_priority_promotes_only_the_original_same_input_job -- --nocapture` failed: the original queued job remained `Sweep` instead of `Stabilizer`. After implementation, `CODEX_THREAD_ID=dedup-priority cargo test --bin t dedup_priority -- --nocapture`: **8 passed**. Covers exact same-input attachment, no new job, unrelated inputs and jobs, grouped checks, attachment before enqueue, monotonicity, unchanged original constraints/principal limits, running/stale handles, adoption loss and retry, denied unauthorized stabilizer requests and Sweep normalization. `cargo fmt --check` and `git diff --check` pass. All compilation went through the cargo wrapper to rcargo. Full final suite: **280 passed, 6 failed**; rerunning only these six also fails on clean base `a5b0fe3`: - `sandbox::tests::checks_downloading_the_same_crates_at_once_share_cargos_lock`: cargo unavailable in sandbox. - `sandbox::tests::checks_with_a_network_of_their_own_cannot_reach_the_nodes_loopback`: pasta PID-file permission denied. - `server::tests::logs_are_found_by_check_across_runs_by_run_and_by_the_start_of_their_hash`: disk warning replaces expected log. - `sources::tests::a_working_copy_travels_as_its_commit_and_what_differs`: commit assertion. - `sweep::tests::a_check_that_never_finishes_holds_up_no_new_commit`: timeout. - `sweep::tests::a_restarted_server_follows_the_head_run_it_resumed_without_waiting_for_it`: timeout. Clippy could not run: remote stable toolchain lacks cargo-clippy. ## Rollout: separate approval required No production deployment, restart, attachment or reprioritization has been performed. Runtime reports `462afb1385`; this patch is based on latest main `a5b0fe3`. Existing uncommitted scheduler/node-placement and queue-reporting work in other checkouts was inspected and left untouched. After approved merge, let the normal main publication produce the image and verify `t main`/`t log publish-image` identify the merged commit and immutable image digest. Submit a separate infrastructure change pinning **only** `apps/t/statefulset.yaml` to that digest; no worker rollout is required for this server-side change. The current `t/t` StatefulSet has `OnDelete`, so syncing that pin alone does not activate it. After explicit rollout approval and the deployment owner's safe restart window: ```sh kubectl -n t get statefulset t -o jsonpath='{.spec.template.spec.containers[?(@.name=="t")].image}{"\n"}' # Verify the approved merged-commit digest above before proceeding. kubectl -n t delete pod t-0 kubectl -n t wait --for=condition=Ready pod/t-0 --timeout=180s t status --json ``` Verify the replacement server version includes this merge. Then, under a principal with `run` and `prioritize`, use a verified existing qualification checkout whose complete relevant inputs match the queued job (not an arbitrary newer checkout): ```sh t jobs --json T_TOKEN="$(< "$HOME/.config/t/integrator-token")" t -C "$QUALIFICATION_CHECKOUT" test sdk-web sdk-web-size sdk-linux --priority stabilizer --detach --json t jobs --json ``` Before attachment, establish exact input identity and that these jobs are still queued; stop if identity is uncertain or any selected check is uncached/isolated. Verify the original owner run/job remains, its queued priority alone rises and its memory estimate is unchanged, with no new execution. Follow the returned subscriber run with `t follow <run> --json`. Do not use `--force`, cancel a run, lower memory, relax OOM handling, or pause unrelated sessions. Cached/running work needs no promotion; `t follow` alone is read-only and does not request a new priority. These commands are an approval-gated procedure, not executed actions.

qwolff added 1 commit 2026-10-15 18:28:39 +00:00

Honor authorized higher priority when attaching to a queued shared check 78584c4e5d

qwolff added 1 commit 2026-10-15 18:46:08 +00:00

Keep adopted priority frozen through fallback enqueue 931d2185a7

qwolff commented 2026-10-15 18:48:30 +00:00

Author

Owner

Copy link

Updated head: 931d2185a7.

Fixed the adoption fallback bug: enqueue no longer rebinds Frozen demand. The regression attaches a higher-priority follower after fallback queues while capacity remains held. Removing the enqueue guard reproduces its failure; restoring the guard passes.

Added dispatch-before/after/concurrent-attach coverage, follower subscription disconnect coverage, and real engine restart tests covering queued arrival order, original-owner resource accounting, resumed priority and ownership, worker adoption, live follower subscription and exactly-once results without duplicate worker execution. The 12 dedup-priority tests pass; the additions were repeatedly exercised. Formatting and diff checks pass.

Full required PR test workflow #111 passed on this exact head: 290 passed, 0 failed, 0 ignored, 0 filtered. Build/publication jobs are intentionally skipped for pull requests. Previous-head workflow #110 also passed (286 tests), but does not validate this fix.

The six earlier local failures are not evidence of scheduler regressions: they passed in CI. Their original individual causes are not all established. Remote diagnosis proved inherited RUSTUP_HOME outside sandbox mounts can cause the nested-cargo failure; source assertion, disk-warning and sweep-timeout causes remain unproven. Sandbox capability coverage remains environment-dependent (bubblewrap/tun availability); no test skips or relaxed limits were introduced.

No merge, deployment, production restart or runtime reprioritization performed. Awaiting reviewer resolution and separate rollout approval.

Updated head: 931d2185a7f0338fc0315e609e4c2289b00c3bb9. Fixed the adoption fallback bug: enqueue no longer rebinds Frozen demand. The regression attaches a higher-priority follower after fallback queues while capacity remains held. Removing the enqueue guard reproduces its failure; restoring the guard passes. Added dispatch-before/after/concurrent-attach coverage, follower subscription disconnect coverage, and real engine restart tests covering queued arrival order, original-owner resource accounting, resumed priority and ownership, worker adoption, live follower subscription and exactly-once results without duplicate worker execution. The 12 dedup-priority tests pass; the additions were repeatedly exercised. Formatting and diff checks pass. Full required PR test workflow #111 passed on this exact head: 290 passed, 0 failed, 0 ignored, 0 filtered. Build/publication jobs are intentionally skipped for pull requests. Previous-head workflow #110 also passed (286 tests), but does not validate this fix. The six earlier local failures are not evidence of scheduler regressions: they passed in CI. Their original individual causes are not all established. Remote diagnosis proved inherited RUSTUP_HOME outside sandbox mounts can cause the nested-cargo failure; source assertion, disk-warning and sweep-timeout causes remain unproven. Sandbox capability coverage remains environment-dependent (bubblewrap/tun availability); no test skips or relaxed limits were introduced. No merge, deployment, production restart or runtime reprioritization performed. Awaiting reviewer resolution and separate rollout approval.

qwolff merged commit c932d22230 into main 2026-10-15 18:51:02 +00:00

qwolff referenced this pull request from a commit 2026-10-15 18:51:02 +00:00

Merge pull request 'Honor higher authorized priority for queued shared checks' (#2) from fix/dedup-priority-promotion into main

xemmerling referenced this pull request 2026-10-29 21:38:49 +00:00

Jobs nothing measured get one jobserver token, run single-threaded, measure parallelism 1, and stay at one core forever while nodes idle #30

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

tilo/t!2

No description provided.