Refresh realm-server shard weights from every push to main - #6004
Closed
backspace wants to merge 2 commits into
Closed
Refresh realm-server shard weights from every push to main#6004backspace wants to merge 2 commits into
backspace wants to merge 2 commits into
Conversation
CS-12731 (#5966) packs realm-server test files by measured duration, but nothing regenerated the weights it committed: a human had to download a merged junit report and run the generator. A day later twelve of 195 files were unmeasured, five of them added since the weights were seeded. A push-to-main job now downloads the run's own merged report and commits the regenerated file, in the shape the matrix and host refreshes already have: a staleness guard against newer commits on main, reset-and-retry around the push, [skip ci] on the commit, and success-or-failure triggering so a flaky week does not freeze the weights. Realm-server CI has no Percy and no index cache, so a push run and a PR run are the same shape and no run selection is needed. The generator gains the --min-drift-seconds / --shard-count gate the other two have, so run-to-run jitter does not become a commit per push. Unlike those, its prediction packs with the real splitter — shard-test-modules.cjs exports slowestShardSeconds — rather than a copy that has to be kept in step. Measured on two consecutive runs of one branch, jitter moved the predicted slowest shard by 26s and 41s; the day-old committed weights cost 93s to 124s. The threshold is 60s. All three refresh jobs now ignore each other's automated commits in their staleness guards. The host job only ignored its own, so a matrix timings commit landing mid-run would have made it skip. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
tests/index.ts loads a hand-maintained list while the splitter walks the disk, and nothing compared the two. A file in the second and not the first is assigned to a shard that never loads it, so it passes CI by never running. Three were in that position: load-links-batching-test.ts (added in May, never listed), server-endpoints/delegate-session-test.ts (added the day after the suite moved to native Node, still written for the qunit CLI), and shard-assignment-test.ts from #5966 — so the test pinning the sharding invariants had never run in CI. The weights generator listing them as unmeasured was the only signal. List all three, pin the list against the disk from the sharding test, and bring delegate-session-test.ts up to the native runner: a named qunit import Node cannot read from the CJS package, and __filename, which does not exist in ES module scope. Whether the two older files pass against the CI stack is for CI to say; locally they fail only on reaching it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
Author
|
Closing as a duplicate. #6002 (stacked on #5996, CS-12759) was already open for CS-12753 when this was started, and covers everything here. The two small things this had that #6002 lacked — printing the unmeasured-files list before the drift gate so gated runs still report it, and the shard-count cross-reference on the realm-server-test matrix — are being ported onto #6002. |
backspace
deleted the
cs-12753-realm-server-shard-weights-have-no-automated-refresh
branch
September 3, 2026 19:29
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.
Linear: CS-12753
What
packages/realm-server/tests/test-module-timings.jsondrives duration-weighted sharding (#5966) but nothing regenerated it. This adds the automated refresh the matrix and host suites already have, and fixes what finding the stale files turned up.Commit 1 — the refresh
realm-server-shard-timings-updatejob inci.yaml: push-to-main only, runs after the shards finish (success or failure), downloads the run's ownrealm-server-test-report-merged, regenerates behind a drift gate, and commitsUpdate realm-server shard timings [skip ci]with the same staleness guard and reset-and-retry push as its siblings. A main push run is representative here: realm-server CI has no Percy and no index cache, so no run selection is needed (unlike CS-12582 for host).generate-test-module-timings.mjsgains--min-drift-seconds/--shard-count. Unlike the matrix and host generators, the prediction packs with the real splitter (shard-test-modules.cjsnow exportsslowestShardSeconds) instead of a mirrored copy that has to be kept in step.Commit 2 — files the shards were packing that never ran
tests/index.tsloads a hand-maintained list; the splitter walks the disk. Three files were on disk but not in the list, so they were assigned to shards that never loaded them and passed CI by never running:load-links-batching-test.ts(May),server-endpoints/delegate-session-test.ts(June, still written for the qunit CLI: named qunit import and__filename), andshard-assignment-test.tsfrom #5966 itself. All three are now listed, the sharding test pins the list against the disk, and delegate-session-test is brought up to the native runner. Whether the two older files pass against the CI stack is what this PR's CI run will tell us; locally they fail only on reaching the stack.Threshold, from real reports
Two merged reports from consecutive runs of the #5966 branch, packed six ways with the actual splitter:
60s separates a stale file from jitter, and matches the host and matrix jobs.
Found along the way, not fixed here
The
TEST_MODULESfilter intests/index.tsbuilds^(?:file)(?:\s>\s|:), so a module titledbasename | qualifiernever matches.cpu-profiler-affinity-gate-test.tsandprerender-artifact-sink-test.tshave only such modules and run zero tests in CI; second modules likenode-realm-test.ts | file statare skipped too. Both reports confirm it: no suite name contains|. One alternation fixes it but would start running tests that have not run for months, so it wants its own issue.Verification
--shard-count, on--shard-count 0, on a non-numeric threshold, and with no report.shard-assignment-test.tslocally: 9/9, including the two new tests.ember-tsc --noEmit, and yamllint (.github/.yamllint.yml, strict) all clean; actionlint reports nothing in the new job.🤖 Generated with Claude Code