Generate the config reference documentation from the config registry - #2146
Merged
Conversation
Replace crate-level #![allow(dead_code)] with #![expect(dead_code)] to ensure the attribute becomes a hard error once main() is wired up and items become reachable, forcing cleanup rather than silently suppressing findings.
The file's test module exercises every function, making the expect(dead_code) attribute unfulfilled under --all-targets. Dead code warnings in non-test builds are expected until main is wired up.
cargo clippy --all-targets compiles both the normal bin target, where the stub main leaves every item unreachable, and the test target, where the tests exercise all of them. An unconditional attribute fails one config or the other. Guarding on not(test) satisfies both, and expect keeps the attribute self-cleaning once main is wired up.
splice() matched BEGIN/END markers on the trimmed line with no regard for fenced code blocks or indentation, so a marker documented as a syntax example (e.g. showing "how to add a config key" inside a fenced block, or indented in a list item) was indistinguishable from a real one. The generator would then silently overwrite or prematurely close a documented example as if it were a live region, corrupting the docs without any error. Require markers to start at column 0 (trailing whitespace still tolerated) and track fenced-code-block state while scanning, ignoring BEGIN/END while inside a fence. The region-body-consuming inner loop tracks fence state too, so an END shown inside a fenced example within a live region no longer closes that region early.
Fix grammar and redundant wording in the descriptions for BALLISTA_PROPAGATE_EMPTY_ENABLED and BALLISTA_COALESCE_TARGET_PARTITION_BYTES, now that the config docs generator publishes them to the user-facing reference table.
GitHub alert syntax is not used anywhere else in docs/source and these pages render through myst-parser, where it may show the literal marker instead of an admonition.
config-docs-check runs cargo run -p ballista-core, which triggers build.rs and requires protoc on PATH. The job was modelled on datafusion-proto-sync-check, which never compiles Rust and doesn't need protoc, so that template omitted the install step. Add it, matching the approach already used in docker.yml.
Every other Rust-compiling CI job uses --profile ci to strip debuginfo and keep the rust-cache payload small. Bring update_config_docs.sh in line; the profile only changes build output, not behaviour.
run() wrote changed files inside its main loop but only reported
them afterwards, in a branch skipped by the early return from the
orphan-key check. A contributor who rewrites a description and adds
an undocumented key in the same commit would see only the orphan
error, with no indication that configs.md had already been rewritten
on disk. Print each "updated {path}" line as soon as that file is
written so it always surfaces before any later failure.
main() only ever checked for the presence of --check, so any other argument (a typo like --chek, or --help) silently fell through to write mode. In CI that would turn the config-docs-check gate into a permanent no-op pass. Parse arguments explicitly: accept --check, print usage and exit 0 for --help/-h, and exit non-zero with usage for anything else.
All descriptions are single-line today, but nothing stopped a future multi-line description from emitting a literal newline mid-row, silently corrupting the table while still leaving the generator idempotent. Collapse newlines and carriage returns to a single space alongside the existing pipe escaping, and add a test mirroring the existing pipe-escaping invariant test.
The message's primary suggestion was to widen an existing region's prefix=, which a contributor could satisfy by widening all the way to prefix=ballista., dumping every key into one table and duplicating the others. Reword so the primary suggestion is adding the key to the marker region for its own section, and mention prefix widening only as the narrower fallback it is.
github-actions
Bot
added
documentation
Member
Author
|
@phillipleblanc fyi |
andygrove
marked this pull request as ready for review
July 22, 2026 16:00
PR apache#2124 changed hash_join_max_build_partition_bytes from 0 to 64 MiB and reworded its description. The generated table now reflects that.
avantgardnerio
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #2145.
Rationale for this change
docs/source/user-guide/configs.mdwas hand-maintained, so it had drifted fromthe config keys the code actually reads. Nine keys were documented nowhere, and
two more only incidentally (in the 54.0.0 upgrade notes and the benchmarking
guide).
Fixing those gaps by hand would leave the same problem in place. Since
ballista/core/src/config.rsalready builds a runtime registry(
CONFIG_ENTRIES) carrying each key's name, description, type, and default,the reference tables can be generated from it, and CI can fail when a
registered key is documented nowhere. That turns a recurring maintenance chore
into a build failure with an actionable message.
This follows the approach the sibling DataFusion repo already uses
(
dev/update_config_docs.shplus a CI check), adapted to Ballista'sci/scripts/convention so the failure names the offending file or key ratherthan printing a raw diff.
What changes are included in this PR?
Generator. A new binary,
ballista/core/src/bin/update_config_docs.rs,reads
BallistaConfig::valid_entries()and rewrites the body ofmarker-delimited regions in
docs/source/**/*.md:The optional
prefix=filter lets one page carry several scoped tables. Thebinary supports
--checkto report drift without writing, and fails if anyregistered key is rendered by no region. All logic lives in the binary rather
than in
src/, so none of it becomes public API. It adds no dependencies —stdonly, including the directory walk.Config registry.
ConfigEntrygains public accessors (name,description,data_type,default_value) plusdoc_default, an optionaloverride for defaults that are machine-dependent or unreadable.
ballista.standalone.parallelismdefaults toavailable_parallelism(), whichwould otherwise render a different number on every machine and fail the check
on any runner; it now renders as
number of available CPU cores.ballista.testing.chaos_execution.seeddefaults to the empty string and nowrenders as
(empty). Three descriptions too terse to stand alone in areference table were reworded.
Documentation.
configs.mdgains three regions whose prefixes partitionthe whole key space — session settings, shuffle settings, and a new "Testing
and Fault Injection" section giving the chaos-monkey keys a home. All 39
registered keys now appear exactly once. The hand-written scheduler CLI table
is unchanged and sits outside every region.
Tooling and CI.
dev/update_config_docs.shregenerates;ci/scripts/rust_config_docs_check.shwraps--check; aconfig-docs-checkjob runs it.
CONTRIBUTING.mddocuments the workflow.Two notes on the generated output. Each table is preceded by
<!-- prettier-ignore -->, with a blank line before eachENDmarker. Bothwere verified empirically against prettier 2.7.1 as necessary for the output to
be a fixed point. Without the directive, the existing
prettierjob and thenew check job would deadlock, each rewriting what the other demands. Separately,
markers are recognised only at column 0 and outside fenced code blocks, so
marker syntax shown as an example in prose is not mistaken for a live region.
Are there any user-facing changes?
Yes, documentation only. The configuration reference now lists eleven
previously undocumented or under-documented settings, and the tables are
generated rather than hand-maintained.
There are no breaking API changes.
ConfigEntrygains public accessors, whichis additive — the struct was previously public but entirely opaque.
docs/source/user-guide/tuning-guide.mdstill carries hand-written tables thatoverlap the generated ones. Their values agree, so there is no contradiction.
Converting them to filtered regions is left to a follow-up, which is doc-only
now that the multi-file walk exists.
One known gap worth calling out for reviewers:
rust.yml's path filters do notinclude
docs/**, so a PR that only hand-editsconfigs.mdwill not triggerthe check job. The main drift vector — editing
config.rs— is covered, and anotice at the top of the page warns against hand-editing generated regions.
Closing the gap properly would need either re-including the file in the path
filters, which runs the full Rust matrix on docs typos, or a separate
lightweight workflow. Happy to take direction on which is preferred.