diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 51c02403..e3f848ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2648,6 +2648,7 @@ jobs: names: ${{ steps.trace-cases.outputs.names }} verify-matrix: ${{ steps.trace-cases.outputs.verify-matrix }} split-matrix: ${{ steps.trace-cases.outputs.split-matrix }} + split-spawn-matrix: ${{ steps.trace-cases.outputs.split-spawn-matrix }} steps: - name: Checkout repo uses: actions/checkout@v6 @@ -2699,6 +2700,22 @@ jobs: exit 1 fi echo "split-matrix=${SPLIT_MATRIX}" >> "$GITHUB_OUTPUT" + # On the disaggregation branches, the same subset without the inproc rows the + # inproc-acceptance retrace rows replay on the same library, which retrace-split uses when + # it reuses build-linux's library. OpenRA keeps its inproc rows: they carry the + # wire-volume evidence. Its self-test proves every dropped row is an acceptance row. It is + # only a saving, so it never fails this job: on a failed self-test or an empty subset it + # is not written, and retrace-split takes the full subset, as it does elsewhere. + if [ "${MOBILEGL_CI_DISAGGREGATED}" = "ON" ]; then + if python3 tools/trace_replay/trace_cases.py --self-test-split-matrix \ + && SPLIT_SPAWN_MATRIX=$(python3 tools/trace_replay/trace_cases.py --ci --format github-split-matrix \ + --transports spawn --keep-inproc OpenRA) \ + && [ "$(printf '%s' "${SPLIT_SPAWN_MATRIX}" | python3 -c 'import json,sys; print(len(json.load(sys.stdin)["include"]))' || echo 0)" -ge 1 ]; then + echo "split-spawn-matrix=${SPLIT_SPAWN_MATRIX}" >> "$GITHUB_OUTPUT" + else + echo "::warning::the spawn-only split retrace subset failed its self-test or is empty, so retrace-split replays the full split subset" + fi + fi trace-fixtures: name: trace fixture (${{ matrix.case }}) @@ -2873,6 +2890,11 @@ jobs: && [ '${{ matrix.case }}' = 'improved-transparency-minecraft-26.3' ]; then export MOBILEGL_MAGMA_DISABLE_BLENDED_DEPTH_WRITE=1 fi + # The inproc rows stand in for retrace-split's dropped inproc rows, so they prove the + # configuration they run with from the client's own log. + if [ "${MOBILEGL_TRANSPORT}" = inproc ]; then + export MGTRACE_EXPECT_IPC='strict=1 role-split-state=1 run-ahead=1' + fi ctest -V --no-tests=error -R '^MobileGLTraceReplay\.${{ matrix.case }}\.${{ matrix.backend }}$' --output-junit "${RUNNER_TEMP}/retrace.xml" # A split row's transport assertions ran on this leg and found their evidence. if [ "${MOBILEGL_TRANSPORT}" != monolith ]; then @@ -2881,6 +2903,33 @@ jobs: --transport "${MOBILEGL_TRANSPORT}" --case '${{ matrix.case }}' --backend '${{ matrix.backend }}' fi + # The refusal census, recorded rather than gated. run_trace_case.cmake already REDS the case + # on any Fatal{, so reaching here means the count is zero - but the number and the distinct + # slot names are what MEASUREMENTS wants from every split run, and reading them out of the + # library's own log is the only way to get them (ctest -V's transcript is a false zero). + - name: Refusal census from the library log + if: ${{ always() && matrix.transport == 'inproc' }} + working-directory: build-retrace/tools/trace_replay + run: | + # BOTH ROLES, because under spawn every applier refusal is raised in the SERVER's + # process and a census reading only the client's file reports a confident zero for the + # half it cannot see. + log="${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.client.log" + server_log="${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.server.log" + cat "${{ matrix.case }}/${{ matrix.backend }}/output/split-evidence.txt" 2>/dev/null || echo "no split-evidence.txt" + if [ ! -f "${log}" ]; then + echo "no ${log} - the replay wrote no library log" + exit 0 + fi + census_files="${log}" + if [ -f "${server_log}" ]; then census_files="${census_files} ${server_log}"; fi + echo "census over: ${census_files}" + echo "Fatal{ lines: $(cat ${census_files} | grep -c 'Fatal{' || true)" + cat ${census_files} | grep -o 'Fatal{[A-Za-z]*, "[^"]*"' | sort | uniq -c | sort -rn | head -20 || true + # The transport line is the CLIENT's: ConfigLoader resolves it there, and under spawn + # the server states its own separately. + grep -m1 "MOBILEGL_TRANSPORT=" "${log}" || echo "no transport line in ${log}" + - name: Annotate failed entries if: failure() run: python3 scripts/ci/junit_annotate.py --excerpts 1 "${RUNNER_TEMP}" @@ -3185,7 +3234,10 @@ jobs: strategy: fail-fast: false max-parallel: 8 - matrix: ${{ fromJSON(needs.trace-cases.outputs.split-matrix) }} + # With build-linux reused, the inproc rows (except OpenRA's) are replayed by the retrace job's + # inproc-acceptance rows on the same library, strict and with the dual block and run-ahead + # on; the spawn rows below prove per leg that those are also the defaults they run with. + matrix: ${{ fromJSON(needs.build-linux-split.outputs.reuse-primary == 'true' && needs.trace-cases.outputs.split-spawn-matrix || needs.trace-cases.outputs.split-matrix) }} steps: - name: Set swap space @@ -3350,6 +3402,10 @@ jobs: export MGTRACE_WIRE_EVIDENCE=1 wire=(--require-wire) fi + if [ "${SPLIT_REUSE}" = "true" ]; then + export MGTRACE_EXPECT_IPC='strict=0 role-split-state=1 run-ahead=1' + export MGTRACE_EXPECT_IPC_REASON="retrace-split dropped its inproc rows because the inproc-acceptance retrace rows replay the same library with strict=1 role-split-state=1 run-ahead=1, which differs from this leg's defaults only in strict while these hold. A default changed: restore the inproc rows (the trace-cases job's split-spawn-matrix without --transports spawn) or update the expectation." + fi ctest -V --no-tests=error --timeout 10800 \ -R '^MobileGLTraceReplay\.${{ matrix.case }}\.${{ matrix.backend }}$' # The transport assertions ran on this leg and found their evidence. diff --git a/docs/ci/ci-cleanup-log.md b/docs/ci/ci-cleanup-log.md index fbbb3ffc..10f7a72b 100644 --- a/docs/ci/ci-cleanup-log.md +++ b/docs/ci/ci-cleanup-log.md @@ -52,6 +52,7 @@ them wrong. | 2026-10-17 | the integration-gpu entries that pin their own `MOBILEGL_TRANSPORT` (804 inproc, 1446 spawn, 131 monolith) and the 12 `TcpServer` fixture entries only they need, in the inproc-acceptance leg (both passes) | Test / `integration` (inproc-acceptance) | their pin overrides the only variable the two legs differ in, so they ran identically in both (173 paired runs; the one one-sided red was a port bind fixed by cde6512b) | the monolith-control leg, which runs them on every branch; `transport_pinned.py` proves per run that the trimmed selection lost exactly that population (else the leg runs everything) and that the monolith-control leg ran every one; skip census: the monolith-control lanes list their own names and the inproc-acceptance lanes drop the pinned ones | | 2026-10-17 | `Strict split lane` on the disaggregation branches (dev keeps it) | Test / `integration-split-strict` | with the role-split state defaulting on, its environment equals the dual-block census lane's, whose entries are a superset (31 paired runs after the flip, never red only in the strict lane) | the dual-block step's first run leaves the knob at its default, proves per entry from its client log that strict, the dual block and run-ahead resolved on (`ipc-config`), writes the integration-split part of its JUnit as `strict-split.xml` (every name exactly once, `junit_tally.py --select-from`), and makes the strict lane's checks on it (hard green, two-sided marker census, StrictArming ran); a default that changed reruns the census with the knob set | | 2026-10-17 | `Magma split lane (strict, dual-block)` on the disaggregation branches when the dual-block step proved its defaults | Test / `integration-split-strict` | the dual-block census run covers the `integration-magma-split` label in the same environment | the dual-block census run | +| 2026-10-17 | the 75 retrace-split inproc rows other than OpenRA's, when `build-linux-split` reuses `build-linux` | Test / `retrace-split` | the retrace job's inproc-acceptance rows replay the same cases on the same library, strict and with the dual block and run-ahead on; no one-sided pair since 10-05 | the inproc-acceptance rows, which now prove per leg from the client log that they run with strict=1, role-split-state=1 and run-ahead=1 (`MGTRACE_EXPECT_IPC`) and keep a refusal census; every remaining retrace-split leg proves its defaults (strict=0, role-split-state=1, run-ahead=1) and a changed default reds every leg with the rows to restore; `trace_cases.py --self-test-split-matrix` (disaggregation branches) proves every dropped row is an acceptance row; OpenRA's inproc rows stay for the wire evidence | ## Kept after investigation diff --git a/tools/trace_replay/run_trace_case.cmake b/tools/trace_replay/run_trace_case.cmake index 6536b798..8a999759 100644 --- a/tools/trace_replay/run_trace_case.cmake +++ b/tools/trace_replay/run_trace_case.cmake @@ -85,6 +85,15 @@ if(NOT "$ENV{MGTRACE_WIRE_EVIDENCE}" STREQUAL "" AND NOT "$ENV{MGTRACE_WIRE_EVID set(ENV{MOBILEGL_PIPE_STATS} 1) set(ENV{MOBILEGL_PIPE_STATS_PERIOD} 1) endif() +# MGTRACE_EXPECT_IPC, another harness name, lists key=value tokens the client's one-line IPC +# configuration must carry (checked in the transport block below); a leg that leaves knobs at their +# defaults uses it to prove what those defaults resolved to. A monolith run has no such line. +if(NOT "$ENV{MGTRACE_EXPECT_IPC}" STREQUAL "" + AND ("$ENV{MOBILEGL_TRANSPORT}" STREQUAL "" OR "$ENV{MOBILEGL_TRANSPORT}" STREQUAL "monolith")) + message(FATAL_ERROR + "MGTRACE_EXPECT_IPC is set for ${TRACE_CASE_NAME} ${TRACE_BACKEND} but MOBILEGL_TRANSPORT is " + "'$ENV{MOBILEGL_TRANSPORT}': a monolith run logs no IPC configuration to check.") +endif() if(EXISTS "${TRACE_OUTPUT_DIR}") file(REMOVE_RECURSE "${TRACE_OUTPUT_DIR}") @@ -497,6 +506,37 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " "(magma-b3.md ยง2). The clamp in VulkanRenderer::OnSubmitsCompletedUpTo is what makes " "this unreachable; a line here means it was removed or bypassed.") endif() + # The IPC configuration the client resolved, against MGTRACE_EXPECT_IPC's tokens. Each token + # must appear whole on the last "Config: IPC" line, so run-ahead=1 never matches + # run-ahead=10. MGTRACE_EXPECT_IPC_REASON, when set, says why the leg relies on them. + set(split_ipc_expected "-") + if(NOT "$ENV{MGTRACE_EXPECT_IPC}" STREQUAL "") + set(split_ipc_expected "$ENV{MGTRACE_EXPECT_IPC}") + file(STRINGS "${mobilegl_log}" split_ipc_lines REGEX "Config: IPC ") + if(NOT split_ipc_lines) + message(FATAL_ERROR "${split_case}: ${mobilegl_log} carries no 'Config: IPC' line, so the " + "configuration MGTRACE_EXPECT_IPC names cannot be checked.") + endif() + list(GET split_ipc_lines -1 split_ipc_line) + string(REPLACE " " ";" split_ipc_tokens "$ENV{MGTRACE_EXPECT_IPC}") + set(split_ipc_missing "") + foreach(token IN LISTS split_ipc_tokens) + if(NOT "${token}" STREQUAL "") + string(FIND " ${split_ipc_line} " " ${token} " split_ipc_at) + if(split_ipc_at EQUAL -1) + list(APPEND split_ipc_missing "${token}") + endif() + endif() + endforeach() + if(split_ipc_missing) + string(REPLACE ";" " " split_ipc_missing "${split_ipc_missing}") + message(FATAL_ERROR + "${split_case}: the client resolved '${split_ipc_line}', which lacks " + "${split_ipc_missing} (MGTRACE_EXPECT_IPC='$ENV{MGTRACE_EXPECT_IPC}'). " + "$ENV{MGTRACE_EXPECT_IPC_REASON}") + endif() + message(STATUS "MGPipe split: ${split_case} IPC expectation met ($ENV{MGTRACE_EXPECT_IPC})") + endif() # The wire volume, on the legs that asked for it (MGTRACE_WIRE_EVIDENCE above). The client # counts each record at the codec commit and prints one frame-sent line per Present, keyed # by raw opcode; the server role's backend counts its draws and prints one stats line per @@ -569,6 +609,7 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " "case=${TRACE_CASE_NAME}\nbackend=${TRACE_BACKEND}\ntransport=$ENV{MOBILEGL_TRANSPORT}\n" "control=${split_control}\nmarker=${split_expected_marker}\nsecond=${split_second_found}\n" "server_log=1\nfatal_lines=${split_fatal_count}\nfloor_lines=${split_floor_count}\n" + "ipc_expected=${split_ipc_expected}\n" "wire=${trace_wire_evidence}\ndraw_opcode=${split_draw_opcode}\n" "wire_draw_records=${split_wire_records}\nserver_draws=${split_server_draws}\n") file(WRITE "${TRACE_OUTPUT_DIR}/output/split-evidence.txt" "${split_evidence}") diff --git a/tools/trace_replay/test_run_trace_case.py b/tools/trace_replay/test_run_trace_case.py index 2ce33d7f..3400fdc7 100644 --- a/tools/trace_replay/test_run_trace_case.py +++ b/tools/trace_replay/test_run_trace_case.py @@ -261,6 +261,40 @@ class RunTraceCaseTests(unittest.TestCase): self.assertNotEqual(quiet_check([str(self.evidence), '--transport', 'inproc']), 0, 'a leg whose transport block was skipped must fail the checker') + # MGTRACE_EXPECT_IPC: the client's IPC line must carry every token, whole. + IPC = 'Config: IPC ring=8MiB verb-barrier=1 run-ahead={ra} present-credit=1 strict={st} audit=0 role-split-state={rs} x' + + def ipc_logs(self, ra='1', st='0', rs='1', line=True): + logs = good_logs('spawn') + if line: + logs['mobilegl.client.log'] += lines(self.IPC.format(ra=ra, st=st, rs=rs)) + return logs + + def ipc_env(self, expect='strict=0 role-split-state=1 run-ahead=1'): + env = self.split_env('spawn') + env.update({'MGTRACE_EXPECT_IPC': expect, 'MGTRACE_EXPECT_IPC_REASON': 'restore the inproc rows'}) + return env + + def test_i1_the_expected_defaults_are_met(self): + output = self.expect_green(self.ipc_env(), self.ipc_logs(), 'spawn') + self.assertIn('IPC expectation met', output) + self.assertEqual(checker.parse(self.evidence.read_text())['ipc_expected'], + 'strict=0 role-split-state=1 run-ahead=1') + + def test_i2_a_changed_default_names_the_reason(self): + self.expect_red(self.ipc_env(), self.ipc_logs(rs='0'), 'lacks role-split-state=1') + self.expect_red(self.ipc_env(), self.ipc_logs(rs='0'), 'restore the inproc rows') + + def test_i3_no_ipc_line(self): + self.expect_red(self.ipc_env(), self.ipc_logs(line=False), "carries no 'Config: IPC' line") + + def test_i4_a_token_is_matched_whole(self): + self.expect_red(self.ipc_env(), self.ipc_logs(ra='10'), 'lacks run-ahead=1') + + def test_i5_an_expectation_on_a_monolith_run(self): + self.expect_red({'MOBILEGL_TRANSPORT': 'monolith', 'MGTRACE_EXPECT_IPC': 'strict=1'}, {}, + 'a monolith run logs no IPC configuration to check') + # The verify block, which shares the script. def test_v1_verify_without_the_armed_line(self): self.expect_red({'MOBILEGL_PIPE_VERIFY': '1'}, {'mobilegl.log': lines('Config: loaded')}, diff --git a/tools/trace_replay/trace_cases.py b/tools/trace_replay/trace_cases.py index 20ef49cb..31a92bc4 100644 --- a/tools/trace_replay/trace_cases.py +++ b/tools/trace_replay/trace_cases.py @@ -351,7 +351,7 @@ def github_verify_matrix(cases): SPLIT_TRANSPORTS = ("inproc", "spawn") -def github_split_matrix(cases): +def github_split_matrix(cases, transports=SPLIT_TRANSPORTS, keep_inproc=()): """{backend, case, transport} for the split arm. THREE DIMENSIONS, NOT TWO. The job used to take {backend, case} and hard-code @@ -359,15 +359,31 @@ def github_split_matrix(cases): backend list is the case's own ci_backends - the same list the monolith matrix uses - so a case restricted to one backend stays restricted here instead of being registered with a backend it does not run. + + `transports` narrows the arms (test.yml drops inproc when the inproc-acceptance retrace rows + already replay the same library); `keep_inproc` names split cases whose inproc rows stay + anyway - OpenRA's, which carry the wire-volume evidence. A narrowed matrix that comes out + empty is an error; the full one is returned empty, and test.yml names that error itself. """ - return { - "include": [ - {"backend": backend, "case": case["name"], "transport": transport} - for case in split_trace_cases(cases) - for backend in ci_backends(case) - for transport in SPLIT_TRANSPORTS - ] - } + unknown = [t for t in transports if t not in SPLIT_TRANSPORTS] + if unknown or not transports: + raise ValueError(f"split transports must be a non-empty subset of {SPLIT_TRANSPORTS}: {transports}") + split_cases = split_trace_cases(cases) + split_names = {case["name"] for case in split_cases} + missing = [name for name in keep_inproc if name not in split_names] + if missing: + raise ValueError(f"--keep-inproc names cases that are not split CI cases: {missing}") + rows = [ + {"backend": backend, "case": case["name"], "transport": transport} + for case in split_cases + for backend in ci_backends(case) + for transport in SPLIT_TRANSPORTS + if transport in transports or (transport == "inproc" and case["name"] in keep_inproc) + ] + narrowed = tuple(transports) != SPLIT_TRANSPORTS or bool(keep_inproc) + if not rows and narrowed: + raise ValueError("the narrowed split matrix is empty") + return {"include": rows} def github_apk_matrix(cases): @@ -393,9 +409,10 @@ def github_transport_matrix(cases, *, apk=False): THE SPAWN LANE IS APK-ONLY, and that is a statement about where the evidence is missing rather than about what spawn can do. On Linux the spawn arm already has a job of its own - - test.yml's `retrace-split`, which is {backend, case, transport} over inproc AND spawn and - whose legs prove in-test that the transport resolved and, on OpenRA, that draw records - crossed the wire (run_trace_case.cmake's evidence). Adding spawn to THIS matrix would pay + test.yml's `retrace-split`, which is {backend, case, transport} over spawn (and inproc too + when it cannot reuse this matrix's inproc rows' library) and whose legs prove in-test that + the transport resolved and, on OpenRA, that draw records crossed the wire + (run_trace_case.cmake's evidence). Adding spawn to THIS matrix would pay for a second full Linux sweep that proves nothing the first one does not, and test.yml's `retrace` job has neither the server image staged nor MOBILEGL_IPC_SERVER_PATH set - so the lane would not merely be redundant, it would be red. @@ -466,6 +483,41 @@ def self_test_transport_matrices(): f"entries; exact metadata and negative controls OK") +def self_test_split_matrix(cases=None): + """retrace-split may drop its inproc rows only because the inproc-acceptance rows replay them.""" + if cases is None: + cases = ci_trace_cases(load_trace_cases()) + full = github_split_matrix(cases)["include"] + spawn_only = github_split_matrix(cases, transports=("spawn",), keep_inproc=("OpenRA",))["include"] + assert [r for r in spawn_only if r["transport"] == "spawn"] == [r for r in full if r["transport"] == "spawn"], \ + "the spawn-only matrix lost or changed a spawn row" + assert any(r["transport"] == "spawn" for r in spawn_only), "the spawn-only matrix has no spawn row" + kept = [r for r in spawn_only if r["transport"] == "inproc"] + assert kept and all(r["case"] == "OpenRA" for r in kept), "the kept inproc rows are not OpenRA's" + assert all(r in full for r in kept), "a kept inproc row is not a row of the full matrix" + acceptance = {(r["backend"], r["case"]) for r in github_transport_matrix(cases)["include"] + if r["lane"] == "inproc-acceptance"} + dropped = [(r["backend"], r["case"]) for r in full if r["transport"] == "inproc" and r not in kept] + uncovered = [row for row in dropped if row not in acceptance] + assert not uncovered, f"inproc rows dropped from retrace-split that no acceptance row replays: {uncovered}" + for bad in ({"transports": ("unix",)}, {"transports": ()}, {"keep_inproc": ("no-such-case",)}): + try: + github_split_matrix(cases, **bad) + except ValueError: + continue + raise AssertionError(f"github_split_matrix accepted {bad}") + try: + github_split_matrix([], transports=("spawn",)) + except ValueError: + pass + else: + raise AssertionError("an empty narrowed split matrix silently passed") + assert github_split_matrix([]) == {"include": []}, \ + "the full split matrix must come back empty, for the workflow's named error" + print(f"split matrix: {len(full)} rows, {len(spawn_only)} with inproc dropped ({len(kept)} OpenRA inproc " + f"kept); all {len(dropped)} dropped inproc rows are inproc-acceptance rows") + + def cmake_quote(value): return '"' + str(value).replace("\\", "/").replace('"', '\\"') + '"' @@ -516,7 +568,13 @@ def parse_args(): parser.add_argument("--case", dest="case_name", help="Trace case name.") parser.add_argument("--ci", action="store_true", help="Only include cases enabled for CI.") parser.add_argument("--self-test-transport-matrices", action="store_true") + parser.add_argument("--self-test-split-matrix", action="store_true", + help="prove the spawn-only split matrix drops only inproc-acceptance rows") parser.add_argument("--fixture-root", default="tools/trace_replay/fixtures") + parser.add_argument("--transports", default=",".join(SPLIT_TRANSPORTS), + help="github-split-matrix: the comma-separated split transports to emit") + parser.add_argument("--keep-inproc", default="", + help="github-split-matrix: comma-separated split cases whose inproc rows stay") parser.add_argument( "--format", choices=( @@ -541,6 +599,9 @@ def main(): if args.self_test_transport_matrices: self_test_transport_matrices() return 0 + if args.self_test_split_matrix: + self_test_split_matrix() + return 0 cases = load_trace_cases() if args.ci: cases = ci_trace_cases(cases) @@ -553,7 +614,9 @@ def main(): elif args.format == "github-verify-matrix": print(json.dumps(github_verify_matrix(cases), separators=(",", ":"))) elif args.format == "github-split-matrix": - print(json.dumps(github_split_matrix(cases), separators=(",", ":"))) + transports = tuple(t for t in args.transports.split(",") if t) + keep = tuple(c for c in args.keep_inproc.split(",") if c) + print(json.dumps(github_split_matrix(cases, transports, keep), separators=(",", ":"))) elif args.format == "github-apk": print(json.dumps([github_apk_case(case) for case in cases], separators=(",", ":"))) elif args.format == "github-apk-matrix":