ReviewOS

zig-utils/zig-js

CI: bound and shard the serialized threads corpus gate

#32
Closed chris opened this 9 days ago ·
9 days ago

Parent: #1 / #13

Goal

Make the serialized/GIL PR-249 corpus gate bounded and easier to diagnose so one long threads-gil job cannot hold the entire required CI gate hostage without intermediate signal.

Current evidence

The full green gate for b89b1bd60ae4e5c0ab32980c8982621036caf361 has all sanitizer, fuzzer, docs, unit, audit, and no-GIL shard legs green, but threads-gil is still running alone. The workflow currently runs it as one matrix leg:

- name: threads-gil
  cmd: zig build threads-test

Unlike the TSan no-GIL corpus, this leg is not sharded, and GitHub does not expose useful in-progress logs for the job through gh api .../logs while it is still running.

Sketch

  • Add a bounded timeout for the threads-gil job that is high enough for normal CI variance but low enough to classify a hang promptly.
  • Consider sharding the serialized corpus the same way the TSan no-GIL corpus is sharded, or add progress output/checkpointing from conformance/threads_test.zig so a stuck case is identifiable without waiting for final log archival.
  • Preserve the existing default zig build threads-test local command for developer convenience unless a better replacement is clearly nicer.

Acceptance criteria

  • A slow or hung serialized corpus case reports the exact file or shard that was active.
  • The full CI gate no longer depends on one opaque long-tail threads-gil leg.
  • Required-check names remain stable or are updated deliberately in repository settings/docs.
9 days ago

Pushed c3b4d5d (ci(threads): shard serialized corpus gate) as the first #32 fix.

What changed:

  • added -Dthreads-shard-index / -Dthreads-shard-count build flags for zig build threads-test;
  • taught conformance/threads_test.zig to filter cases by shard and print the active RUN case before executing it;
  • split the CI threads-gil matrix leg into four serialized/GIL shards;
  • documented the shard command in docs/threads/testing.md.

Focused validation:

  • zig build threads-test -Dthreads-case=api/thread-basic.js -Dthreads-shard-index=0 -Dthreads-shard-count=1 --summary none
  • git diff --check

This intentionally preserves the plain local zig build threads-test command while making CI failures/hangs identify a shard and current case instead of one opaque long-tail job.

9 days ago

Pushed e67c93a (ci: bound required gate duration) as a follow-up #32 hardening slice.

The main required-check matrix now has timeout-minutes: 60, and the testing docs call out that a true hang should become an archived, diagnosable failed job instead of an unbounded spinner. This complements c3b4d5d's sharding and RUN <case> progress output.

Validation:

  • git diff --check
9 days ago

Fresh evidence from push run https://github.com/zig-utils/zig-js/actions/runs/29168739848 at 14a1634debeac388fce50162d484075a3c835c5a:

  • 23 jobs passed and 1 skipped.
  • The only non-green legs were unit and tsan-unit.
  • Both were cancelled almost exactly at the 60-minute job timeout after entering zig build test / zig build test -Dtsan=true, with no assertion failure or TSan report in the logs.

Follow-up fix pushed in 28ec38ced0cc0fc42216befe6ee5783407aed4bb:

  • Added a simple custom unit test runner that prints each selected test before running it.
  • Added -Dunit-shard-index / -Dunit-shard-count build options via UNIT_SHARD_INDEX / UNIT_SHARD_COUNT runner env.
  • Split both unit and tsan-unit into 4 CI shards.

Focused local validation:

  • zig fmt build.zig tools/unit_test_runner.zig
  • zig build test -Dunit-shard-index=0 -Dunit-shard-count=64 --summary none
  • zig build test -Dtsan=true -Dunit-shard-index=0 -Dunit-shard-count=128 --summary none
  • zig build test -Dunit-shard-index=0 -Dunit-shard-count=4 --summary none (164/164 selected tests passed)
  • git diff --check

The new CI run for the sharded unit gates is queued/running here: https://github.com/zig-utils/zig-js/actions/runs/29170712091

9 days ago

Follow-up from the first sharded run (28ec38ced0cc0fc42216befe6ee5783407aed4bb): the new runner immediately identified the remaining plain-unit long pole as unit (shard 2/4), specifically Context heap_limit_bytes lets sibling Threads survive one Thread OOM.

Root cause: that fixture used an unbounded allocation loop inside a worker Thread. If the heap-limit recovery path kept making progress on a loaded runner instead of tripping the cap quickly, the test became an hour-long negative proof. That was the silent timeout behavior we saw before sharding.

Fix pushed in b2e7dbe5e9bc2a27a87ff1bad8be8b0be777e46e:

  • Replaced the unbounded object-pressure loop with a bounded string-side-storage pressure loop.
  • Kept the oracle strict: if the worker does not throw the expected frozen OutOfMemoryError, it throws heap limit did not trip before bounded pressure loop ended, so the test fails instead of hanging.

Focused validation:

  • zig build test -Dtest-filter="Context heap_limit_bytes lets sibling Threads survive one Thread OOM" --summary none
  • zig build test -Dunit-shard-index=2 -Dunit-shard-count=4 --summary none
  • zig build test -Dtsan=true -Dtest-filter="Context heap_limit_bytes lets sibling Threads survive one Thread OOM" --summary none
  • git diff --check

Latest CI for this fix is queued/pending here: https://github.com/zig-utils/zig-js/actions/runs/29170980761

9 days ago

Completed by 28ec38ced0cc0fc42216befe6ee5783407aed4bb and b2e7dbe5e9bc2a27a87ff1bad8be8b0be777e46e.

Evidence:

  • The previous full gate at 14a1634debeac388fce50162d484075a3c835c5a timed out/cancelled only in monolithic unit and tsan-unit.
  • unit and tsan-unit are now 4-way sharded with a simple runner that prints per-test progress.
  • The previously hanging unit (shard 2/4) fixture is bounded and fails explicitly if the heap limit does not trip.
  • Fresh full CI is green for b2e7dbe5e9bc2a27a87ff1bad8be8b0be777e46e: https://github.com/zig-utils/zig-js/actions/runs/29170980761

Remaining runtime smell from threads-gil (shard 1/4) is split to #33.

Sign in to comment on this issue.