Repository navigation
feat(cli): compact stopped datadirs with client commands - #351
Conversation
Neither client ships an offline compaction command, so db_compaction now runs RocksDB's ldb (11.8.1, Dockerfile.rocksdb-ldb) against the stopped datadir: every directory with an OPTIONS-* file is a database, and each of its column families is compacted with --try_load_options. - DBMaintenanceCommands.CompactImage runs Compact in a tool image, with Compact[0] as the entrypoint; inspection and prepare steps keep the client image. db_compaction.image overrides either, and an image other than the instance's is pulled if not present. - list_column_families prints raw names after a header line, and besu's are single bytes including 0x09 and 0x0a, so the script splits on ", " only and never trims. - A database whose OPTIONS names a merge operator ldb lacks is refused, since ldb would substitute a string-append operator; and because `ldb compact` silently falls back to the default family for an unknown name, each family must show an empty level 0 afterwards.
…8 blocks geth's path scheme writes its 128 diff layers and the unflushed write buffer below them to geth/triedb/merkle.journal on a clean stop, and loads them back into memory on the next start, so that state is never read from the database a compaction rewrote. After the compaction the runner now parses the journal (layout version 3, triedb/pathdb/journal.go in geth v1.17.7) and fails the phase when the write buffer is not empty or there are more than 128 diff layers. Another journal version is an error rather than a guess. DBMaintenanceCommands gains Verify for this; db_compaction.verify: false skips it, and a volume datadir, which the runner cannot read, is not checked. Checked against three journals written by geth v1.17.7 --dev: layers=25 and layers=128 (after --cache.gc=0) pass with 25 and 128 diff layers; layers=150 fails with 128 diff layers over a non-empty buffer.
… a run
benchmarkoor db compact --client <geth|nethermind|besu|erigon|reth|ethrex> \
--datadir <dir> [--prepare <step>]...
runs the same containers and checks as a db_compaction phase
(runner.CompactDatadir drives runDBCompactionContainers) against a stopped
client's host datadir, and exits non-zero on any failure, the geth journal
check included. reth is a no-op with a log line (MDBX, persisted every
block); a client without offline compaction is an error. --image,
--compaction-image, --extra-arg, --timeout and --no-inspect mirror the
db_compaction settings. No marker is written: that belongs to a run.
TestCompactDatadir_RealLdb runs it through the docker daemon against real
RocksDB databases when BENCHMARKOOR_TEST_LDB_IMAGE names an ldb image.
…ult in db compact A node stopped with a non-empty write buffer (any default-cache run) fails the check, so defaulting it on in db_compaction would break existing configs. benchmarkoor db compact keeps it on (--no-verify skips it).
…d after compaction A 52-minute compaction of besu's 1.1 TB jochemnet database logged only the family names and a total duration: nothing in the output showed whether the database changed. The level 0 check now reads the same levelstats.
![redpandabot[bot]](https://avatars.githubusercontent.com/in/3749543?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ex stays uncompacted reth was listed as 'nothing to compact' (MDBX). MDBX never shrinks: the pages a replay frees stay in mdbx.dat and in every archive of it, and Reth 2.7.0 bundles mdbx_copy as `reth db copy --compact`. The compaction is a shell in the reth image: check free space, copy compacted into <datadir>/db.compact, carry database.version over, swap, report the sizes. DBMaintenanceCommands gains OwnEntrypoint for a client whose compaction is a shell around its own binary. `reth db stats` is the inspection. ethrex (29.x) is RocksDB, but one family (TRANSACTION_LOCATIONS) has a merge operator ldb does not have, so the generic ldb compaction refuses it, as it should; recorded on the spec instead of 'no compaction command yet'.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
clap rejected it on the subcommand ("unexpected argument '--datadir' found",
exit 2) on the 2026-10-11 build; verified against ghcr.io/paradigmxyz/reth:v2.7.0.
It fails with EEXIST (error 17) on a pre-created one (2026-10-11 build).
….dat Reth opens MDBX in no-subdir mode, so mdbx_env_copy writes a single file at the destination (seen on the 2026-10-11 build: db.compact as a growing file). Replace db/mdbx.dat with it and drop the stale mdbx.lck; database.version stays where it is.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
qu0b
changed the title
There was a problem hiding this comment.
Summary
This PR adds benchmarkoor db compact, which runs Geth's/Erigon's own offline compaction against a stopped host datadir, reusing the runner's inspection, optional prepare steps, timeout and error handling without writing run markers or reports. The two open findings no longer apply: the refactor dropped the custom RocksDB and reth compactors (and pkg/client/rocksdb_compact.sh) entirely, so there is no level-0 guard or dropped reth extra_args left to fix. Two input-validation gaps remain in the new CLI surface.
Issues
- 🟡
pkg/runner/db_compaction.go:990— empty --datadir silently compacts the working directory — see the thread on that line - 🟢
cmd/benchmarkoor/db.go:64— non-positive --timeout parses, then immediately expires the compaction context — see the thread on that line
Reviewed @ d6995bae
"Testing shows the presence, not the absence of bugs." — Edsger Dijkstra
| return err | ||
| } | ||
|
|
||
| dataDir, err := filepath.Abs(c.DataDir) |
There was a problem hiding this comment.
🟡 empty --datadir silently compacts the working directory
--datadir= still sets the flag, so Cobra's MarkFlagRequired("datadir") passes, and filepath.Abs("") returns the process working directory; CompactDatadir then bind-mounts that directory at /data and runs the client's compaction against it. Reject an empty DataDir before Abs — the run path always supplies a non-empty mount source, so only the CLI can hit this.
| return fmt.Errorf("client %q has no offline database compaction", ct) | ||
| } | ||
|
|
||
| if _, err := time.ParseDuration(dbCompactFlags.timeout); err != nil { |
There was a problem hiding this comment.
🟢 non-positive --timeout parses, then immediately expires the compaction context
Only time.ParseDuration is checked, so --timeout 0 or a negative duration is accepted; DBCompactionConfig.EffectiveTimeout returns it unchanged and context.WithTimeout(ctx, d) starts already cancelled, failing every container with a deadline error. The run-phase validation rejects d <= 0 (must be positive), so the CLI check should match.
There was a problem hiding this comment.
Adds benchmarkoor db compact, a config-free CLI wrapper (Geth/Erigon only) around the runner's existing db-compaction machinery, plus runner.CompactDatadir. The previously reported empty-datadir and non-positive-timeout issues are fixed by commit 5b1ada3 with tests; the earlier RocksDB/reth findings are gone because that code was removed from the net diff. I found no remaining correctness, resource-lifecycle, or concurrency problems.
Reviewed 5 changed file(s) @ 5b1ada31 — no blocking issues found.
"Testing shows the presence, not the absence of bugs." — Edsger Dijkstra
1 participant
Adds
benchmarkoor db compactso snapshot builders can run the existing offline maintenance without starting a benchmark run.Only Geth and Erigon are supported. The command uses their own
db compactcommands in a client container. Use--imageto select the version that wrote the datadir; the client must be stopped first. Other clients return an unsupported error.The CLI reuses the runner's inspection, optional preparation steps, timeout and error handling. It streams container output and does not write benchmark markers or reports.
No new compaction implementation, database-file replacement, journal parser, helper image or image-publishing workflow is included. Snapshot workflow #44 uses this command for Geth and Erigon and skips compaction for the other clients.
Validation: client/config/runner/CLI package tests and golangci-lint pass. New tests cover pinned client images, native command execution, unsupported clients, and image-pull/compaction failures.