fix: restore the site's mermaid diagrams and make a dropped one fail CI - #6064
Conversation
All three mermaid diagrams are absent from the published pages. apache#6021 moved rendering from the reader's browser to build-time SVG, which fixed apache#6020's CSP block, and mmdc has produced nothing since: the publish commit for that change removed the raw blocks and added no <object> and no _images/mermaid-*.svg. sphinxcontrib-mermaid downgrades a render failure to a warning and drops the node, so the deploy stays green and the page publishes with a hole in it. docs/README.md and the workflow comment both already said this was the failure mode; nothing enforced it, and the docs job runs only on push to main, so apache#6021 could not have been caught before merge either. Two candidate causes, both closed out here rather than guessed between: Chrome's setuid sandbox cannot start under the AppArmor policy Ubuntu ships from 23.10 onwards, and puppeteer's postinstall catches its own browser download failure and exits 0, which leaves a green install step with no browser at all. A puppeteer config with --no-sandbox covers the first; an explicit browsers install, allowed to fail the job, covers the second. The durable half is dev/ci/check-mermaid.py. It renders every fence with the arguments conf.py gives the build, so preflight fails on the pull request rather than after the merge, and --built asserts the built site carries a non-empty SVG per fence with a page referencing it, before the publish step runs. Checked against the currently published HTML, where it reports 3 fences and 0 SVGs.
The first run of this check on a runner reproduced the failure -- all three diagrams -- but the annotation said only 'mmdc cannot render this diagram', and a job log is not always reachable. The annotation is the part of a failing job that always is, so it carries the whole error now.
The annotation from the previous run named the cause: the browser downloads onto ubuntu-slim and then cannot start, because the slim image carries none of Chrome's shared libraries. Install the ones chrome-headless-shell links against.
|
Both causes are now pinned down empirically rather than inferred, and they are different on the two runners. The docs deploy,
and the failing arm says it outright: So that is #6062's root cause, confirmed: Chrome cannot start on the deploy runner, mmdc exits non-zero, Preflight, Worth noting how that second one was diagnosed, because it is the reason for the last commit: the first CI run reproduced the failure but the annotation said only "mmdc cannot render this diagram", and GitHub-hosted job logs are not always reachable. The check now puts mmdc's full stderr in the annotation, which is what made both of these readable without opening a log. Cost. Preflight goes from ~1m to 2m24s: 80s for the apt packages, npm install and browser fetch, 7s for the three renders. If that is too much for every pull request, the alternatives are to cache |
sunchao
left a comment
There was a problem hiding this comment.
Reviewed 643d9787 against base d1bf687e, including the current discussion and CI merge dcfe2de3. No verified P1/P2 findings.
Correctness
The previous failure could leave CI green while Sphinx dropped a Mermaid node. This change checks both stages: rendering every current source fence with the configured mmdc arguments, then checking the built tree for enough non-empty SVGs and references from HTML before the publish step. Missing output, render failures, and the unsupported client-rendered format now produce a failing exit status. The error annotation preserves multiline diagnostics with workflow-command escaping. The configuration path resolves to the same JSON file from both docs/source/conf.py and the docs/temp/ copy.
I verified all nine authored files and the relevant build/configuration sources are identical on the tested merge. The 20 focused local checks passed using synthetic built sites and renderer test doubles, including one/all missing diagrams, empty SVGs, an unreferenced SVG, missing mmdc, browser-launch diagnostics, and exit zero without output. The existing CI-config check also passed. These checks did not launch a browser, mmdc or Sphinx locally.
The current-head preflight checked out dcfe2de3, whose parents are the stated base and head, and logged that all three diagrams rendered on ubuntu-slim. The full docs deployment is skipped on this PR. The author's two-arm ubuntu-24.04 fork probe is separate reported evidence, not a current-head deployment result. At September 20, 15:44 UTC, CodeQL and preflight were successful and the Linux expression shard was still running.
This changes documentation and CI behavior, with no Spark expression, operator, type, null, error or fallback semantics changed. No Spark/JNI execution or version-specific compatibility result is claimed. Maintained Spark 3.4/4.1 sources remain unavailable.
Performance
The cost is on the shared CI gate. The verified preflight took 2m24s, including 80 seconds for the Mermaid dependency/browser installation step and 7 seconds for three renders. Those timings corroborate the author's current-run figures. The roughly one-minute prior baseline and the proposed cache savings remain author reports.
The installation runs on every preflight event, including changes unrelated to docs, before heavy jobs can start. That buys early detection of renderer failures but adds network and package-install dependencies to the gate. The author has already identified caching as an alternative. There is no Spark runtime overhead or measured query-performance change.
Design
Keeping the CLI version in one place and loading the build's actual mermaid_params reduces drift between preflight and deployment. Explicit browser installation makes a failed fetch fail the step. On deployment, render validation precedes the build and artifact validation precedes copying to asf-site, so either failure retains the existing published site. The source-to-output check is an aggregate guard for the current three distinct diagrams, not a per-page DOM completeness proof.
Abstraction & complexity
The single script gives both workflows shared configuration and diagnostics without another service or test framework. Distinct-source counting accounts for Sphinx's reuse of diagram files, and the existing routing table and routing check cover edits to the new script. The RAT exception is restricted to the JSON configuration. The separation between render validation and built-output validation is justified by their different failure boundaries.
Which issue does this PR close?
Closes #6062.
Rationale for this change
All three mermaid diagrams are missing from the published site — not blank, not broken images, absent from the HTML:
#6021 moved rendering from the reader's browser to build-time SVG, which fixed #6020's CSP block. Its publish commit (
b9ea47c16) removed the<pre class="mermaid">blocks and the jsdelivr import from both pages and added nothing in their place, and nothing has rendered since.sphinxcontrib-mermaiddowngrades a render failure to a Sphinx warning and drops the node, so the deploy stays green and the page publishes with a diagram-shaped hole.docs/README.mdand the comment on theInstall mermaid-clistep both already said this was the failure mode. Nothing enforced it. And thedocsjob is push-tier by POLICY, so no docs build ran on #6021's pull request either — the change was verified locally on macOS, where it produced the expected 2 and 1<object>embeds.Two candidate causes, and rather than guess between them without the job log, this closes out both:
ubuntu-24.04runner.puppeteer'spostinstall, which fetches the browsermmdcdrives, catches its own download failure and exits 0. A flaky fetch leaves a green install step with no browser at all, andmmdcdies withCould not find chrome-headless-shell. That is reproducible locally and is written up on the issue.What changes are included in this PR?
docs/puppeteer-config.json+mermaid_paramsindocs/source/conf.py: passmmdca puppeteer config with--no-sandbox. The CI container is already the isolation boundary.DOCS_DIRresolves from__file__, so the path is right whetherconf.pyis read fromdocs/source/or from thedocs/temp/copybuild.shmakes.$RUNNER_TEMPand then runpuppeteer browsers install chrome-headless-shellexplicitly, where a failure is allowed to fail the job.dev/ci/check-mermaid.py, the durable half, in two modes:```mermaidfence underdocs/source/with mmdc. It readsmermaid_paramsout ofconf.pyrather than repeating them, so the check cannot pass with flags the build does not use. Run from preflight, so a diagram that only mmdc rejects fails on the pull request.--built docs/build/html: assert the built site carries a non-empty SVG per distinct fence and that some page references each one. Run after the build and before the publish step, so a bad render leaves the previous site in place.--cli-spec, so the pull-request check and the deploy cannot render with different versions.dev/ci/compute-changes.pyroutes the new script to thedocsjob, with the spot check incheck-ci-config.py;pom.xmlgets a RAT exclude for the JSON, which can carry no license header;docs/README.mddocuments both checks and theCould not find chrome-headless-shellcase.How are these changes tested?
The new script is the test, and it was calibrated against the real failure rather than a synthetic one:
memory_management.htmlandci.htmlfromasf-site, no_imagesdocs/source has 3 distinct ```mermaid fences but the build produced 0 SVG(s)mermaid-*.svgreferenced by<object>PATHnpx puppeteer browsers installhintdocs/source/**ci.md:35,memory_management.md:113and:395conf.pyfromdocs/temp/conf.pyone directory overDOCS_DIRstill resolves todocs/, config file found-pis actually readmmdc -p /nonexistent.jsonvs the real pathactionlint,python3 dev/ci/check-ci-config.py,./mvnw -N apache-rat:checkandprettier --checkall pass locally.What I could not verify locally: a successful end-to-end render. Chrome's download host is blocked in my environment and there is no Chrome on the machine, so every render attempt stops at browser launch. The first docs run on main after this merges is what proves the fix — and if a third cause is in play, that run now fails loudly with mmdc's stderr instead of publishing a hole.