Repository navigation
scheduler: strict patch migration and held concurrency qualification - #180
krunkosaurus wants to merge 11 commits into
Conversation
Default to stock chunked prefill and upgrade the legacy overlay default. Retain explicit decode protection and add progress regressions plus a live canary that rejects serialized HTTP-success responses.
Describe the existing per-request prefill cap for sharing a small batch budget, and distinguish the per-request context limit from combined resident KV capacity.
Reviewed — changes/evidence requested; do not merge yetMaintainer source fixes are now pushed to this PR, preserving author history: Incorporated source fixesThe scheduler patch now validates its canonical helper and both gates, rejects missing/drifted/duplicated definitions without rewriting the target, and compiles successful rewrites. Current-main/TP3 changes are integrated without broadening TP4 policy. Refreshed prefill-concurrency, pinned-scheduler, mixed-policy, long-prefill-threshold, launcher override, numeric and rank-parity checks passed. Why performance is still unqualifiedThe failed acquisition was my harness error, not a demonstrated scheduler regression. My wrapper supplied the server origin instead of the full completion URL; the first A warmup returned HTTP 404. There were zero measured waves. The old operator also failed to propagate capture rejection and began B loading; it was interrupted and restored safely before any B capture. A2 was not captured. No replacement acquisition was used to turn this into a pass. Next actions — maintainer harness correction, then scheduled qualification
Required qualification before marking readyUse the published TheGrill v0.2.0 experimental prerelease, from source Follow the measurement/evidence scope, claim-specific recipe guidance, and report template. Pin the executable, workload/producer, actual backend/runtime and model revisions, and declared settings. Qualify supported request controls before the prospective acquisition; use the full For this PR, please keep it draft and mark it ready only after the requested changes and separate correctness/numerical checks pass, and the relevant TheGrill 0.2.0 reports complete and satisfy the predeclared PR-specific acceptance gates. Then request maintainer re-review of the exact final head. Exit 0 or TheGrill publication is not blanket backend support or persisted-model-KV/eviction proof; unsupported claims need separate real-backend evidence or must remain explicitly unqualified. This is the review gate for this PR, not a claim that every project must adopt TheGrill. Disposition: reviewed; changes/evidence requested; merge hold. Tooling availability is now resolved, so only Coordinated scheduler scope and explicit unblock conditionThe maintainer selects stock Next-action owner: maintainer (plotarmordev) for the acquisition-wrapper correction, integration and prospective protocol/resource scheduling. The author is not being asked for another immediate run or to fix already-correct canary/source behavior. The study outline above is a qualification requirement, not a fully specified executable experiment: the maintainer must first pin supported controls, workload, limits and acceptance criteria. After valid acquisition and separate correctness checks meet those predeclared gates on the final head, the maintainer performs exact-head re-review before readiness or any production default landing. Until then the draft/evidence/merge hold remains; no gates have passed by virtue of this correction. |
plotarmordev
left a comment
There was a problem hiding this comment.
Changes/evidence requested on 587b39e058069a87bf39d02eb2cc8ea32bdf0a7e. Maintainer fixes are pushed and scoped CPU checks passed; this is not approval of the remaining runtime claims. Specific remaining changes, acknowledged positive/negative evidence, and exact readiness gates are in the updated maintainer handoff.
Use TheGrill v0.2.0 experimental prerelease with its published CI-built artifact checksums/receipts, not staging hashes. Keep the PR draft until its separate correctness/numerical checks and relevant predeclared performance gates are satisfied; then request exact-head re-review. Tooling availability is resolved, but the merge hold remains. No additional GPU qualification is claimed by this review.
… canary unchanged PR180's validator work re-expressed against main's versioned installer instead of forking a second helper/gate definition. - overlay/patch_scheduler_decode_floor.py: byte-identical to the MiaAI-Lab#80 branch (installer v6). It validates an existing patch by unpatch/re-patch round-trip (a marker alone is never trusted), refuses drift/partial/duplicate/decorated patches without a write, is idempotent on re-apply, migrates v1-v5 images, and accepts the opt-in warm/deadline gate forms so a supported gate-v2 scheduler is not rejected at container start. - start.sh / .env.example / README.md: main's defaults kept (fair on TP=2, skip on TP=4, FP8 off). The TP=2 CHUNK=0 default flip is held as a gated candidate; it is not landed here. - tests/check_concurrent_agents.py: unchanged from the reviewed head (full completion URL, fail-closed assessment, socket cancel, no API key in receipts). - tests/test_prefill_concurrency.py: rewritten to the installed policy (the old copy exec'd the v1 HELPER literal and asserted the pre-fair launcher defaults). Patch-integrity/migration assertions moved to tests/test_scheduler_decode_floor.py, which now runs the v0-v5 matrix, re-apply idempotence, drift and marker-only refusal against the same installer. - docs/concurrent-agents.md and CHANGELOG: shipped defaults, the migration contract and the v6 acceptance stated explicitly.
…round-trip v1-v5 The v6 installer removed everything between `class _Glm53MixedPrefill:` and the next anchor without checking what it removed, and only a v6 image was round-trip verified: v1-v5 were unpatched and rewritten on the strength of their markers. The migration tests built their legacy fixtures as `class ... pass`, so a real legacy image was never exercised. - registry of the published helper texts (v1 f3043c9, cross-attested byte-identical by cad980f7263:HELPER_V1; v2 9a23b2d; v5 a9bbbad), keyed by sha256 and length, validated BEFORE anything is removed. v3/v4 have no published producer (180725a introduced their markers only), so their site is validated structurally: one undecorated class plus its binding and policy function, the trailer that version's own frozen sites call, and the members those sites need. v6 is compared against the installer's own text. - one exact byte range is removed and the whole file is round-trip checked: re-adding that span and re-applying the version's frozen sites must reproduce the input byte-for-byte. Missing, duplicated, decorated, drifted, marker-only and unpublished variants are refused with no write; the unversioned "0"-default build is refused because that baked default is a held direction. - the fabricated `class ... pass` helpers are replaced by the real published texts in tests/fixtures/legacy_scheduler_helpers.py, and every migration must land on exactly the bytes a fresh install of the pristine scheduler produces. Refusal and idempotence cases cover drift, duplication, decoration, a required-member gap and unrelated bytes after the helper. The v1 strip also left (or dropped) a byte relative to the pristine scheduler; the byte-identity assertion catches that now. v6 defaults are unchanged: the warm bypass and the deadline stay 0, and `fair` is untouched.
… default flips held Conservative split of PR149 against main's fresh scheduler defaults (fair v5 on TP=2, skip on TP=4, GLM53_DENSE_FP8=off). - overlay/exl3.py, overlay/patch_dense_fp8.py, start.sh: FP8 stays off at every site; the KL-panel screening numbers are documented as a shared-support proxy, not a qualification pass. - CHUNK=0 TP=2 default is NOT landed: it is a gated candidate until #180's contention qualification, and main's fair v5 default stands. PR149 therefore keeps its scheduler/threshold/quality work without rewriting main's adopted defaults. - LONG_PREFILL_TOKEN_THRESHOLD: unset now derives half the validated MNBT, clamped to [1,3584]; explicit empty keeps the stock scheduler; invalid explicit values are rejected pre-restart; the derived value reaches ARGS and the container env on both launchers and both ranks (TP=4 gains the flag). - scripts/quality/*, tests/test_kl_panel_compare.py, tests/test_long_prefill_threshold.py carried. The deleted test_documented_defaults / test_recipe_stamp_wiring stay deleted (source-text/default pinning). - Review of the carried quality scripts found no false-scoring defect: compare_arms propagates kl_panel refusals (exit 2), capture refuses on errors/warnings while retaining the partial file, toolcall counts leaks and malformed JSON as failures, code_eval discloses truncation/no-code-block/error counts.
…s, bound-case test Same installer and migration matrix as the MiaAI-Lab#80 cleanup branch (byte-identical): a v3/v4 marker is refused without touching the source because no authenticated producer of those helper bodies was recovered, while v1/v2/v5 still migrate to v6 and re-verify by byte-identical round trip. Remove the shadowed duplicate _helper_text() and V4_PAIRS definitions, the unused HELPER alias, and the v3 pair table plus the structural member list that existed only for the removed path. Fair and gate defaults are unchanged. tests/test_prefill_concurrency.py: the capped bound case now proves the newcomer is offered min(cap, remaining) tokens on every step under a decoding peer, instead of only proving eventual completion. Docs: the migration wording and the CPU-coverage paragraph state the v3/v4 refusal.
plotarmordev
left a comment
There was a problem hiding this comment.
Source-only final review at 4ffef08: normal fast-forward verified; strict authenticated v1/v2/v5 migration and v6 reapplication accepted against Main CPU/public-producer receipts and independent HELP review. Unsupported v3/v4 refuse without mutation and require approved restore/reinstall, not marker removal. Gate knobs remain opt-in 0/0, deadline means eligibility only, and warm observations are exploratory. Older-base publication does not qualify current main or replace later TP3/TP4 defaults. Maintainer plotarmordev owns current-main integration, exact-head cumulative checks and authorized runtime/contended qualification; no author homework, numerical approval or merge. HOLD remains.
plotarmordev
left a comment
There was a problem hiding this comment.
Hold update
Public head checked: 4ffef088edb18797ba2763a84e38fd520d39a0f7.
Why it is held: The live contention check did not establish the progress we need. Request B produced no content before request A finished its 1,024-token output, about 21 seconds later. B was cancelled. Both HTTP responses were 200; that alone was not a fairness pass.
What we tested: 30 CPU scheduler checks passed. The bounded live run used a private integrated candidate and the earlier scheduler settings. It was not a test of the newer defaults from #194, and it does not by itself identify the cause.
Next action — me, @plotarmordev: Integrate the selected scheduler work and define the exact contention comparison before scheduling it. Use overlapping long-prefill and active-decode requests, unchanged controls, fixed limits, and per-request progress, time to first output and completion time. Count an unfinished request as incomplete, not success.
Author action now: None. Your source/migration fixes were accepted in the current-head review. I am clearing the old native change request; this remains a maintainer-owned qualification hold, not merge approval.
Please keep the hold until the conditions above are met. When a new version is ready for review, include its commit ID, the checks actually run, their results, and anything not tested. Keep failed or incomplete runs in the report. Share only sanitized results—not private prompts, credentials, or machine configuration.
Status correction: source changes have been accepted; the remaining hold is maintainer-owned protocol/integration/qualification work. No new author run or rebase is requested. See the latest current-head hold update. This dismissal is not merge approval.
Maintainer hold update — original #180; tracking #283Reviewed public head: A private upstream-intended combined candidate received independent source-only review and CPU checks (379 passed / 65 skipped); both baseline and candidate TP2 images booted. In the first frozen baseline natural-pressure run, native evidence observed a selected WAITING allocator refusal, but all three trials were invalid (one truncated trace, two W stream timeouts). Neither candidate C nor the return-to-baseline control was scored. No TheGrill performance repetitions were run. The historical contention hold therefore remains; HTTP 200, boot success and CPU checks do not constitute its fairness/latency/throughput pass. Next owner: maintainer @plotarmordev until @hecisaza accepts the invitation to be single integration lead in #283. Unblock: integrate strict migration/current-main policy without unintended default changes, obtain valid published-recipe native correctness and claim-specific TheGrill v0.2.0 performance evidence under a prospective protocol, then independent exact-final-head review and normal original-PR merge. Compatible contributor hardware is fine. The failed acquisition is retained, not replaced or blamed on this PR's author. No immediate author rebase or benchmark request; draft/merge hold stays in force. |
…ic producers Address review on #309: - installation_tests() verifies an already-current v7 scheduler exactly (as the patcher's main() does) before unpatching, then checks it equals a fresh install and that re-applying is a verified no-op. - New multi-step regression: a selected waiter natively refused KV lets the runnable peer progress in the same step, is not preselected for one step, stays a candidate and recovers once memory frees, while the peer keeps progressing (#246). Fails if the refusal is ignored or the refused waiter keeps its slot. - Byte-exact producer fixtures for every accepted public identity: v1-image-d9758a6 (helper rebuilds image ef9f5013 byte-for-byte), v5-historical (e329e55), v6-warm-deadline (#180, f89e240) and v6-carry (#246, dc702b4), each migrated and re-applied.
…er_helpers.py Replace the six vendored copies of older patcher scripts under tests/fixtures/decode_floor_producers/ with the byte-exact helper texts of the same identities (v1-image-d9758a6, v5-main, v5-priority, v6-warm-deadline, v6-carry), appended to tests/fixtures/legacy_scheduler_helpers.py in the format #180 established for v1/v2/v5-historical, with provenance in its docstring. installation_tests() rebuilds every accepted public identity the same way: helper text at the installer's anchor plus that version's hook pairs. Each rebuild was checked byte-identical to the output of the removed patcher copy, and the installer still authenticates each by sha256 and length. v5-historical is now exercised directly as well. Same coverage (9 of 11), about 3,600 fewer lines, no new file layout.
…s decode-floor v7 (MiaAI-Lab#283) Replace the fair-prefill decode-floor overlay with the integrated v7 patcher qualified in MiaAI-Lab#283: - a selected WAITING request refused KV allocation no longer stalls a runnable prefill; it keeps being retried and recovers once memory frees (MiaAI-Lab#246) - explicit priorities keep ranking fair-prefill candidates under --scheduling-policy priority; FCFS is unchanged (MiaAI-Lab#221) - installer migration follows MiaAI-Lab#180's fail-closed contract: pristine, v1, v2, v5 and v5+priority installs migrate to the fresh-install v7 bytes; v3/v4, drifted, duplicated, marker-only and unmarked helpers are refused unwritten patch_mamba_align_chunking.py accepts decode-floor v7. MiaAI-Lab#180's concurrency canary, its docs and its CPU progress regressions land with it. The decode-floor installation test is ported to the v7 contract with deployed producer fixtures, and the mamba A4 fixture downgrades v7 as well as v5. On image ef9f5013 these overlays install scheduler sha256 97c90a18fd0df6f9aeb1452c39801455d90756d315c6846ca0642e934fe4e269, the identity qualified live in MiaAI-Lab#283 (baseline -> candidate -> baseline, TP2). Co-authored-by: RandomLlama <108938230+beastllama@users.noreply.github.com> Co-authored-by: Surlebeat <64695525+surlebeat@users.noreply.github.com> Co-authored-by: Mauvis Ledford <20738+krunkosaurus@users.noreply.github.com>
|
Landed through #309: the strict migration contract, the concurrency canary and its CPU regressions. The canary passed 5/5 in every arm. Thanks @krunkosaurus. The TP=2 |
Reviewed source publication — HOLD, not merge-ready
Exact head
4ffef088edb18797ba2763a84e38fd520d39a0f7was fast-forward published to the original author branch, preserving history. Integrated base:a9bbbadc4c7253870d0d519c3d445e4ee7b97540. This is older-base source publication, not current-main integration qualification. Later public main changes, including the TP=3/TP=4 default decisions, must be preserved in a subsequent normal integration; this publication does not overwrite main or establish current default policy.Current source scope
Retain the single canonical installer, policy-progress checks, existing concurrency canary and concurrency documentation. The installer is byte-identical to #80, accepting the supported v6 opt-in helper without introducing a second policy implementation. This branch does not change launcher defaults or land the proposed TP=2
CHUNK=0default. The concurrency canary is not a completed contention study; no new live result is claimed.Strict installer migration and recovery requirement
One shared installer authenticates published v1/v2/v5 helper bytes, validates exact helper/gate regions before replacement, requires round-trip consistency and verifies current v6 idempotence. Main independently matched the three registry SHA-256/length entries against public producer source, not only the test fixtures. Drifted, partial, duplicated, decorated and marker-only inputs are refused without a write.
v3/v4 migration is unsupported and fails closed without rewriting the source. No authenticated producer for those helper bodies was recovered; method names or invented fixtures do not establish canonical provenance. An operator encountering these markers must restore/reinstall the pinned pristine or an authenticated supported scheduler through an approved maintenance procedure before applying v6; do not erase markers or bypass validation. This is a deliberate narrowed support contract, not a claim that every historical variant migrates.
CPU evidence and remaining hold
Main supplied the pinned-scheduler installer plus 30 fair-policy checks and five corrected concurrency-policy checks. Independent HELP review accepted the shared strict installer delta. CPU policy progress is not live fairness, throughput or latency qualification. This final reviewer read the supplied evidence and focused deltas; no tests, live endpoints, model/GPU/node/service operations were run here. Historical runtime receipts remain tied to their recorded source/image and are not qualification of this head.
Next-action owner: maintainer plotarmordev. Complete normal current-main integration while retaining current public defaults, obtain exact-head cumulative source/CPU clearance, and schedule the predeclared supported protocol/resource window. #80 live opt-in thresholds and #180 long-prefill/active-decode contention, separate correctness and fairness/latency/throughput admission remain unqualified. The prospective study must retain failed/partial captures and unchanged controls; the earlier maintainer wrapper HTTP 404 produced zero measured waves and is not a scheduler regression or a passed gate. Main owns wrapper readiness and protocol scheduling, not the author.
No unqualified TP=2
CHUNK=0production default may land through #149 or #180. No immediate author rebase or new benchmark is requested.merge-holdremains and #180 remains draft; this is a COMMENT source review, not approval or merge authorization.Prior PR description and historical proposal — preserved; superseded by the current scope above
Description and Motivation
With
GLM53_MIXED_PREFILL_CHUNK=skip, an agent processing a new prompt can receive zero prefill tokens for the entire time another agent generates. This also affects requests already admitted to the running list. A client can exhaust its idle timeout despite available sequence slots and KV memory.Default the launcher, example configuration, and scheduler overlay to stock chunked prefill (
0) so new prompts can use the remaining step budget during generation. Preserve explicitskip/-1and positive caps. Reapplying the overlay also upgrades the known legacy default in existing images. The launcher propagates the same setting to both TP=2 ranks.This favors concurrent prompt progress and can reduce an individual stream's decode throughput during mixed work. The docs explain that tradeoff, migration of existing
.envfiles, the existing--long-prefill-token-thresholdoption for sharing a small batch budget, and the difference between a per-request context limit and total resident KV capacity.Add CPU regressions and a single live canary that requires a short request to answer before a long response finishes. The canary checks completed streams, the expected short answer, and overlapping content timestamps; serialized HTTP 200 responses do not pass.
Related Issues
Related to #6. The mixed-prefill throughput tradeoff remains available as explicit decode protection; this change addresses the starvation caused by making that policy the default.
Testing
Validated the exact published head,
14b6a9f6d4b6256ef4a51dade72581c4cf4a3175:bash -npasses separately forstart.sh,stop.sh, anddownload.sh; changed Python files compile;git diff --checkpasses.Live runtime validation used two DGX Sparks at clean upstream commit
f906ee990596486e10ddbe381efa6f0e496f77e3, built with the original Dockerfile into imagesha256:232573f48556c301b516ebc86a651501ed4f5c69aa72ef957168ff5a6ecd4fdf. The runtime applies this PR's concurrent-prefill policy through explicit configuration; the published PR source is covered by the regressions above.The recipe uses
GLM53_MIXED_PREFILL_CHUNK=0,LONG_PREFILL_TOKEN_THRESHOLD=512, a 1,024-token batch budget, a 1,000,000-token per-request limit, four sequence slots, GPU memory utilization 0.87, FP8 KV, DFlash k=7 with draft TP=2, and the upstream FAT/grouped/temp-row/rightsize performance settings. The engine reports 2,070,680 total KV-cache tokens. A separate LiteLLM stream-lifetime backport is installed, and GLM-route, proxy, and Hermes stale/read timeouts are 1,800 seconds.OKin 9.013 seconds, with first content at 8.837 seconds. B answered 13.014 seconds before A's final content.OKin 1.455 seconds while the large prompt was still processing.These are functional canaries and an operational endurance qualification of the whole deployed recipe, not a controlled throughput comparison or a guarantee of bounded latency under arbitrary load. Hermes normally compresses around 500K tokens; the separate capacity canary tests near-1M API requests.
Checklist:
Update README.md).