Skip to content

Commit 988a54e

Browse files
committed
T1.32e: the contract suite within CI's memory; a hang fails fast
CI's contract-python (ref-pandas3) job (run 38508814868 at 77bdc12) went silent at 83% and was killed after 14 minutes. Reproduced on Linux (python:3.12, the cell's pins, CI's command, two workers as on CI's runner, about 8 GB): test_t132e_time_feature_family.py alone needed about 13 GB. Its module fixture held both of the family's runs parsed (600 MB of observations, about 6.5 GB of Python objects) while a runner subprocess made the second (3.5 GB), and capture_check.py, run on the same files, parsed both again (6.6 GB). Workers were killed mid-file, and xdist then waited on them until the job was killed. On macOS, with more memory, the suite passed. - capture_check.py reads the two files one pair of observations at a time (check_files, by each line's offset), giving the same problems and summary lines in the same order; check() over lists is unchanged. Peak on the family's runs: 6.6 GB down to 239 MB. - The family test's fixture makes both runs before it parses the captured one, and never parses the capture-off run, which only the capture check reads, from its file; no assertion changes. - contract/tests/conftest.py: under --dist loadfile, the two files whose module fixtures hold a family run parsed (the time and feature family and the multicategorical vectors, about 3.5 GB and 4 GB) share one worker, one after the other, so they are never in memory together; every other file goes to a worker of its own as before. No test is dropped (an --ignore would be, and l0's skips check refuses it). - ci.yml: pytest-timeout fails a test past 10 minutes with its stack, and --max-worker-restart=0 ends the run when a worker dies instead of waiting on it. Measured on Linux, two workers: 2,503 passed in 8 minutes, the machine's memory in use at most 5.5 GB (7.8 GB before the fixture's order, about 13 GB needed before any of this). Signed-off-by: Adi Wadaskar <awadaskar@nvidia.com>
1 parent 4372334 commit 988a54e

4 files changed

Lines changed: 182 additions & 42 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -651,14 +651,21 @@ jobs:
651651
run: |
652652
printf '%s\n' $CELL_PINS duckdb==1.5.6 > "$RUNNER_TEMP/cell.txt"
653653
pip install --upgrade pip
654-
pip install -c "$RUNNER_TEMP/cell.txt" $CELL_PINS duckdb==1.5.6 pyyaml jsonschema pytest pytest-xdist \
654+
pip install -c "$RUNNER_TEMP/cell.txt" $CELL_PINS duckdb==1.5.6 pyyaml jsonschema pytest pytest-xdist pytest-timeout \
655655
./packages/structured-data-models-api ./packages/structured-data-models-pql \
656656
'./packages/structured-data-models-connectors[duckdb]' \
657657
./packages/structured-data-models-relational-engine "$(ls native/*.whl)[test]"
658658
# In parallel, each test file on one worker so its module fixtures are
659-
# built once; the results match a serial run's, test by test.
659+
# built once; the results match a serial run's, test by test. The
660+
# files whose module fixtures hold a family run parsed share one
661+
# worker, one after the other (contract/tests/conftest.py), so this
662+
# runner's memory holds one at a time. A test past 10 minutes fails
663+
# with its stack, and a worker that dies ends the run rather than
664+
# leaving xdist waiting on it.
660665
- name: The runner's controls and the route tests
661-
run: python -m pytest contract/tests -q -n auto --dist loadfile
666+
run: |
667+
python -m pytest contract/tests -q -n auto --dist loadfile \
668+
--timeout=600 --max-worker-restart=0
662669
# As in rust-test, whether the gate runs fails closed (T1.18); off
663670
# the phase branch, the candidate's own runners and comparator run.
664671
# Every cell gates, the floor's too (T1.18's own run alone reported

‎contract/runners/capture_check.py‎

Lines changed: 107 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import json
4242
import re
4343
import sys
44+
from collections.abc import Iterable, Iterator
4445
from pathlib import Path
4546
from typing import Any
4647

@@ -341,8 +342,7 @@ def _first_difference(on: Any, off: Any, path: str) -> str | None:
341342
return None if on == off else f'{path}: differs'
342343

343344

344-
def _cases(run: list[dict[str, Any]], name: str) -> list[str]:
345-
ids = [item['case'] for item in run]
345+
def _cases(ids: list[str], name: str) -> list[str]:
346346
problems = []
347347
if not ids:
348348
problems.append(f'the {name} run holds no case')
@@ -352,24 +352,30 @@ def _cases(run: list[dict[str, Any]], name: str) -> list[str]:
352352
return problems
353353

354354

355-
def check(
356-
on: list[dict[str, Any]],
357-
off: list[dict[str, Any]],
358-
required: tuple[str, ...] | list[str] = (),
355+
#: One captured observation and its capture-off twin (``None`` when the
356+
#: capture-off run lacks the case).
357+
Pair = tuple[dict[str, Any], dict[str, Any] | None]
358+
359+
360+
def _check(
361+
on_ids: list[str],
362+
off_ids: list[str],
363+
required: tuple[str, ...] | list[str],
364+
pairs: Iterable[Pair],
359365
) -> list[str]:
360-
r"""Every problem with the capture over two runs of the same cases,
361-
each case in ``required`` among them."""
362-
problems = _cases(on, 'captured') + _cases(off, 'uncaptured')
363-
by_case = {item['case']: item for item in off}
364-
if [item['case'] for item in on] != [item['case'] for item in off]:
366+
r""":func:`check` over the two runs' case ids and their observations
367+
in pairs, taken one pair at a time, so a caller reading files holds
368+
one pair in memory, never a whole run."""
369+
problems = _cases(on_ids, 'captured') + _cases(off_ids, 'uncaptured')
370+
if on_ids != off_ids:
365371
problems.append('the two runs hold different cases')
372+
on_set, off_set = set(on_ids), set(off_ids)
366373
for case in required:
367-
if case not in by_case or case not in {i['case'] for i in on}:
374+
if case not in off_set or case not in on_set:
368375
problems.append(f'{case}: required, and not in both runs')
369-
for item in on:
376+
for item, other in pairs:
370377
case = item['case']
371378
problems.extend(f'{case}: {p}' for p in complete(item))
372-
other = by_case.get(case)
373379
if other is None:
374380
continue
375381
if other['driver']['capture'] is not None:
@@ -382,12 +388,88 @@ def check(
382388
return problems
383389

384390

385-
def _read(path: Path) -> list[dict[str, Any]]:
386-
return [
387-
json.loads(line)
388-
for line in path.read_text(encoding='utf-8').splitlines()
389-
if line
390-
]
391+
def check(
392+
on: list[dict[str, Any]],
393+
off: list[dict[str, Any]],
394+
required: tuple[str, ...] | list[str] = (),
395+
) -> list[str]:
396+
r"""Every problem with the capture over two runs of the same cases,
397+
each case in ``required`` among them."""
398+
by_case = {item['case']: item for item in off}
399+
return _check(
400+
[item['case'] for item in on],
401+
[item['case'] for item in off],
402+
required,
403+
((item, by_case.get(item['case'])) for item in on),
404+
)
405+
406+
407+
def _lines(path: Path) -> Iterator[tuple[int, bytes]]:
408+
r"""Each non-empty line of ``path`` with the offset it starts at."""
409+
with path.open('rb') as handle:
410+
while True:
411+
offset = handle.tell()
412+
line = handle.readline()
413+
if not line:
414+
return
415+
if line.strip(b'\r\n'):
416+
yield offset, line
417+
418+
419+
def _index(path: Path) -> list[tuple[str, int]]:
420+
r"""Each observation's case id and the offset of its line, in file
421+
order; one line is parsed at a time."""
422+
return [(json.loads(line)['case'], offset) for offset, line in _lines(path)]
423+
424+
425+
def _at(path: Path, offset: int) -> dict[str, Any]:
426+
with path.open('rb') as handle:
427+
handle.seek(offset)
428+
loaded: dict[str, Any] = json.loads(handle.readline())
429+
return loaded
430+
431+
432+
def _summary(item: dict[str, Any]) -> str | None:
433+
capture = item['driver']['capture']
434+
logged = item['driver']['query_log']
435+
if capture is None:
436+
return None
437+
return (
438+
f'{item["case"]}: {len(capture["statements"])} statements and '
439+
f'{len(capture["bindings"])} bindings captured on '
440+
f'{capture["cursors"]} derived cursors; query log '
441+
f'{logged["statements"]} and {logged["bindings"]}; '
442+
f'{logged["compared"]} statement texts compared with it'
443+
)
444+
445+
446+
def check_files(
447+
on: Path, off: Path, required: tuple[str, ...] | list[str] = ()
448+
) -> tuple[list[str], list[str]]:
449+
r""":func:`check` over two observation files, and each captured
450+
case's summary line, reading one pair of observations at a time: a
451+
family's runs reach hundreds of megabytes, which parsed whole took
452+
gigabytes (T1.32e's time and feature family: 6.6 GB on Linux)."""
453+
on_index, off_index = _index(on), _index(off)
454+
off_at = dict(off_index)
455+
summaries: list[str] = []
456+
457+
def pairs() -> Iterator[Pair]:
458+
for case, offset in on_index:
459+
item = _at(on, offset)
460+
summary = _summary(item)
461+
if summary is not None:
462+
summaries.append(summary)
463+
where = off_at.get(case)
464+
yield item, None if where is None else _at(off, where)
465+
466+
problems = _check(
467+
[case for case, _ in on_index],
468+
[case for case, _ in off_index],
469+
required,
470+
pairs(),
471+
)
472+
return problems, summaries
391473

392474

393475
def main(argv: list[str] | None = None) -> int:
@@ -396,22 +478,13 @@ def main(argv: list[str] | None = None) -> int:
396478
parser.add_argument('--off', type=Path, required=True)
397479
parser.add_argument('--require', action='append', default=[])
398480
arguments = parser.parse_args(argv)
399-
on, off = _read(arguments.on), _read(arguments.off)
400-
problems = check(on, off, arguments.require)
481+
problems, summaries = check_files(
482+
arguments.on, arguments.off, arguments.require
483+
)
401484
for problem in problems:
402485
print(problem, file=sys.stderr)
403-
for item in on:
404-
capture = item['driver']['capture']
405-
logged = item['driver']['query_log']
406-
if capture is None:
407-
continue
408-
print(
409-
f'{item["case"]}: {len(capture["statements"])} statements and '
410-
f'{len(capture["bindings"])} bindings captured on '
411-
f'{capture["cursors"]} derived cursors; query log '
412-
f'{logged["statements"]} and {logged["bindings"]}; '
413-
f'{logged["compared"]} statement texts compared with it'
414-
)
486+
for summary in summaries:
487+
print(summary)
415488
print('capture: ' + ('FAILED' if problems else 'complete and conserving'))
416489
return 1 if problems else 0
417490

‎contract/tests/conftest.py‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.
2+
# All rights reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
r"""How pytest-xdist shares the contract suite out under ``--dist loadfile``.
6+
7+
Each test file goes to one worker, as ``loadfile`` sends it, except the
8+
files in :data:`ONE_AT_A_TIME`, whose module fixtures hold a whole family
9+
run parsed (about 3.5 GB and 4 GB on Linux). They go to one worker together,
10+
which runs them one after the other, so no two of them are in memory at
11+
once: side by side on CI's two workers and about 8 GB they ran the runner
12+
out of memory, and the worker killed mid-file left xdist waiting until the
13+
job was killed. Every test still runs; only which worker runs it changes.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
from pathlib import PurePosixPath
19+
from typing import Any
20+
21+
import pytest
22+
23+
#: The files run on one worker, one after the other.
24+
ONE_AT_A_TIME = frozenset(
25+
{
26+
'test_t132e_multicategorical.py',
27+
'test_t132e_time_feature_family.py',
28+
}
29+
)
30+
31+
#: The scope the files in :data:`ONE_AT_A_TIME` share.
32+
SHARED_SCOPE = 'contract/tests: a family run in memory'
33+
34+
35+
def scope_of(nodeid: str) -> str:
36+
r"""The unit a worker takes: the test's file, as ``loadfile`` has it,
37+
or :data:`SHARED_SCOPE` for a file in :data:`ONE_AT_A_TIME`."""
38+
path = nodeid.split('::', 1)[0]
39+
if PurePosixPath(path).name in ONE_AT_A_TIME:
40+
return SHARED_SCOPE
41+
return path
42+
43+
44+
@pytest.hookimpl(optionalhook=True)
45+
def pytest_xdist_make_scheduler(config: pytest.Config, log: Any) -> Any:
46+
if config.getvalue('dist') != 'loadfile':
47+
return None
48+
from xdist.scheduler import LoadScopeScheduling
49+
50+
class _Scheduling(LoadScopeScheduling): # type: ignore[misc]
51+
def _split_scope(self, nodeid: str) -> str:
52+
return scope_of(nodeid)
53+
54+
return _Scheduling(config, log)

‎contract/tests/test_t132e_time_feature_family.py‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def _forced(tmp: Path) -> Path:
176176
return out
177177

178178

179-
def _run(cases: Path, out: Path, *extra: str) -> list[dict[str, Any]]:
179+
def _execute(cases: Path, out: Path, *extra: str) -> None:
180180
done = subprocess.run(
181181
[
182182
sys.executable,
@@ -194,6 +194,9 @@ def _run(cases: Path, out: Path, *extra: str) -> list[dict[str, Any]]:
194194
cwd=ROOT,
195195
)
196196
assert done.returncode == 0, done.stdout + done.stderr
197+
198+
199+
def _read(out: Path) -> list[dict[str, Any]]:
197200
return [
198201
json.loads(line)
199202
for line in out.read_text(encoding='utf-8').splitlines()
@@ -204,11 +207,14 @@ def _run(cases: Path, out: Path, *extra: str) -> list[dict[str, Any]]:
204207
def runs(tmp_path_factory: pytest.TempPathFactory) -> dict[str, Any]:
205208
tmp = tmp_path_factory.mktemp('t132e2')
206209
cases = _forced(tmp)
207-
on = _run(cases, tmp / 'on.jsonl')
208-
off = _run(cases, tmp / 'off.jsonl', '--no-capture')
210+
# The capture-off run is read from its file by the capture check alone,
211+
# never parsed here, and runs before the captured run is parsed: on
212+
# Linux a run parsed holds about 3.5 GB and the runner making one about
213+
# as much, past CI's runner when they overlap.
214+
_execute(cases, tmp / 'on.jsonl')
215+
_execute(cases, tmp / 'off.jsonl', '--no-capture')
209216
return {
210-
'on': {o['case']: o for o in on},
211-
'off': {o['case']: o for o in off},
217+
'on': {o['case']: o for o in _read(tmp / 'on.jsonl')},
212218
'on_path': tmp / 'on.jsonl',
213219
'off_path': tmp / 'off.jsonl',
214220
'tmp': tmp,

0 commit comments

Comments
 (0)