ci(polars): parallelize the polars lane so the coverage cell stops racing its 10-minute timeout - #1814
Merged
Merged
Conversation
…cing its timeout `test-polars (3.12)` is the only cell that runs the coverage-instrumented pass plus the per-file coverage audit, and it had already been cancelled twice at its 10-minute budget (615s, 611s) with zero test failures — pure budget exhaustion, the same commit passing or failing depending on the runner it drew. Measured on the master run that produced this base commit, the script step alone is 484s of the cell's 501s. The lever that needs no workflow edit: pytest-xdist is already in the [test] extra, and test-gfql-core already runs `-n auto` under `--cov`, so coverage+xdist is established here. The main pytest phase now runs `-n auto --maxprocesses 4 --dist load`. Measured (py3.12, polars 1.43.1, pandas 3.0.5, POLARS_COV=1), pinned to 4 CPUs to stand in for a GitHub-hosted ubuntu-latest runner: 325.65s serial -> 109.34s parallel, 2.98x. Verified, not assumed: - pass/skip NODE-ID SETS (not counts) identical serial vs parallel: 2417 node ids, 2404 passed / 13 skipped, across 4-worker, 2-worker, --dist loadfile and 4-CPU-pinned runs - merged coverage is a strict superset of serial (28,480 vs 28,478 covered lines; zero lines lost, zero files dropped; the 2 extra are versioneer fallback lines in _version.py) - the second `--cov-append` invocation still appends into the xdist-produced data file (+1,791 lines, nothing lost) and the result is line-identical to the whole-script run - `bin/coverage_audit.py --profile gfql-polars` emits a byte-identical report (modulo its timestamp) from the parallel data, so `changed-line-coverage` sees no change `--dist load` rather than `loadfile` because one module (test_engine_polars_chain.py) is 69% of the lane's test time, which caps file-level distribution at ~1.4x; no test in this lane depends on execution order or cross-test module state. POLARS_XDIST=0 restores the serial path and POLARS_XDIST_DIST selects another distribution mode if that ever changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
The first commit quoted a local 4-CPU-pinned A/B (326s -> 109s, 2.98x). CI has now run this branch and realizes less: the `test-polars (3.12)` cell goes 501s -> 322s and its script step 484s -> 297s (1.63x), with the five non-coverage cells at 222-318s -> 115-172s. Coverage tracing is per-worker CPU cost that does not parallelize away, and the ~8s serial second phase plus 4x interpreter startup are fixed, so the coverage cell gains less than the plain cells (which see ~2x). The CI figure is the one that decides whether #1805 fits, so it is the one the changelog quotes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
lmeyerov
force-pushed
the
ci/polars-lane-xdist
branch
from
July 28, 2026 03:01
077867b to
1e499f5
Compare
This was referenced Jul 28, 2026
lmeyerov
added a commit
that referenced
this pull request
Jul 28, 2026
Resolves both conflicts and corrects two defects the conflict exposed. bin/test-polars.sh -- take master's version unchanged. This branch adds graphistry/tests/compute/gfql/test_rows_table_named_middle.py and test_rewrite_param_discard.py to POLARS_TEST_FILES, but #1805 (49db91c, this branch's own merge-base) already added both. Keeping the branch side would list each file twice, so the polars lane would run both modules twice -- and that lane's py3.12 cell is the coverage cell, currently 442s against a 600s cap. Master's version also carries #1814's xdist plumbing, which the branch side predates. CHANGELOG.md -- the entry claimed "neither file was in bin/test-polars.sh, so test_rows_table_named_middle.py ... ran in no CI lane at all". That is false at this branch's base: both files are listed there at lines 43 and 50. Rewritten to the residual that is true -- the suites were parametrized over pandas + polars only, so cuDF and polars-gpu never exercised the table guards; and the module-level pytest.importorskip("polars") skipped the whole file in test-gfql-core, so its pandas cases ran only in the polars lane. The measured claims (45 passed / 3 xfailed / 0 skipped on GB10; 24 mutation failures, 6 per engine) are unchanged. Also types engine_skip_reason, per review: the engine -> required-modules map is hoisted to a module-level Mapping[str, Tuple[str, ...]] constant rather than rebuilt inline per call, the signature is (engine: str, smoke: Callable[[], object]) -> Optional[str], and gpu_environment_reason's "str | None" string annotation is normalized to Optional[str] to match (this file is imported under py3.8 lanes). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
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.
The problem
test-polars (3.12)is the only matrix cell that runs the coverage-instrumented pass and the per-file coverage audit. It has already been cancelled twice at its 10-minute budget (615s, 611s) with zero test failures — pure budget exhaustion, and the same commit passes or times out depending on which runner it draws.Measured on the master CI run that produced this PR's base commit (run
30300891340, head233b64c8), the cell is 501s, of which 484s is the singlebin/test-polars.shstep — 97% of the cell. So the lane's runtime is the script's runtime; everything else is noise.timeout-minuteslives in.github/workflows/ci.yml, which cannot be pushed with the current token. This PR raises no budget — it makes the work smaller, entirely insidebin/test-polars.sh.The fix
The main pytest phase now runs under
pytest-xdist:-n auto --maxprocesses 4 --dist load.No dependency changes:
pytest-xdistis already in the[test]extra (setup.py:44), andtest-gfql-core (3.12)already runs-n autounder--covand feeds the samechanged-line-coveragecombine step — so coverage+xdist is an established combination in this repo, not a new bet. It was still verified from scratch here (below), because "established elsewhere" is not evidence about this lane.Worker count, deliberately:
auto, capped at 4.ubuntu-latestis a 4-vCPU runner, soauto(=os.cpu_count()) resolves to 4 there — the right number, discovered rather than hardcoded.autoalso scales down: on a 2-vCPU runner it picks 2, where a fixed-n 4risks being slower than serial. A 2-worker run was measured (below) and is still a 1.67x win, so the small-runner case degrades gracefully instead of regressing.--maxprocesses 4caps the other direction, so a 24-core dev box does not fan out 24 polars processes whose own thread pools then fight each other.POLARS_XDIST=0restores the serial path (used for the A/B below);POLARS_XDIST_DISTpicks another distribution mode.Measured ON CI — this branch's own runs
This is the number that decides the question, and it is smaller than the local speedup. Reported as measured, over two runs of this branch (
30322706850,30323434758) against the master run of the base commit (30300891340). Cell time first,bin/test-polars.shstep time in parentheses.All six cells green in both runs. Runner-to-runner spread is real (it is the same spread that made the timeouts intermittent), so the slowest observation is the one used for the #1805 projection below.
Why the coverage cell gains less than its siblings (~1.6–1.9x vs ~2x), and less than local (2.98x): coverage tracing is per-worker CPU cost that does not parallelize away; the second
--cov-appendphase stays serial (~8s locally, more under CI's coverage tracing); and four interpreter startups + fourimport graphistryare fixed overhead. The local box also has faster cores than a GitHub runner. Quote 1.63–1.89x, not 2.98x.Local A/B (context only, not the CI claim)
py3.12 / polars 1.43.1 / pandas 3.0.5,
POLARS_COV=1, whole-script wall clock:POLARS_XDIST=0), pinned to 4 CPUs-n auto --maxprocesses 4 --dist load, pinned to 4 CPUsPOLARS_XDIST=2, unpinned--dist loadfile, 4 workers, unpinnedWhy
--dist loadand notloadfile: aggregating per-test durations,test_engine_polars_chain.pyis 307.9s of the lane's 447.1s (69%). File-level distribution is bounded by the largest module, soloadfilecannot exceed ~1.4x — measured at 1.36x.Does #1805 now fit inside the budget, with margin? Yes.
#1805's 3.12 cell measured 9m57s = 597s against a 600s timeout — green by 3 seconds. Its script step is therefore ~580s. Applying the CI-measured 1.63x:
That takes #1805 from 3 seconds of headroom to roughly 3.8 minutes. The #1805 row is the only projected number here; every other figure is measured. Even if #1805's added tests parallelize worse than the current set and it only realizes 1.3x, its cell lands near 465s — still inside 600s.
Verification — what was proven, not assumed
1. Coverage + xdist actually work here. Full lane run with
POLARS_COV=1under 4 workers, thencoverage jsonon both data files and a per-file executed-line-set diff against the serial run:graphistry/_version.py(versioneer's git fallback, which workers reach and the parent does not). Nothing undergraphistry/computediffers at all.2. The per-file coverage audit still passes, and agrees.
bin/coverage_audit.py --profile gfql-polarswas run against the serial data file and against each parallel data file. All exit 0, and the emittedgfql-polars-coverage-audit.mdis byte-identical apart from itsGenerated:timestamp line for the 4-worker, 2-worker and 4-CPU-pinned parallel runs. Additionally, replaying whatchanged-line-coverageitself does —coverage combineover the artifact, thencoverage report— gives identical totals from the serial and parallel artifacts (57,249 statements, 28,769 missing, 50%). The audit step also ran green on CI in this PR's own 3.12 cell.3. The second (
--cov-append) invocation still appends correctly. Isolated: ran phase 1 alone under xdist into a fresh data file (26,689 lines), copied it, then ran phase 2 with--cov-appendinto the copy. Result: +1,791 lines added, 0 lines lost, and the final line sets are identical to the full-script parallel run in every file. Phase 2 is deliberately left serial — it is one module and ~8s, so worker startup would eat the gain, andloadfilecould not split it anyway.4. Identical pass/skip/xfail sets — node ids, not counts. Both
-vvstreams (serial and xdist print in different formats) were parsed into{nodeid: outcome}maps, anchored on the progress marker so parametrized ids containing spaces are not truncated. Every parallel configuration gives exactly the same 2417 node ids with the same outcomes (2404 PASSED, 13 SKIPPED):0only-in-serial,0only-in-parallel,0outcome mismatches — for 4-workerload, 2-workerload, 4-workerloadfile, and the 4-CPU-pinned run. The 13 skips are the same cuDF/GPU-gated node ids in every run.5. Order-dependent tests. Audited and none found: the lane's modules carry no
autousefixtures, no module-level mutable state, no global cache/counter assertions, and no environment or filesystem mutation. The onlyscope="module"fixtures (index/test_index.py) are pure deterministic data builders (seededdefault_rng) — worker-local recomputation is correct, just repeated. The empirical check backs the audit: four different distributions/worker counts, including one that keeps modules whole (loadfile) and two that split them (load), all produce identical outcomes, and all six CI cells passed in both CI runs.POLARS_XDIST_DIST=loadfile(orloadscope) is the one-env-var escape hatch if a future test does become order-dependent.6. Worker count — see "Worker count, deliberately" above;
automatches the 4-vCPU runner,--maxprocessescaps oversubscription, and the 2-worker measurement (1.67x) shows the small-runner case is still a win rather than a regression.Relationship to #1805
Mechanically separate on purpose. #1805 also touches
bin/test-polars.sh, but only inside thePOLARS_TEST_FILESarray (widening the file list) and in the comment directly above it. This PR touches only the block after the array's closing paren, plusCHANGELOG.md. Non-overlapping hunks with unchanged lines between them, so either can rebase onto the other cleanly.Also checked for a semantic collision: #1805's
test_polars_lane_completeness.pyparses the lane script withre.search(r"^POLARS_TEST_FILES=\((.*?)^\)", ...). That anchored, non-greedy match is unaffected by the newXDIST_ARGS=( ... )array, which is declared after it.Suggested landing order: this PR, then #1805. That inverts the ordering constraint recorded for #1805 — it no longer has to wait on a
ci.ymljob split that only a human withworkflowscope can push.What this does not do
It does not remove the argument for the
ci.ymljob split (a sharedtimeout-minutesacross six cells with very different work is still a blunt instrument, and 3.12 doing strictly more work than its siblings is still a design smell). It removes the urgency: the lane goes from 84–99% of its budget to ~54%, so the split can land on its own schedule rather than as a prerequisite.🤖 Generated with Claude Code
https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB