Skip to content

ci(unit-tests): widen the apt bounds so a slow mirror still installs tshark - #1309

Merged
JarryShaw merged 1 commit into
mainfrom
ci/1306-apt-bounds
Oct 7, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci/1306-apt-bounds

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • ci — workflows or build tooling

Description of your pull request and other information

Closes #1306. A ~0.5 MB/s mirror overran apt-get update's 45s bound, then apt-get install tshark's 90s bound mid-way through libwireshark17t64 (23.7 MB), in job 112943258387.

update install worst case (2 attempts) step cap job cap
before 45s 90s 270s 5 min 30 / 45
after 60s 180s 480s 10 min (120s spare) 30 / 45

Both apt steps (engine-tests and pypcap-parity) change together. The rationale comments are rewritten with the new arithmetic and keep their line counts, so the unit-tests.yml line citations in docs/ still resolve. test_unit_tests_queue.py now pins the bounds [60, 180] and the 10-minute cap.

@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on a3c42f2f1: NEEDS CHANGES (Sonnet reviewed; the author ran on Opus). The code, the arithmetic, the doc line citations and the pinned test are all sound. Two statements in the rewritten comment are not.

  • Must fix: the comment at unit-tests.yml:459-467 says "180s for install (~90 MB at 0.5 MB/s)". The tshark install is 26.5–27.9 MB (Need to get 26.5 MB of archives; a passing PyShark job fetched it in 78 s, and parity fetched 27.9 MB in 56 s). Restate the sizing on the real payload: about 27 MB, tolerated down to roughly 0.15 MB/s.
  • Should fix: "slowest of 120 was 34s" now stands undated. That figure is from 2026-10-04/05 (7958b6425, ci: cut queue pressure and diagnose slow legs #1052/ci: cut queue pressure in unit-tests (#1052) #1055). Over the last 2 days the step reached 167 s, and about 2% of samples ran over 120 s. Date the figure or drop it.
  • Bounds confirmed against 451 recent samples: PyShark p99 was 135 s and max 160 s; parity p99 134 s and max 167 s. The new 180 s install bound and 10-minute step cap fit that tail.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Round-2 verdict on e2b4ed347: NEEDS CHANGES. The two round-1 blockers are fixed — the "~90 MB" figure is gone and the arithmetic checks out (2 × (60 + 180) = 480, 120s under the 600s cap). But the replacement distribution is wrong, and it is going into a permanent comment, so I re-derived it myself.

My measurement. Every Install this engine's system packages and Install libpcap headers, a C toolchain, and tshark step across all 335 unit-tests.yml runs in the window, by started_at/completed_at: 2,083 observations (2,071 success, 11 failure, 1 cancelled), 1,324 of them non-zero, over 2026-10-06T02:16Z → 2026-10-07T18:50Z.

n p95 p99 max
both steps 1324 32 143 210
tshark step 405 80 167 193
engine step 919 28 103 210

So the comment's "451 samples … max 160-167s, p99 ~135s" understates the sample count about threefold and the max by 43s. 167 is the tshark step's p99, not anyone's max — I think the two got conflated. The 11 failures sit at 91, 139, 143, 155, 187, 190, 191, 192, 192, 193 and 194 seconds, which is the number worth citing: that cluster is what the old bounds were cutting off.

The change: replace the figures with measured ones and say what they measure (whole-step wall time, both attempts included — not per-attempt, which step timings cannot give). The bounds, the caps and the test are fine as they stand.

Note the window is ~1.7 days, not 2; created>=2026-10-05 returned nothing before 02:16Z on the 6th.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Correction to my own round-2 figures — they were a lower bound, not a distribution. I owe this before the next revision goes up.

My scan paginated actions/workflows/unit-tests.yml/runs page by page while new runs were being created, and that endpoint re-sorts by creation descending on every request, so entries slid across page boundaries and some runs were never visited. A worker caught it by measuring job 112943258387's install step at 141s, a value absent from my list.

Re-running with a closed window and --paginate, then taking the union of both passes, deduplicated by (job id, step name):

pass unique job-steps max
first 5,236 210s
second 2,631 192s
union 5,296 469s

So the honest statement is that the sample count grew from 2,083 to 5,296 between passes and the max from 210s to 469s, and I cannot show that even the union is complete. Union figures, over 2026-10-05T00:34Z → 2026-10-07T19:15Z: 3,119 non-zero, p50 13s, p90 21s, p95 26s, p99 105s, max 469s; the tshark step alone n=597, p95 56s, p99 191s, max 309s; the engine step n=2,522, p95 23s, p99 77s, max 469s. 14 failures, at 91, 139, 143, 149, 155, 187, 190, 191, 191, 192, 192, 192, 193 and 194 seconds.

What the comment should say. Do not cite a percentile distribution at all — I cannot stand behind one. Cite the per-job measured step times you derived yourself (141s for 112943258387, 155s for 112948578234), which are exact and checkable, and the failure band, with a plainly bounded phrase such as "observed across the runs sampled" rather than "in N samples". A comment that overstates its own evidence is the thing we have been fixing all afternoon.

One substantive point the 469s raises. If a single step can reach 469s, the 2 × (60 + 180) = 480s worst case has about 11s of headroom against the longest step actually observed — thinner than the comment's "120s under the cap" framing suggests, because that 120s is measured against the cap, not against observed behaviour. Worth a sentence, since it is the argument someone will re-examine the next time this step times out.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 7, 2026
…tshark

A ~0.5 MB/s Azure mirror overran the 45s update and 90s install bounds
(job 112943258387). Raise them to 60s/180s and both apt step caps from
5 to 10 minutes, so two worst-case attempts (480s) fit with 120s spare.
Comment line counts are unchanged so doc line citations still resolve.

Closes #1306
@JarryShaw

Copy link
Copy Markdown
Owner Author

The 469s I cited is real but does not apply here. Job 111955998031 (Engines Python 3.10 (PyShark), run 37367471177) spent 469s in Install this engine's system packages on 2026-10-05 at 2d62d9db8. At that sha the step had a 20-minute cap and no per-attempt timeout(1), so it describes the old regime rather than the 60s/180s bounds this PR sets. It is also outside the 2026-10-06..07 window the comment samples. The author was right not to write a figure they could not reproduce, and the comment rightly rests on 210s.

7934b7732 is the head for review: percentiles removed, the per-job times (141s, 155s) cited with timestamps, and the failure band described as an observation.

@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Round-3 verdict on 7934b7732: GOOD TO GO (Sonnet cross-review plus my own checks; author Opus).

  • Per-job times confirmed against the API: job 112943258387's Install this engine's system packages ran 18:17:51Z→18:20:12Z (141s, failure); job 112948578234's tshark step ran 18:31:41Z→18:34:16Z (155s, failure).
  • The phrasing is now an observation, not a statistic: no sample counts, no percentiles, no "slowest was".
  • The arithmetic holds: 2 × (60 + 180) = 480, under the 600s cap.
  • Bounds: timeout 60/timeout 180 and timeout-minutes: 10 are on both steps (unit-tests.yml:477,482-483 and :744,749-750), and no other workflow still has a 45/90 apt bound.
  • Tests and docs: the test pins [60, 180] and 10. Both comment blocks keep their line counts, and tests/project gives 389 passed, 1 skipped.

The reviewer could not confirm two sentences, so I checked them myself:

  • "27.9 MB" and the Ign: retries come straight from job 112948578234's log: Need to get 27.9 MB of archives at 18:32:46, then Ign: from azure.archive.ubuntu.com at 18:33:16 — the 30s cadence.
  • "Steps past 200s are on record" holds within the window: job 112947818496 (Engines (PyShark), run 37665375078) spent 210s in the install step, 18:29:01Z→18:32:31Z, and succeeded. Citing that job id in the comment would make the sentence checkable. It is a nit, not a blocker.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Merging this first would stop the red legs on the other PRs. The #1306 apt stall has now failed the PyPCAP/PyPCAPFile parity or Engines (PyPCAP) install step on #1307, #1310, #1311, #1336 and #1339. Each failure was one of the old bounds being hit twice: job 112991123073 (#1339) ran 20:22:35→20:25:42Z, and job 112984058084 (#1336) hit apt-get update at 45s on both attempts. Each failed run means one re-run per PR, and every merge to main restarts them all under the strict ruleset.

@JarryShaw
JarryShaw merged commit 08c874e into main Oct 7, 2026
36 checks passed
@JarryShaw
JarryShaw deleted the ci/1306-apt-bounds branch October 7, 2026 20:41
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 7, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

ci(unit-tests): tshark apt install exceeds its 90s per-attempt bound on a slow mirror

1 participant