From 3638b3461a5c88d129bf94b72b715038b9ae6b1c Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 8 Oct 2026 16:35:03 +0200 Subject: [PATCH 01/13] feat(client): compact nethermind and besu with RocksDB ldb 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. --- .../workflows/deploy-docker.rocksdb-ldb.yaml | 72 +++++++ Dockerfile.rocksdb-ldb | 18 ++ docs/configuration.md | 26 ++- pkg/client/besu.go | 18 +- pkg/client/client.go | 5 + pkg/client/nethermind.go | 9 +- pkg/client/rocksdb.go | 21 ++ pkg/client/rocksdb_compact.sh | 69 +++++++ pkg/client/rocksdb_test.go | 189 ++++++++++++++++++ pkg/config/config.go | 6 +- pkg/config/config_test.go | 4 +- pkg/runner/db_compaction.go | 47 ++++- pkg/runner/db_compaction_test.go | 53 ++++- 13 files changed, 499 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/deploy-docker.rocksdb-ldb.yaml create mode 100644 Dockerfile.rocksdb-ldb create mode 100644 pkg/client/rocksdb.go create mode 100644 pkg/client/rocksdb_compact.sh create mode 100644 pkg/client/rocksdb_test.go diff --git a/.github/workflows/deploy-docker.rocksdb-ldb.yaml b/.github/workflows/deploy-docker.rocksdb-ldb.yaml new file mode 100644 index 000000000..c9f071327 --- /dev/null +++ b/.github/workflows/deploy-docker.rocksdb-ldb.yaml @@ -0,0 +1,72 @@ +name: Deploy - Docker (RocksDB ldb) + +# The ldb image db_compaction uses for nethermind and besu +# (client.RocksDBLdbImage). Tagged by RocksDB version, so bump both together. +on: + push: + branches: + - master + paths: + - Dockerfile.rocksdb-ldb + - .github/workflows/deploy-docker.rocksdb-ldb.yaml + workflow_dispatch: + +env: + IMAGE: ghcr.io/${{ github.repository }}-rocksdb-ldb + ROCKSDB_VERSION: "11.8.1" + +jobs: + build: + name: Build ${{ matrix.arch }} image + runs-on: ${{ matrix.runner }} + strategy: + matrix: + include: + - arch: amd64 + runner: ubuntu-latest + - arch: arm64 + runner: ubuntu-24.04-arm + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Log in to GHCR + uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Build and push + run: | + docker build . --file Dockerfile.rocksdb-ldb \ + --build-arg ROCKSDB_VERSION="$ROCKSDB_VERSION" \ + --platform "linux/${{ matrix.arch }}" \ + --tag "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" + docker run --rm "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" --version | grep -F "RocksDB $ROCKSDB_VERSION" + docker push "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" + + manifest: + name: Multi-arch manifest + needs: build + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@c47758b77c9736f4b2ef4073d4d51994fabfe349 # v3.7.1 + + - name: Log in to GHCR + uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Create manifest + run: | + docker buildx imagetools create -t "$IMAGE:$ROCKSDB_VERSION" \ + "$IMAGE:$ROCKSDB_VERSION-amd64" "$IMAGE:$ROCKSDB_VERSION-arm64" diff --git a/Dockerfile.rocksdb-ldb b/Dockerfile.rocksdb-ldb new file mode 100644 index 000000000..f70da56b2 --- /dev/null +++ b/Dockerfile.rocksdb-ldb @@ -0,0 +1,18 @@ +# RocksDB's ldb, which db_compaction uses to compact nethermind and besu: neither +# ships an offline compaction command. The compaction script (pkg/client/ +# rocksdb_compact.sh) also needs sh, find, sort, grep and od, which the base has. +# Release build: a default `make ldb` links the debug tools library. +FROM debian:bookworm AS build +ARG ROCKSDB_VERSION=11.8.1 +RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \ + ca-certificates git build-essential libsnappy-dev zlib1g-dev libbz2-dev liblz4-dev libzstd-dev \ + && rm -rf /var/lib/apt/lists/* +RUN git clone -q --depth 1 -b "v${ROCKSDB_VERSION}" https://github.com/facebook/rocksdb /rocksdb \ + && cd /rocksdb && DEBUG_LEVEL=0 PORTABLE=1 make -j"$(nproc)" ldb + +FROM debian:bookworm-slim +RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \ + libsnappy1v5 zlib1g libbz2-1.0 liblz4-1 libzstd1 \ + && rm -rf /var/lib/apt/lists/* +COPY --from=build /rocksdb/ldb /usr/local/bin/ldb +ENTRYPOINT ["ldb"] diff --git a/docs/configuration.md b/docs/configuration.md index b3014cfd5..312f73144 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1135,19 +1135,29 @@ runner: ##### Database Compaction -The `db_compaction` option compacts the client database before the run measures anything. Compaction needs exclusive access to the database, so the client is never running while it happens: the runner runs the client's own offline command in a one-shot container against the datadir. +The `db_compaction` option compacts the client database before the run measures anything. Compaction needs exclusive access to the database, so the client is never running while it happens: the runner runs an offline command — the client's own, or RocksDB's `ldb` for clients without one — in a one-shot container against the datadir. -Only clients that ship an offline compaction command support this. Today that is **geth** and **erigon**; every other client fails validation with a clear message. +Supported clients are **geth**, **erigon**, **nethermind** and **besu**; every other client fails validation with a clear message. reth needs no compaction: MDBX is persisted every block. -| Client | Compaction | Inspection | -|--------|------------|------------| -| geth | `geth db compact` | `geth db inspect` | -| erigon | `erigon db compact` | `erigon seg du --verbose` | +| Client | Compaction | Inspection | Check after | +|--------|------------|------------|-------------| +| geth | `geth db compact` | `geth db inspect` | - | +| erigon | `erigon db compact` | `erigon seg du --verbose` | - | +| nethermind, besu | RocksDB `ldb compact`, every column family of every database | - | level 0 of every column family is empty | `erigon db compact` rewrites every mdbx database of the datadir without its free pages. It needs **Erigon 3.7.0-dev or newer** (September 2026); an older binary, including every pinned glamsterdam-devnet image, fails the step with `command db not found`. > **On erigon, `before_benchmarks` needs `--exec.no-prune` on the instance.** That phase stops and restarts the client, and erigon refuses to reopen a datadir whose receipt domain it pruned past the snapshot files: `[snapshots] gap between snapshot files and DB for domain receipt: files end at txNum 0 but the DB was pruned up to 390625`. A synthetic snapshot has no snapshot files, so the client's own pruning is enough to create the gap. `--exec.no-prune` disables the state-aggregator pruning that advances the marker. Erigon documents the flag for diagnostic and perf-comparison use, which is what a benchmark is, and it removes housekeeping the benchmark does not want anyway. `before_pre_runs` needs no flag: it compacts before the client ever boots, so nothing restarts. +###### nethermind and besu: RocksDB `ldb` + +Neither client ships an offline compaction command, so their databases are compacted with RocksDB's own `ldb`, run from a separate image (`ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1`, built from [`Dockerfile.rocksdb-ldb`](../Dockerfile.rocksdb-ldb); `image` overrides it). Every directory under the datadir holding an `OPTIONS-*` file is a database — nethermind has one per store (state, code, blocks, receipts, ...), besu one with a column family per segment — and each of its column families is compacted with `--try_load_options`, i.e. with the options the client wrote it with. `extra_args` reach every `ldb compact`. + +RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databases open with it too. Nethermind 2.1.0 and Besu 26.9.0 each reopened a compacted mainnet-genesis datadir and served genesis block and state from it. Two guards make a wrong compaction fail rather than pass: + +- A database whose `OPTIONS` names a merge operator `ldb` does not have (e.g. nethermind's log index, off by default) is refused: `ldb` would merge its operands with a string-append operator instead. +- `ldb compact` falls back to the default column family for a name it does not know, so after each family its level 0 must be empty. + ###### Preparation steps (`prepare`) A client can offer steps that run before the compaction to make it reclaim more. None run unless `prepare` names them, because a step that helps one datadir can ruin another. @@ -1166,8 +1176,6 @@ On a **real synced erigon datadir** this is the pairing erigon documents, and it > **Do not enable `seg-retire` for a state-actor or otherwise synthetic snapshot.** Its history is far shorter than one step, so the retire finds nothing to freeze but prunes anyway, and erigon then refuses to reopen its own datadir with the gap error above. Verified in CI on a snapshot advanced to block 39: `retiring blocks from=0 to=39`, `Build state history snapshots` (nothing to build), `Prune state history` (marker advances regardless). The compaction alone is worth running there — it took that datadir's `chaindata` from 2.0GB to 32MB. -Besu was checked and cannot be supported yet: as of Besu 26.6.1, `besu storage` has no compaction subcommand. `trie-log prune` deletes trie logs below the retention limit instead of rewriting the database, which is a different operation and removes history the Bonsai rollback needs. - ```yaml runner: client: @@ -1187,7 +1195,7 @@ runner: | `inspect` | bool | No | `true` | Run the client database inspection before and after each compaction. A failed inspection is logged and never fails the run | | `prepare` | []string | No | - | Client preparation steps to run before each compaction, in order (see [Preparation steps](#preparation-steps-prepare)) | | `timeout` | string | No | `3h` | Cap for one phase's work — every preparation step, the compaction, and both inspections (Go duration). Applies per phase | -| `image` | string | No | the instance image | Image of the compaction container. The default keeps the tool version and the client version identical | +| `image` | string | No | the instance image; the `ldb` image for nethermind and besu | Image of the compaction container. The default keeps the tool version and the client version identical | | `extra_args` | []string | No | - | Extra arguments for the compaction command, e.g. `--cache=16384` | | `continue_on_error` | bool | No | `false` | Downgrade a compaction failure to a warning. A failed compaction makes the results incomparable, so the run fails by default | | `skip_if_marked` | bool | No | `true` with `persist`, else `false` | Skip a phase the datadir marker already names (see below) | diff --git a/pkg/client/besu.go b/pkg/client/besu.go index 154ceeb87..d42797c09 100644 --- a/pkg/client/besu.go +++ b/pkg/client/besu.go @@ -117,18 +117,18 @@ func (s *besuSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands returns nil: Besu ships no offline compaction command. +// DBMaintenanceCommands compacts Besu's database with RocksDB's ldb: Besu +// ships no offline compaction command. // // Checked against Besu 26.6.1. `besu storage` offers revert-variables, // rocksdb {usage, x-stats}, trie-log {count, prune, export, import}, // revert-metadata and prune-pre-merge-blocks, and no --Xplugin-rocksdb-* -// option compacts either. `trie-log prune` is the nearest thing operators -// reach for, but it DELETES trie logs below the retention limit rather than -// rewriting the SSTs — a different operation, and one that takes away the -// history the Bonsai rollback needs between tests. +// option compacts either. `trie-log prune` DELETES trie logs below the +// retention limit rather than rewriting the SSTs, which takes away the history +// the Bonsai rollback needs between tests. // -// `besu --data-path= storage rocksdb usage` would serve as the Inspect -// side, but Compact is what makes a client supported, so this stays nil. -func (s *besuSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { - return nil +// Besu keeps one RocksDB directory with a column family per segment, named by +// a single byte (0x01..0x12). +func (s *besuSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { + return rocksDBCompactCommands(dataDir) } diff --git a/pkg/client/client.go b/pkg/client/client.go index a5dee442d..2cd065ec2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -76,6 +76,11 @@ type DBMaintenanceCommands struct { // compaction. Optional: a client that can compact but not inspect leaves // it nil. Inspect []string + + // CompactImage runs Compact in this image instead of the client's, with + // Compact[0] as the entrypoint. It is set for a client that ships no + // offline compactor and is compacted by a separate tool. + CompactImage string } // Spec provides client-specific container configuration. diff --git a/pkg/client/nethermind.go b/pkg/client/nethermind.go index db7599b8d..833f53e70 100644 --- a/pkg/client/nethermind.go +++ b/pkg/client/nethermind.go @@ -106,8 +106,9 @@ func (s *nethermindSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands returns nil; benchmarkoor has no offline -// compaction command for Nethermind yet. -func (s *nethermindSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { - return nil +// DBMaintenanceCommands compacts Nethermind's databases with RocksDB's ldb: +// Nethermind ships no offline compaction command. Each database (state, code, +// blocks, receipts, ...) is its own RocksDB directory under the datadir. +func (s *nethermindSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { + return rocksDBCompactCommands(dataDir) } diff --git a/pkg/client/rocksdb.go b/pkg/client/rocksdb.go new file mode 100644 index 000000000..8d50c4ee7 --- /dev/null +++ b/pkg/client/rocksdb.go @@ -0,0 +1,21 @@ +package client + +import _ "embed" + +// RocksDBLdbImage is the RocksDB `ldb` tool that compacts nethermind and besu, +// built from Dockerfile.rocksdb-ldb. 11.8.1 is the RocksDB nethermind binds; +// besu's rocksdbjni 10.6.2 databases open with it too. +const RocksDBLdbImage = "ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1" + +//go:embed rocksdb_compact.sh +var rocksDBCompactScript string + +// rocksDBCompactCommands compacts every column family of every RocksDB +// database under dataDir with ldb, using the options each was written with. +// db_compaction.extra_args reach every `ldb compact`. +func rocksDBCompactCommands(dataDir string) *DBMaintenanceCommands { + return &DBMaintenanceCommands{ + Compact: []string{"sh", "-c", rocksDBCompactScript, "rocksdb-compact", dataDir}, + CompactImage: RocksDBLdbImage, + } +} diff --git a/pkg/client/rocksdb_compact.sh b/pkg/client/rocksdb_compact.sh new file mode 100644 index 000000000..ffa871755 --- /dev/null +++ b/pkg/client/rocksdb_compact.sh @@ -0,0 +1,69 @@ +# Compacts every column family of every RocksDB database under a datadir, with +# the options each database was written with, for clients that ship no offline +# compactor (nethermind, besu). The client must be stopped. +# +# sh -c "$(cat rocksdb_compact.sh)" rocksdb-compact [ldb option]... +# +# Extra arguments are passed to every `ldb compact`. +set -eu + +[ $# -ge 1 ] || { echo "usage: rocksdb-compact [ldb option]..." >&2; exit 2; } +root=$1 +shift + +nl=' +' + +# A column family name is printed as hex when it is not printable: besu names +# its families by single bytes (0x01..0x12), one of which is a newline. +label() { + case $1 in + *[![:print:]]* | '') printf '0x%s' "$(printf %s "$1" | od -An -tx1 | tr -d ' \n')" ;; + *) printf %s "$1" ;; + esac +} + +dbs=$(find "$root" -type f -name 'OPTIONS-*' -exec dirname {} \; | sort -u) +[ -n "$dbs" ] || { echo "no RocksDB database (no OPTIONS-* file) under $root" >&2; exit 1; } + +echo "$dbs" | while IFS= read -r db; do + # ldb opens a family that has no merge operator with a string-append one + # (and records it in OPTIONS), so a family with a merge operator of its own + # would have its operands merged wrongly. + opts=$(ls "$db"/OPTIONS-* | sort -V | tail -n 1) + foreign=$(grep -E '^[[:space:]]*merge_operator=' "$opts" | + grep -vxE '[[:space:]]*merge_operator=(nullptr|\{id=StringAppendOperator;delimiter=\\:;\})' || true) + if [ -n "$foreign" ]; then + echo "$db: refusing to compact: $opts sets a merge operator ldb does not have:$nl$foreign" >&2 + exit 1 + fi + + # ldb prints `Column families in : ` and then `{a, b, c}`, names raw, + # and exits 0 even when it cannot read the database. + out=$(ldb --db="$db" list_column_families &2; exit 1 ;; + esac + cfs=${out#*": $nl{"} + cfs=${cfs%"}"} + + while :; do + case $cfs in + *", "*) cf=${cfs%%", "*}; cfs=${cfs#*", "} ;; + *) cf=$cfs; cfs= ;; + esac + + echo "compacting $db column_family=$(label "$cf")" + ldb --db="$db" --try_load_options --column_family="$cf" "$@" compact &2; exit 1; } + + [ -n "$cfs" ] || break + done +done diff --git a/pkg/client/rocksdb_test.go b/pkg/client/rocksdb_test.go new file mode 100644 index 000000000..ac1240d5d --- /dev/null +++ b/pkg/client/rocksdb_test.go @@ -0,0 +1,189 @@ +package client + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRocksDBClientsCompactWithLdb(t *testing.T) { + for _, ct := range []ClientType{ClientNethermind, ClientBesu} { + spec, err := NewRegistry().Get(ct) + require.NoError(t, err) + + cmds := spec.DBMaintenanceCommands("/data") + require.NotNil(t, cmds, ct) + + assert.Equal(t, RocksDBLdbImage, cmds.CompactImage, ct) + assert.Equal(t, []string{"sh", "-c", rocksDBCompactScript, "rocksdb-compact", "/data"}, cmds.Compact, ct) + assert.Empty(t, cmds.Prepare, ct) + assert.Empty(t, cmds.Inspect, ct) + assert.True(t, SupportsDBCompaction(ct), ct) + } +} + +// stubLdb answers list_column_families with `listing` (raw bytes, as ldb +// prints them), get_property with `level0`, and records every compact argv, +// NUL-separated with one line per call, in $LDB_CALLS. +const stubLdb = `#!/bin/sh +for a in "$@"; do + case $a in + list_column_families) printf '%s' "$LDB_LISTING"; exit 0 ;; + get_property) echo "rocksdb.num-files-at-level0: $LDB_LEVEL0"; exit 0 ;; + compact) for b in "$@"; do printf '%s\0' "$b"; done >>"$LDB_CALLS"; echo >>"$LDB_CALLS"; exit 0 ;; + esac +done +exit 3 +` + +type ldbStub struct { + datadir string + calls string + env []string +} + +func newLdbStub(t *testing.T, listing string, dbs ...string) *ldbStub { + t.Helper() + + bin := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(bin, "ldb"), []byte(stubLdb), 0o755)) + + s := &ldbStub{datadir: t.TempDir(), calls: filepath.Join(t.TempDir(), "calls")} + + for _, db := range dbs { + dir := filepath.Join(s.datadir, db) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "OPTIONS-000007"), []byte("[CFOptions \"default\"]\n merge_operator=nullptr\n"), 0o644)) + } + + s.env = append(os.Environ(), + "PATH="+bin+":"+os.Getenv("PATH"), + "LDB_LISTING="+listing, + "LDB_LEVEL0=0", + "LDB_CALLS="+s.calls, + ) + + return s +} + +func (s *ldbStub) run(t *testing.T, args ...string) (string, error) { + t.Helper() + + cmd := exec.Command("sh", append([]string{"-c", rocksDBCompactScript, "rocksdb-compact", s.datadir}, args...)...) + cmd.Env = s.env + + out, err := cmd.CombinedOutput() + + return string(out), err +} + +// compacted returns the --column_family of every compact call, in order. +func (s *ldbStub) compacted(t *testing.T) []string { + t.Helper() + + data, err := os.ReadFile(s.calls) + require.NoError(t, err) + + var cfs []string + + for _, call := range bytes.Split(bytes.TrimSuffix(data, []byte("\x00\n")), []byte("\x00\n")) { + for _, arg := range strings.Split(string(call), "\x00") { + if cf, ok := strings.CutPrefix(arg, "--column_family="); ok { + cfs = append(cfs, cf) + } + } + } + + return cfs +} + +// TestRocksDBCompactScript_ParsesColumnFamilies feeds the script ldb's real +// output shape: a header line, then `{a, b, c}` with names printed raw. Besu +// names its families by single bytes, so one of them is a newline and another +// a tab. +func TestRocksDBCompactScript_ParsesColumnFamilies(t *testing.T) { + besu := []string{"default", "\x01", "\x09", "\x0a", "\x12"} + s := newLdbStub(t, "Column families in /data/database: \n{"+strings.Join(besu, ", ")+"}\n", "database") + + out, err := s.run(t) + require.NoError(t, err, out) + + assert.Equal(t, besu, s.compacted(t)) + assert.Contains(t, out, "column_family=0x0a") + assert.Contains(t, out, "column_family=default") +} + +func TestRocksDBCompactScript_EveryDatabase(t *testing.T) { + s := newLdbStub(t, "Column families in x: \n{default, Blocks, with space}\n", + "nethermind_db/mainnet/state/0", "nethermind_db/mainnet/receipts") + + out, err := s.run(t, "--max_open_files=-1") + require.NoError(t, err, out) + + assert.Equal(t, []string{ + "default", "Blocks", "with space", // receipts + "default", "Blocks", "with space", // state/0 + }, s.compacted(t)) + + data, err := os.ReadFile(s.calls) + require.NoError(t, err) + assert.Contains(t, string(data), "--max_open_files=-1\x00compact\x00", "extra args reach ldb") + assert.Contains(t, string(data), "--try_load_options\x00") +} + +func TestRocksDBCompactScript_Failures(t *testing.T) { + t.Run("no database", func(t *testing.T) { + s := newLdbStub(t, "") + + out, err := s.run(t) + require.Error(t, err) + assert.Contains(t, out, "no RocksDB database") + }) + + t.Run("unreadable database", func(t *testing.T) { + // ldb prints the error to stderr and exits 0. + s := newLdbStub(t, "", "db") + + out, err := s.run(t) + require.Error(t, err) + assert.Contains(t, out, "cannot list column families") + }) + + t.Run("level 0 not empty after compaction", func(t *testing.T) { + s := newLdbStub(t, "Column families in db: \n{default}\n", "db") + s.env = append(s.env, "LDB_LEVEL0=2") + + out, err := s.run(t) + require.Error(t, err) + assert.Contains(t, out, "rocksdb.num-files-at-level0: 2 after compaction") + }) + + t.Run("a merge operator ldb does not have", func(t *testing.T) { + s := newLdbStub(t, "Column families in db: \n{default}\n", "db") + opts := filepath.Join(s.datadir, "db", "OPTIONS-000012") + require.NoError(t, os.WriteFile(opts, []byte(" merge_operator=nullptr\n merge_operator=LogIndexMergeOperator\n"), 0o644)) + + out, err := s.run(t) + require.Error(t, err) + assert.Contains(t, out, "merge_operator=LogIndexMergeOperator") + assert.NoFileExists(t, s.calls, "nothing is compacted") + }) +} + +// ldb itself records its string-append operator in OPTIONS, so a database it +// has compacted before compacts again. +func TestRocksDBCompactScript_AcceptsLdbsOwnMergeOperator(t *testing.T) { + s := newLdbStub(t, "Column families in db: \n{default}\n", "db") + opts := filepath.Join(s.datadir, "db", "OPTIONS-000012") + require.NoError(t, os.WriteFile(opts, []byte(" merge_operator={id=StringAppendOperator;delimiter=\\:;}\n"), 0o644)) + + out, err := s.run(t) + require.NoError(t, err, out) + assert.Equal(t, []string{"default"}, s.compacted(t)) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 7a2a80d1e..fe4e5a1d4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1486,7 +1486,8 @@ const ( // the global one; it does not merge field by field. // // Only clients whose spec returns compaction commands support this. Today -// that is geth and erigon. +// that is geth, erigon, nethermind and besu; the last two have no offline +// compactor of their own and are compacted with RocksDB's ldb. type DBCompactionConfig struct { Enabled bool `yaml:"enabled" mapstructure:"enabled" json:"enabled"` @@ -1525,7 +1526,8 @@ type DBCompactionConfig struct { // Image overrides the image of the compaction container. Empty uses the // instance image, which keeps the tool version and the client version - // identical. + // identical, or for nethermind and besu the RocksDB ldb image + // (client.RocksDBLdbImage). Image string `yaml:"image,omitempty" mapstructure:"image" json:"image,omitempty"` // ExtraArgs appends arguments to the compaction command, e.g. diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 9d8e580db..bcffc2929 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -3763,7 +3763,7 @@ func TestValidateDBCompaction(t *testing.T) { }, { name: "unsupported client", - client: "besu", + client: "ethrex", cfg: &DBCompactionConfig{Enabled: true}, wantErr: true, errSubstr: "not supported for client", @@ -3997,7 +3997,7 @@ func TestValidateDBCompaction_SkipsInactiveInstances(t *testing.T) { Client: ClientConfig{ Config: ClientDefaults{DBCompaction: &DBCompactionConfig{Enabled: true}}, }, - Instances: []ClientInstance{{ID: "besu", Client: "besu"}}, + Instances: []ClientInstance{{ID: "ethrex", Client: "ethrex"}}, }, } diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index a2a06f487..bc4081e9c 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -168,6 +168,21 @@ func (req *dbCompactionRequest) image() string { return req.ImageName } +// compactImage returns the image of the compaction container: the tool image +// of a client compacted by a separate tool, unless db_compaction.image names +// another. +func (req *dbCompactionRequest) compactImage(cmds *client.DBMaintenanceCommands) string { + if req.Cfg != nil && req.Cfg.Image != "" { + return req.Cfg.Image + } + + if cmds.CompactImage != "" { + return cmds.CompactImage + } + + return req.ImageName +} + // hostPath returns the host path of the datadir mount, or "" when // the datadir is a container volume. The marker and the size measurements need // a path the runner can read. @@ -227,7 +242,7 @@ func (r *runner) runDBCompaction( report := &dbCompactionReport{ Phase: req.Phase, Client: req.Instance.Client, - Image: req.image(), + Image: req.compactImage(cmds), RunID: req.RunID, StartedAt: started.UTC().Format(time.RFC3339), Persisted: req.Persisting, @@ -380,11 +395,18 @@ func (r *runner) runDBCompactionContainers( ctx, cancel := context.WithTimeout(ctx, req.Cfg.EffectiveTimeout()) defer cancel() + compactImage := req.compactImage(cmds) + if compactImage != req.ImageName { + if err := r.containerMgr.PullImage(ctx, compactImage, "if-not-present"); err != nil { + return fmt.Errorf("pulling compaction image: %w", err) + } + } + if req.Cfg.InspectEnabled() && len(cmds.Inspect) > 0 { log.Info("Inspecting the database before compaction") if err := r.runDBMaintenanceContainer( - ctx, req, "inspect-before", cmds.Inspect, + ctx, req, "inspect-before", req.image(), req.Instance.Entrypoint, cmds.Inspect, filepath.Join(phaseDir, "inspect-before.txt"), ); err != nil { log.WithError(err).Warn("Database inspection before compaction failed") @@ -403,7 +425,7 @@ func (r *runner) runDBCompactionContainers( }).Info("Preparing the database for compaction") if err := r.runDBMaintenanceContainer( - ctx, req, step.Name, step.Args, + ctx, req, step.Name, req.image(), req.Instance.Entrypoint, step.Args, filepath.Join(phaseDir, step.Name+".log"), ); err != nil { return fmt.Errorf("preparation step %q: %w", step.Name, err) @@ -412,8 +434,13 @@ func (r *runner) runDBCompactionContainers( log.WithField("timeout", timeout).Info("Compacting the database") + entrypoint, command := req.Instance.Entrypoint, dbCompactionCommand(cmds, req.Cfg) + if cmds.CompactImage != "" { + entrypoint, command = command[:1], command[1:] + } + if err := r.runDBMaintenanceContainer( - ctx, req, "compact", dbCompactionCommand(cmds, req.Cfg), + ctx, req, "compact", compactImage, entrypoint, command, filepath.Join(phaseDir, "compact.log"), ); err != nil { return err @@ -423,7 +450,7 @@ func (r *runner) runDBCompactionContainers( log.Info("Inspecting the database after compaction") if err := r.runDBMaintenanceContainer( - ctx, req, "inspect-after", cmds.Inspect, + ctx, req, "inspect-after", req.image(), req.Instance.Entrypoint, cmds.Inspect, filepath.Join(phaseDir, "inspect-after.txt"), ); err != nil { log.WithError(err).Warn("Database inspection after compaction failed") @@ -441,8 +468,8 @@ func (r *runner) runDBCompactionContainers( func (r *runner) runDBMaintenanceContainer( ctx context.Context, req *dbCompactionRequest, - step string, - command []string, + step, image string, + entrypoint, command []string, outputFile string, ) error { name := fmt.Sprintf( @@ -451,8 +478,8 @@ func (r *runner) runDBMaintenanceContainer( spec := &docker.ContainerSpec{ Name: name, - Image: req.image(), - Entrypoint: req.Instance.Entrypoint, + Image: image, + Entrypoint: entrypoint, Command: command, Mounts: []docker.Mount{req.Mount}, NetworkName: r.cfg.ContainerNetwork, @@ -476,7 +503,7 @@ func (r *runner) runDBMaintenanceContainer( }() _, _ = fmt.Fprintf( - out, "# %s %s\n# %v\n\n", req.image(), step, command, + out, "# %s %s\n# %v\n\n", image, step, append(append([]string{}, entrypoint...), command...), ) var stdout, stderr io.Writer = out, out diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index ad95a2741..36b9abe87 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -191,7 +191,6 @@ func TestErigonDBMaintenanceCommands(t *testing.T) { func TestSupportsDBCompaction_UnsupportedClients(t *testing.T) { for _, other := range []client.ClientType{ - client.ClientBesu, client.ClientNethermind, client.ClientReth, client.ClientNimbus, client.ClientEthrex, } { assert.False(t, client.SupportsDBCompaction(other), string(other)) @@ -359,13 +358,22 @@ type fakeDBMaintenanceMgr struct { // failCommand fails any container whose command starts with this word. failCommand string - ran []string + ran []string + specs []*docker.ContainerSpec + pulled []string +} + +func (f *fakeDBMaintenanceMgr) PullImage(_ context.Context, image, _ string) error { + f.pulled = append(f.pulled, image) + + return nil } func (f *fakeDBMaintenanceMgr) RunInitContainer( _ context.Context, spec *docker.ContainerSpec, stdout, _ io.Writer, ) error { f.ran = append(f.ran, strings.Join(spec.Command, " ")) + f.specs = append(f.specs, spec) _, _ = fmt.Fprintln(stdout, "fake container output") @@ -713,3 +721,44 @@ func TestDBCompactionMarkerRecordsTheOverriddenImage(t *testing.T) { entry := marker.Phases[config.DBCompactionBeforePreRuns] assert.Equal(t, "erigontech/erigon:v3.7.0", entry.Image) } + +// TestRunDBCompactionContainers_ToolImage pins how a client compacted by a +// separate tool runs: the compaction in the tool image with Compact[0] as the +// entrypoint and extra_args after the rest, everything else in the client +// image, and the tool image pulled first. +func TestRunDBCompactionContainers_ToolImage(t *testing.T) { + mgr := &fakeDBMaintenanceMgr{} + r, resultsDir := dbCompactionTestRunner(t, mgr) + + cmds := &client.DBMaintenanceCommands{ + Compact: []string{"sh", "-c", "script", "rocksdb-compact", "/data"}, + Inspect: []string{"storage", "rocksdb", "usage"}, + CompactImage: "ldb:11.8.1", + } + + req := dbCompactionTestRequest(resultsDir) + req.Instance.Entrypoint = []string{"/opt/besu/bin/besu"} + req.Cfg.ExtraArgs = []string{"--max_open_files=-1"} + + require.NoError(t, r.runDBCompactionContainers( + context.Background(), req, cmds, nil, resultsDir, r.log, + )) + + assert.Equal(t, []string{"ldb:11.8.1"}, mgr.pulled) + require.Len(t, mgr.specs, 3) + + compact := mgr.specs[1] + assert.Equal(t, "ldb:11.8.1", compact.Image) + assert.Equal(t, []string{"sh"}, compact.Entrypoint) + assert.Equal(t, []string{"-c", "script", "rocksdb-compact", "/data", "--max_open_files=-1"}, compact.Command) + + for _, inspect := range []*docker.ContainerSpec{mgr.specs[0], mgr.specs[2]} { + assert.Equal(t, req.ImageName, inspect.Image) + assert.Equal(t, req.Instance.Entrypoint, inspect.Entrypoint) + } + + // db_compaction.image overrides the tool image too. + req.Cfg.Image = "my-ldb:dev" + assert.Equal(t, "my-ldb:dev", req.compactImage(cmds)) + assert.Equal(t, req.ImageName, (&dbCompactionRequest{ImageName: req.ImageName}).compactImage(&client.DBMaintenanceCommands{})) +} From 80034f620a302bda5ba4a4c84ddcb7d81d728e7f Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 8 Oct 2026 16:35:21 +0200 Subject: [PATCH 02/13] feat(client): fail a geth compaction whose journal holds more than 128 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. --- docs/configuration.md | 7 +- pkg/client/client.go | 4 + pkg/client/geth.go | 23 ++++ pkg/client/geth_journal.go | 229 +++++++++++++++++++++++++++++++ pkg/client/geth_journal_test.go | 220 +++++++++++++++++++++++++++++ pkg/client/rocksdb_test.go | 1 + pkg/config/config.go | 15 ++ pkg/runner/db_compaction.go | 30 ++++ pkg/runner/db_compaction_test.go | 35 +++++ 9 files changed, 563 insertions(+), 1 deletion(-) create mode 100644 pkg/client/geth_journal.go create mode 100644 pkg/client/geth_journal_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 312f73144..0fe233511 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1141,7 +1141,7 @@ Supported clients are **geth**, **erigon**, **nethermind** and **besu**; every o | Client | Compaction | Inspection | Check after | |--------|------------|------------|-------------| -| geth | `geth db compact` | `geth db inspect` | - | +| geth | `geth db compact` | `geth db inspect` | `triedb/merkle.journal` holds at most the last 128 blocks | | erigon | `erigon db compact` | `erigon seg du --verbose` | - | | nethermind, besu | RocksDB `ldb compact`, every column family of every database | - | level 0 of every column family is empty | @@ -1158,6 +1158,10 @@ RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databas - A database whose `OPTIONS` names a merge operator `ldb` does not have (e.g. nethermind's log index, off by default) is refused: `ldb` would merge its operands with a string-append operator instead. - `ldb compact` falls back to the default column family for a name it does not know, so after each family its level 0 must be empty. +###### geth: the journal check + +geth's path scheme keeps the last 128 blocks as in-memory diff layers and older, not yet flushed state in a write buffer (up to 256 MiB). A clean stop writes both to `/geth/triedb/merkle.journal`, and the next start loads them back into memory, so that state is never read from the database a compaction just rewrote. After the compaction the runner parses the journal (layout version 3, written by geth v1.17.7) and fails the phase if the write buffer is not empty or there are more than 128 diff layers. To pass, run geth with `--cache.gc=0` (a 0-byte buffer) for at least 128 blocks before it stops. `verify: false` skips the check; a datadir on a container volume is not checked. + ###### Preparation steps (`prepare`) A client can offer steps that run before the compaction to make it reclaim more. None run unless `prepare` names them, because a step that helps one datadir can ruin another. @@ -1196,6 +1200,7 @@ runner: | `prepare` | []string | No | - | Client preparation steps to run before each compaction, in order (see [Preparation steps](#preparation-steps-prepare)) | | `timeout` | string | No | `3h` | Cap for one phase's work — every preparation step, the compaction, and both inspections (Go duration). Applies per phase | | `image` | string | No | the instance image; the `ldb` image for nethermind and besu | Image of the compaction container. The default keeps the tool version and the client version identical | +| `verify` | bool | No | `true` | Run the client's check of the compacted datadir (geth's journal check) | | `extra_args` | []string | No | - | Extra arguments for the compaction command, e.g. `--cache=16384` | | `continue_on_error` | bool | No | `false` | Downgrade a compaction failure to a warning. A failed compaction makes the results incomparable, so the run fails by default | | `skip_if_marked` | bool | No | `true` with `persist`, else `false` | Skip a phase the datadir marker already names (see below) | diff --git a/pkg/client/client.go b/pkg/client/client.go index 2cd065ec2..1b4a2721c 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -81,6 +81,10 @@ type DBMaintenanceCommands struct { // Compact[0] as the entrypoint. It is set for a client that ships no // offline compactor and is compacted by a separate tool. CompactImage string + + // Verify checks the datadir after the compaction, given its host path, and + // returns a one-line summary of what it found. Optional. + Verify func(hostDataDir string) (string, error) } // Spec provides client-specific container configuration. diff --git a/pkg/client/geth.go b/pkg/client/geth.go index dfbeb4d78..4ab0266fe 100644 --- a/pkg/client/geth.go +++ b/pkg/client/geth.go @@ -1,5 +1,7 @@ package client +import "fmt" + type gethSpec struct{} // NewGethSpec creates a new Geth client specification. @@ -120,9 +122,30 @@ func (s *gethSpec) SnapshotPrepareArgs() []string { // lock, so a running node makes it fail. The commands take --datadir rather // than inheriting the one in DefaultCommand, since a datadir config may mount // the data somewhere other than /data. +// +// Verify fails a datadir whose path-scheme journal holds more than the last +// GethMaxDiffLayers blocks: geth loads that state into memory on start, so +// the compaction never reaches it. func (s *gethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { return &DBMaintenanceCommands{ Compact: []string{"db", "compact", "--datadir=" + dataDir}, Inspect: []string{"db", "inspect", "--datadir=" + dataDir}, + Verify: verifyGethJournal, + } +} + +func verifyGethJournal(hostDataDir string) (string, error) { + j, err := CheckGethJournal(hostDataDir) + if err != nil { + return "", err + } + + if j == nil { + return "no " + GethJournalFile + ": no state is loaded into memory on start", nil } + + return fmt.Sprintf( + "%s: %d diff layers (blocks %d-%d), write buffer empty", + GethJournalFile, j.DiffLayers, j.FirstBlock, j.LastBlock, + ), nil } diff --git a/pkg/client/geth_journal.go b/pkg/client/geth_journal.go new file mode 100644 index 000000000..0c5187a7b --- /dev/null +++ b/pkg/client/geth_journal.go @@ -0,0 +1,229 @@ +package client + +import ( + "bufio" + "errors" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/ethereum/go-ethereum/rlp" +) + +const ( + // GethJournalFile is where geth's path-scheme state database writes its + // in-memory layers on a clean stop, relative to the datadir. The next start + // loads them back into memory. + GethJournalFile = "geth/triedb/merkle.journal" + + // gethJournalVersion is the only journal layout ReadGethJournal parses + // (triedb/pathdb journalVersion, written by geth v1.17.7). + gethJournalVersion = 3 + + // GethMaxDiffLayers is the number of recent blocks geth keeps as diff + // layers (triedb/pathdb maxDiffLayers). + GethMaxDiffLayers = 128 +) + +// GethJournal summarises a path-scheme journal: the diff layers, one per +// block, and the disk layer's write buffer, which holds the state of every +// older block that has not been flushed to the database yet. +type GethJournal struct { + DiffLayers int + FirstBlock uint64 + LastBlock uint64 + + // What the write buffer holds: tries with dirty nodes, accounts, and + // accounts with dirty storage slots. All zero when it is empty. + BufferTries int + BufferAccounts int + BufferStorages int +} + +// BufferEmpty reports whether everything below the diff layers is on disk. +func (j *GethJournal) BufferEmpty() bool { + return j.BufferTries == 0 && j.BufferAccounts == 0 && j.BufferStorages == 0 +} + +// CheckGethJournal fails when the journal in dataDir holds the state of more +// than the last GethMaxDiffLayers blocks, i.e. when the write buffer below the +// diff layers is not empty: geth would serve that state from memory, so a +// compaction never reaches it. A datadir without a journal passes, and the +// returned journal is nil. +func CheckGethJournal(dataDir string) (*GethJournal, error) { + path := filepath.Join(dataDir, GethJournalFile) + + j, err := ReadGethJournal(path) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + + if err != nil { + return nil, err + } + + if j.DiffLayers > GethMaxDiffLayers || !j.BufferEmpty() { + return j, fmt.Errorf( + "%s holds the state of more than the last %d blocks, which geth loads back into memory"+ + " on start: %d diff layers (blocks %d-%d) over a write buffer of %d tries,"+ + " %d accounts and %d storages; flush it by running geth with --cache.gc=0 for"+ + " at least %d blocks before stopping it", + path, GethMaxDiffLayers, j.DiffLayers, j.FirstBlock, j.LastBlock, + j.BufferTries, j.BufferAccounts, j.BufferStorages, GethMaxDiffLayers, + ) + } + + return j, nil +} + +// ReadGethJournal parses a path-scheme journal (triedb/pathdb/journal.go): +// the version, the disk root, the disk layer (root, state id, nodes, states) +// and then one entry per diff layer, oldest first. +func ReadGethJournal(path string) (*GethJournal, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + + defer func() { _ = f.Close() }() + + s := rlp.NewStream(bufio.NewReaderSize(f, 1<<20), 0) + + version, err := s.Uint64() + if err != nil { + return nil, fmt.Errorf("%s: reading version: %w", path, err) + } + + if version != gethJournalVersion { + return nil, fmt.Errorf( + "%s: journal version %d, only %d can be checked", path, version, gethJournalVersion, + ) + } + + j := &GethJournal{} + + // Disk root, then the disk layer's root and state id. + for _, what := range []string{"disk root", "disk layer root"} { + if _, err := s.Bytes(); err != nil { + return nil, fmt.Errorf("%s: reading %s: %w", path, what, err) + } + } + + if _, err := s.Uint64(); err != nil { + return nil, fmt.Errorf("%s: reading disk layer id: %w", path, err) + } + + if j.BufferTries, err = listLen(s); err != nil { + return nil, fmt.Errorf("%s: reading buffered nodes: %w", path, err) + } + + if j.BufferAccounts, j.BufferStorages, err = readStateSet(s); err != nil { + return nil, fmt.Errorf("%s: reading buffered states: %w", path, err) + } + + for { + if _, err := s.Bytes(); errors.Is(err, io.EOF) { + return j, nil + } else if err != nil { + return nil, fmt.Errorf("%s: reading diff layer %d root: %w", path, j.DiffLayers, err) + } + + block, err := s.Uint64() + if err != nil { + return nil, fmt.Errorf("%s: reading diff layer %d block: %w", path, j.DiffLayers, err) + } + + if err := skipDiffLayerBody(s); err != nil { + return nil, fmt.Errorf("%s: reading diff layer %d (block %d): %w", path, j.DiffLayers, block, err) + } + + if j.DiffLayers == 0 { + j.FirstBlock = block + } + + j.LastBlock = block + j.DiffLayers++ + } +} + +// skipDiffLayerBody consumes a diff layer's nodes, their optional origins +// (a list where the state set's leading bool would be), its state set and +// the state set's origins. +func skipDiffLayerBody(s *rlp.Stream) error { + if _, err := s.Raw(); err != nil { + return fmt.Errorf("nodes: %w", err) + } + + kind, _, err := s.Kind() + if err != nil { + return err + } + + if kind == rlp.List { + if _, err := s.Raw(); err != nil { + return fmt.Errorf("node origins: %w", err) + } + } + + if _, _, err := readStateSet(s); err != nil { + return err + } + + for _, what := range []string{"account origins", "storage origins"} { + if _, err := s.Raw(); err != nil { + return fmt.Errorf("%s: %w", what, err) + } + } + + return nil +} + +// readStateSet consumes a stateSet (raw-key flag, accounts, storages) and +// returns how many accounts and storages it holds. +func readStateSet(s *rlp.Stream) (accounts, storages int, err error) { + if _, err := s.Bool(); err != nil { + return 0, 0, fmt.Errorf("raw storage key flag: %w", err) + } + + // struct{ AddrHashes []common.Hash; Accounts [][]byte } + raw, err := s.Raw() + if err != nil { + return 0, 0, fmt.Errorf("accounts: %w", err) + } + + fields, _, err := rlp.SplitList(raw) + if err != nil { + return 0, 0, fmt.Errorf("accounts: %w", err) + } + + _, hashes, _, err := rlp.Split(fields) + if err != nil { + return 0, 0, fmt.Errorf("account hashes: %w", err) + } + + if accounts, err = rlp.CountValues(hashes); err != nil { + return 0, 0, fmt.Errorf("account hashes: %w", err) + } + + if storages, err = listLen(s); err != nil { + return 0, 0, fmt.Errorf("storages: %w", err) + } + + return accounts, storages, nil +} + +// listLen consumes a list and returns its number of elements. +func listLen(s *rlp.Stream) (int, error) { + raw, err := s.Raw() + if err != nil { + return 0, err + } + + content, _, err := rlp.SplitList(raw) + if err != nil { + return 0, err + } + + return rlp.CountValues(content) +} diff --git a/pkg/client/geth_journal_test.go b/pkg/client/geth_journal_test.go new file mode 100644 index 000000000..8acd3b40f --- /dev/null +++ b/pkg/client/geth_journal_test.go @@ -0,0 +1,220 @@ +package client + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/rlp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The types below mirror what geth v1.17.7's triedb/pathdb journal() writes, +// so the tests build journals the way geth does. + +type testJournalNode struct { + Path []byte + Blob []byte +} + +type testJournalNodes struct { + Owner common.Hash + Nodes []testJournalNode +} + +type testAccounts struct { + AddrHashes []common.Hash + Accounts [][]byte +} + +type testStorage struct { + AddrHash common.Hash + Keys []common.Hash + Vals [][]byte +} + +type testOriginAccounts struct { + Addresses []common.Address + Accounts [][]byte +} + +type testOriginStorage struct { + Address common.Address + Keys []common.Hash + Vals [][]byte +} + +type testLayerState struct { + nodes []testJournalNodes + accounts testAccounts + storages []testStorage +} + +func dirtyState(i byte) testLayerState { + return testLayerState{ + nodes: []testJournalNodes{{Nodes: []testJournalNode{{Path: []byte{i}, Blob: []byte{0xc0}}}}}, + accounts: testAccounts{AddrHashes: []common.Hash{{i}}, Accounts: [][]byte{{i}}}, + storages: []testStorage{{AddrHash: common.Hash{i}, Keys: []common.Hash{{1}}, Vals: [][]byte{{2}}}}, + } +} + +func emptyState() testLayerState { + return testLayerState{nodes: []testJournalNodes{}, accounts: testAccounts{}, storages: []testStorage{}} +} + +type testJournal struct { + version uint64 + buffer testLayerState + diffs int + noOrigin bool // pre-origin layout: no node origins after the nodes +} + +func (j testJournal) encode(t *testing.T) []byte { + t.Helper() + + var b bytes.Buffer + + enc := func(v any) { require.NoError(t, rlp.Encode(&b, v)) } + + state := func(s testLayerState) { + enc(false) + enc(s.accounts) + enc(s.storages) + } + + enc(j.version) + enc(common.Hash{0xd1}) // disk root + enc(common.Hash{0xd1}) // disk layer root + enc(uint64(1000)) // disk layer state id + enc(j.buffer.nodes) + state(j.buffer) + + for i := range j.diffs { + s := dirtyState(byte(i)) + + enc(common.Hash{byte(i), 0xff}) + enc(uint64(500 + i)) + enc(s.nodes) + + if !j.noOrigin { + enc(s.nodes) + } + + state(s) + enc(testOriginAccounts{Addresses: []common.Address{{byte(i)}}, Accounts: [][]byte{{}}}) + enc([]testOriginStorage{{Address: common.Address{byte(i)}, Keys: []common.Hash{{1}}, Vals: [][]byte{{}}}}) + } + + return b.Bytes() +} + +func writeJournal(t *testing.T, data []byte) string { + t.Helper() + + dir := t.TempDir() + path := filepath.Join(dir, GethJournalFile) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, data, 0o644)) + + return dir +} + +func TestCheckGethJournal(t *testing.T) { + t.Run("the last 128 blocks over an empty buffer pass", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 128}.encode(t)) + + j, err := CheckGethJournal(dir) + require.NoError(t, err) + assert.Equal(t, &GethJournal{DiffLayers: 128, FirstBlock: 500, LastBlock: 627}, j) + }) + + t.Run("fewer blocks than that pass", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 3}.encode(t)) + + j, err := CheckGethJournal(dir) + require.NoError(t, err) + assert.Equal(t, 3, j.DiffLayers) + }) + + t.Run("a disk layer alone passes", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState()}.encode(t)) + + j, err := CheckGethJournal(dir) + require.NoError(t, err) + assert.Zero(t, j.DiffLayers) + }) + + t.Run("an unflushed write buffer fails", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: dirtyState(7), diffs: 128}.encode(t)) + + j, err := CheckGethJournal(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "128 diff layers (blocks 500-627) over a write buffer of 1 tries, 1 accounts and 1 storages") + assert.Equal(t, 1, j.BufferTries) + }) + + t.Run("a buffer holding only accounts fails", func(t *testing.T) { + buffer := emptyState() + buffer.accounts = dirtyState(1).accounts + dir := writeJournal(t, testJournal{version: 3, buffer: buffer, diffs: 1}.encode(t)) + + _, err := CheckGethJournal(dir) + require.Error(t, err) + }) + + t.Run("more than 128 diff layers fail", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 129}.encode(t)) + + _, err := CheckGethJournal(dir) + require.ErrorContains(t, err, "129 diff layers") + }) + + t.Run("diff layers without node origins parse", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 5, noOrigin: true}.encode(t)) + + j, err := CheckGethJournal(dir) + require.NoError(t, err) + assert.Equal(t, 5, j.DiffLayers) + }) + + t.Run("no journal passes", func(t *testing.T) { + j, err := CheckGethJournal(t.TempDir()) + require.NoError(t, err) + assert.Nil(t, j) + }) + + t.Run("another journal version is not guessed at", func(t *testing.T) { + dir := writeJournal(t, testJournal{version: 2, buffer: emptyState()}.encode(t)) + + _, err := CheckGethJournal(dir) + require.ErrorContains(t, err, "journal version 2, only 3 can be checked") + }) + + t.Run("a truncated journal fails", func(t *testing.T) { + data := testJournal{version: 3, buffer: emptyState(), diffs: 2}.encode(t) + dir := writeJournal(t, data[:len(data)-5]) + + _, err := CheckGethJournal(dir) + require.ErrorContains(t, err, "diff layer 1") + }) +} + +func TestGethVerifiesItsJournal(t *testing.T) { + spec, err := NewRegistry().Get(ClientGeth) + require.NoError(t, err) + + verify := spec.DBMaintenanceCommands("/data").Verify + require.NotNil(t, verify) + + summary, err := verify(t.TempDir()) + require.NoError(t, err) + assert.Contains(t, summary, "no geth/triedb/merkle.journal") + + dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 128}.encode(t)) + summary, err = verify(dir) + require.NoError(t, err) + assert.Equal(t, "geth/triedb/merkle.journal: 128 diff layers (blocks 500-627), write buffer empty", summary) +} diff --git a/pkg/client/rocksdb_test.go b/pkg/client/rocksdb_test.go index ac1240d5d..8b78fed79 100644 --- a/pkg/client/rocksdb_test.go +++ b/pkg/client/rocksdb_test.go @@ -24,6 +24,7 @@ func TestRocksDBClientsCompactWithLdb(t *testing.T) { assert.Equal(t, []string{"sh", "-c", rocksDBCompactScript, "rocksdb-compact", "/data"}, cmds.Compact, ct) assert.Empty(t, cmds.Prepare, ct) assert.Empty(t, cmds.Inspect, ct) + assert.Nil(t, cmds.Verify, ct) assert.True(t, SupportsDBCompaction(ct), ct) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index fe4e5a1d4..d6175e7b1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1530,6 +1530,11 @@ type DBCompactionConfig struct { // (client.RocksDBLdbImage). Image string `yaml:"image,omitempty" mapstructure:"image" json:"image,omitempty"` + // Verify runs the client's check of the compacted datadir. geth's fails + // when triedb/merkle.journal holds the state of more than the last 128 + // blocks, which geth would load back into memory on start. Default: true. + Verify *bool `yaml:"verify,omitempty" mapstructure:"verify" json:"verify,omitempty"` + // ExtraArgs appends arguments to the compaction command, e.g. // ["--cache=16384"] for geth. ExtraArgs []string `yaml:"extra_args,omitempty" mapstructure:"extra_args" json:"extra_args,omitempty"` @@ -1682,6 +1687,16 @@ func (c *DBCompactionConfig) InspectEnabled() bool { return *c.Inspect } +// VerifyEnabled reports whether to run the client's check of the compacted +// datadir. Defaults to true. +func (c *DBCompactionConfig) VerifyEnabled() bool { + if c == nil || c.Verify == nil { + return true + } + + return *c.Verify +} + // PrepareSteps returns the preparation steps to run before each compaction, in // the configured order. Nil means none, which is the default. func (c *DBCompactionConfig) PrepareSteps() []string { diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index bc4081e9c..3bbc32373 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -446,6 +446,10 @@ func (r *runner) runDBCompactionContainers( return err } + if err := verifyDBCompaction(req, cmds, log); err != nil { + return err + } + if req.Cfg.InspectEnabled() && len(cmds.Inspect) > 0 { log.Info("Inspecting the database after compaction") @@ -460,6 +464,32 @@ func (r *runner) runDBCompactionContainers( return nil } +// verifyDBCompaction runs the client's check of the compacted datadir. It +// needs the datadir on the host, so a container volume is not checked. +func verifyDBCompaction( + req *dbCompactionRequest, cmds *client.DBMaintenanceCommands, log logrus.FieldLogger, +) error { + if cmds.Verify == nil || !req.Cfg.VerifyEnabled() { + return nil + } + + hostPath := req.hostPath() + if hostPath == "" { + log.Warn("Not verifying the compacted database: the datadir is a container volume") + + return nil + } + + summary, err := cmds.Verify(hostPath) + if err != nil { + return fmt.Errorf("verifying the compacted database: %w", err) + } + + log.WithField("result", summary).Info("Verified the compacted database") + + return nil +} + // runDBMaintenanceContainer runs one database command in a one-shot container // on the datadir mount and writes its output to outputFile. // diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index 36b9abe87..4b6b737b0 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -762,3 +762,38 @@ func TestRunDBCompactionContainers_ToolImage(t *testing.T) { assert.Equal(t, "my-ldb:dev", req.compactImage(cmds)) assert.Equal(t, req.ImageName, (&dbCompactionRequest{ImageName: req.ImageName}).compactImage(&client.DBMaintenanceCommands{})) } + +// TestRunDBCompactionContainers_Verify checks that a failed verification fails +// the phase before the inspection after it, that verify: false skips it, and +// that a volume datadir is not verified. +func TestRunDBCompactionContainers_Verify(t *testing.T) { + var checked []string + + cmds := &client.DBMaintenanceCommands{ + Compact: []string{"db", "compact"}, + Inspect: []string{"db", "inspect"}, + Verify: func(dir string) (string, error) { + checked = append(checked, dir) + + return "", fmt.Errorf("journal holds 1354 blocks") + }, + } + + mgr := &fakeDBMaintenanceMgr{} + r, resultsDir := dbCompactionTestRunner(t, mgr) + req := dbCompactionTestRequest(resultsDir) + + err := r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log) + require.ErrorContains(t, err, "journal holds 1354 blocks") + assert.Equal(t, []string{resultsDir}, checked, "the host path of the datadir") + assert.Equal(t, []string{"db inspect", "db compact"}, mgr.ran) + + off := false + req.Cfg.Verify = &off + require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) + + req.Cfg.Verify = nil + req.Mount = docker.Mount{Type: "volume", Source: "vol", Target: "/data"} + require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) + assert.Len(t, checked, 1) +} From 125db09e4c07f26434d7fccbceb01d320666e886 Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 8 Oct 2026 16:35:34 +0200 Subject: [PATCH 03/13] feat(cmd): add `benchmarkoor db compact` to compact a datadir outside a run benchmarkoor db compact --client \ --datadir [--prepare ]... 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. --- cmd/benchmarkoor/db.go | 114 +++++++++++++++++++++++++++ docs/configuration.md | 10 +++ pkg/runner/db_compaction.go | 95 ++++++++++++++++++++++ pkg/runner/db_compaction_ldb_test.go | 111 ++++++++++++++++++++++++++ 4 files changed, 330 insertions(+) create mode 100644 cmd/benchmarkoor/db.go create mode 100644 pkg/runner/db_compaction_ldb_test.go diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go new file mode 100644 index 000000000..6928394ee --- /dev/null +++ b/cmd/benchmarkoor/db.go @@ -0,0 +1,114 @@ +package main + +import ( + "fmt" + "os/signal" + "syscall" + "time" + + "github.com/ethpandaops/benchmarkoor/pkg/client" + "github.com/ethpandaops/benchmarkoor/pkg/config" + "github.com/ethpandaops/benchmarkoor/pkg/docker" + "github.com/ethpandaops/benchmarkoor/pkg/runner" + "github.com/spf13/cobra" +) + +// dbCompactionUnneeded names the clients whose database an offline +// compaction has nothing to do for. +var dbCompactionUnneeded = map[client.ClientType]string{ + client.ClientReth: "MDBX, persisted every block", +} + +var dbCompactFlags struct { + client string + datadir string + prepare []string + image string + toolImage string + extraArgs []string + timeout string + noInspect bool +} + +var dbCmd = &cobra.Command{ + Use: "db", + Short: "Offline client database maintenance", +} + +var dbCompactCmd = &cobra.Command{ + Use: "compact", + Short: "Compact a stopped client's datadir, as db_compaction does in a run", + Long: `Compact a stopped client's datadir with the same containers a run's +db_compaction phase uses: the client's own command for geth and erigon, RocksDB's +ldb over every column family for nethermind and besu. geth's datadir then fails +if triedb/merkle.journal holds the state of more than the last 128 blocks. +reth needs nothing (MDBX, persisted every block).`, + Args: cobra.NoArgs, + RunE: runDBCompact, +} + +func init() { + rootCmd.AddCommand(dbCmd) + dbCmd.AddCommand(dbCompactCmd) + + f := dbCompactCmd.Flags() + f.StringVar(&dbCompactFlags.client, "client", "", "client that wrote the datadir (geth, nethermind, besu, erigon, reth, ethrex)") + f.StringVar(&dbCompactFlags.datadir, "datadir", "", "host path of the datadir; the client must be stopped") + f.StringArrayVar(&dbCompactFlags.prepare, "prepare", nil, "preparation step to run before the compaction (repeatable, e.g. erigon's seg-retire)") + f.StringVar(&dbCompactFlags.image, "image", "", "client image (default: the client's default image)") + f.StringVar(&dbCompactFlags.toolImage, "compaction-image", "", "image of the compaction container, as db_compaction.image") + f.StringArrayVar(&dbCompactFlags.extraArgs, "extra-arg", nil, "argument appended to the compaction command (repeatable)") + f.StringVar(&dbCompactFlags.timeout, "timeout", config.DefaultDBCompactionTimeout, "time limit for the whole compaction") + f.BoolVar(&dbCompactFlags.noInspect, "no-inspect", false, "skip the database inspection before and after") + + _ = dbCompactCmd.MarkFlagRequired("client") + _ = dbCompactCmd.MarkFlagRequired("datadir") +} + +func runDBCompact(cmd *cobra.Command, _ []string) error { + ct := client.ClientType(dbCompactFlags.client) + if why, ok := dbCompactionUnneeded[ct]; ok { + log.WithField("client", ct).Infof("Nothing to compact: %s", why) + + return nil + } + + if !client.SupportsDBCompaction(ct) { + return fmt.Errorf("client %q has no offline database compaction", ct) + } + + if _, err := time.ParseDuration(dbCompactFlags.timeout); err != nil { + return fmt.Errorf("--timeout: %w", err) + } + + inspect := !dbCompactFlags.noInspect + cfg := &config.DBCompactionConfig{ + Enabled: true, + Prepare: dbCompactFlags.prepare, + Image: dbCompactFlags.toolImage, + ExtraArgs: dbCompactFlags.extraArgs, + Timeout: dbCompactFlags.timeout, + Inspect: &inspect, + } + + ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + mgr, err := docker.NewManager(log) + if err != nil { + return err + } + + if err := mgr.Start(ctx); err != nil { + return err + } + + defer func() { _ = mgr.Stop() }() + + return runner.CompactDatadir(ctx, log, mgr, &runner.DatadirCompaction{ + Client: ct, + DataDir: dbCompactFlags.datadir, + Image: dbCompactFlags.image, + Cfg: cfg, + }) +} diff --git a/docs/configuration.md b/docs/configuration.md index 0fe233511..58089c223 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1162,6 +1162,16 @@ RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databas geth's path scheme keeps the last 128 blocks as in-memory diff layers and older, not yet flushed state in a write buffer (up to 256 MiB). A clean stop writes both to `/geth/triedb/merkle.journal`, and the next start loads them back into memory, so that state is never read from the database a compaction just rewrote. After the compaction the runner parses the journal (layout version 3, written by geth v1.17.7) and fails the phase if the write buffer is not empty or there are more than 128 diff layers. To pass, run geth with `--cache.gc=0` (a 0-byte buffer) for at least 128 blocks before it stops. `verify: false` skips the check; a datadir on a container volume is not checked. +###### Outside a run: `benchmarkoor db compact` + +The same containers and checks run against a stopped client's datadir without a config file, e.g. when producing a snapshot: + +```bash +benchmarkoor db compact --client --datadir [--prepare ]... +``` + +It exits non-zero on any failure, the geth journal check included. reth is a no-op with a log line; a client without offline compaction (ethrex) is an error. `--image` picks the client image (default: the client's default image), `--compaction-image` the compaction container's (as `image`), and `--extra-arg`, `--timeout` and `--no-inspect` mirror `extra_args`, `timeout` and `inspect`. No marker is written: that belongs to a run. The journal check reads the datadir from the host, so the CLI needs read access to it. + ###### Preparation steps (`prepare`) A client can offer steps that run before the compaction to make it reclaim more. None run unless `prepare` names them, because a step that helps one datadir can ruin another. diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index 3bbc32373..1f8eabe74 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -1007,3 +1007,98 @@ func datadirMountFor( return docker.Mount{}, false } + +// DatadirCompaction is one compaction of a stopped client's host datadir +// outside a run, as `benchmarkoor db compact` does it. +type DatadirCompaction struct { + Client client.ClientType + DataDir string + + // Image is the client image; empty uses the client's default. + Image string + + // Cfg carries the db_compaction settings that apply outside a run: + // prepare, extra_args, image, inspect, verify and timeout. + Cfg *config.DBCompactionConfig +} + +// CompactDatadir runs what a db_compaction phase runs — the inspections, the +// preparation steps, the compaction and the verification, each in the same +// container — against a host datadir. Container output goes to stdout. No +// marker or report is written: those belong to a run. +func CompactDatadir( + ctx context.Context, log *logrus.Logger, mgr docker.ContainerManager, c *DatadirCompaction, +) error { + spec, err := client.NewRegistry().Get(c.Client) + if err != nil { + return err + } + + cmds := spec.DBMaintenanceCommands(spec.DataDir()) + if cmds == nil || len(cmds.Compact) == 0 { + return fmt.Errorf("client %s has no database compaction command", c.Client) + } + + steps, err := dbCompactionSelectedSteps(cmds, c.Cfg) + if err != nil { + return err + } + + dataDir, err := filepath.Abs(c.DataDir) + if err != nil { + return err + } + + if info, err := os.Stat(dataDir); err != nil { + return err + } else if !info.IsDir() { + return fmt.Errorf("%s is not a directory", dataDir) + } + + image := c.Image + if image == "" { + image = spec.DefaultImage() + } + + logDir, err := os.MkdirTemp("", "benchmarkoor-db-compact-") + if err != nil { + return err + } + + defer func() { _ = os.RemoveAll(logDir) }() + + r := &runner{ + logger: log, + log: log.WithField("component", "db-compact"), + cfg: &Config{ClientLogsToStdout: true}, + containerMgr: mgr, + } + + req := &dbCompactionRequest{ + Instance: &config.ClientInstance{ID: string(c.Client), Client: string(c.Client)}, + Spec: spec, + Cfg: c.Cfg, + Phase: "cli", + ImageName: image, + RunID: time.Now().UTC().Format("20060102T150405Z"), + Mount: docker.Mount{Type: "bind", Source: dataDir, Target: spec.DataDir()}, + } + + usesClientImage := req.compactImage(cmds) == image || len(steps) > 0 || + (c.Cfg.InspectEnabled() && len(cmds.Inspect) > 0) + if usesClientImage && c.Cfg.Image == "" { + if err := mgr.PullImage(ctx, image, "if-not-present"); err != nil { + return fmt.Errorf("pulling client image: %w", err) + } + } + + started := time.Now() + + if err := r.runDBCompactionContainers(ctx, req, cmds, steps, logDir, r.log); err != nil { + return err + } + + r.log.WithField("duration", time.Since(started).Round(time.Second)).Info("Database compaction completed") + + return nil +} diff --git a/pkg/runner/db_compaction_ldb_test.go b/pkg/runner/db_compaction_ldb_test.go new file mode 100644 index 000000000..244c66da7 --- /dev/null +++ b/pkg/runner/db_compaction_ldb_test.go @@ -0,0 +1,111 @@ +package runner + +import ( + "bytes" + "context" + "fmt" + "os" + "regexp" + "testing" + + "github.com/ethpandaops/benchmarkoor/pkg/client" + "github.com/ethpandaops/benchmarkoor/pkg/config" + "github.com/ethpandaops/benchmarkoor/pkg/docker" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ldbTestSetup creates two databases with besu's single-byte family names +// (0x01, 0x09 and 0x0a among them) next to nethermind-style ones. +const ldbTestSetup = `set -e +nl=' +' +for db in /data/a/rdb /data/b; do + mkdir -p $db + ldb --db=$db --create_if_missing put k0 v0 >/dev/null + for cf in "$(printf '\001')" "$(printf '\011')" Blocks "with space" "$nl"; do + ldb --db=$db create_column_family "$cf" >/dev/null + done + # Every ldb run flushes on close, so each put leaves its own level-0 file. + for i in 1 2 3; do + for cf in default "$(printf '\001')" Blocks "$nl"; do + ldb --db=$db --try_load_options --column_family="$cf" put k$i "v$i$cf" >/dev/null + done + done +done +` + +const ldbTestDump = `set -e +nl=' +' +for db in /data/a/rdb /data/b; do + for cf in default "$(printf '\001')" "$(printf '\011')" Blocks "with space" "$nl"; do + echo "$db $(printf %s "$cf" | od -An -tx1 | tr -d ' \n')" + ldb --db=$db --try_load_options --column_family="$cf" get_property rocksdb.num-files-at-level0 + ldb --db=$db --try_load_options --column_family="$cf" --value_hex scan + done +done +` + +var levelZero = regexp.MustCompile(`(rocksdb\.num-files-at-level0: )\d+`) + +// TestCompactDatadir_RealLdb runs the nethermind/besu compaction against real +// RocksDB databases with the ldb image named by BENCHMARKOOR_TEST_LDB_IMAGE +// (built from Dockerfile.rocksdb-ldb), through the docker daemon. +func TestCompactDatadir_RealLdb(t *testing.T) { + image := os.Getenv("BENCHMARKOOR_TEST_LDB_IMAGE") + if image == "" { + t.Skip("BENCHMARKOOR_TEST_LDB_IMAGE is not set") + } + + ctx := context.Background() + log := logrus.New() + + mgr, err := docker.NewManager(log) + require.NoError(t, err) + require.NoError(t, mgr.Start(ctx)) + + t.Cleanup(func() { _ = mgr.Stop() }) + + dir := t.TempDir() + mnt := docker.Mount{Type: "bind", Source: dir, Target: "/data"} + + sh := func(name, script string) string { + var out bytes.Buffer + + require.NoError(t, mgr.RunInitContainer(ctx, &docker.ContainerSpec{ + Name: fmt.Sprintf("benchmarkoor-ldb-test-%s-%d", name, os.Getpid()), + Image: image, + Entrypoint: []string{"sh"}, + Command: []string{"-c", script}, + Mounts: []docker.Mount{mnt}, + }, &out, &out), out.String()) + + return out.String() + } + + t.Cleanup(func() { sh("chown", fmt.Sprintf("chown -R %d:%d /data", os.Getuid(), os.Getgid())) }) + + sh("setup", ldbTestSetup) + before := sh("before", ldbTestDump) + require.Contains(t, before, "/data/b 0a\nrocksdb.num-files-at-level0: 2\n", "the newline family has data in level 0") + + compact := func() { + require.NoError(t, CompactDatadir(ctx, log, mgr, &DatadirCompaction{ + Client: client.ClientNethermind, + DataDir: dir, + Cfg: &config.DBCompactionConfig{Enabled: true, Image: image}, + })) + } + + compact() + + after := sh("after", ldbTestDump) + assert.Equal(t, levelZero.ReplaceAllString(before, "${1}0"), after, + "every family keeps its data and has nothing left in level 0") + + // ldb records its own merge operator in OPTIONS; a second pass accepts it. + compact() + assert.Equal(t, after, sh("again", ldbTestDump)) +} From 3b028dec2f4395933a1495cd44a496e139ab3831 Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 8 Oct 2026 16:38:59 +0200 Subject: [PATCH 04/13] feat(runner): make the geth journal check opt-in in a run; on by default 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). --- cmd/benchmarkoor/db.go | 4 ++++ docs/configuration.md | 4 ++-- pkg/config/config.go | 8 +++++--- pkg/runner/db_compaction_test.go | 12 +++++++++--- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go index 6928394ee..d01f05182 100644 --- a/cmd/benchmarkoor/db.go +++ b/cmd/benchmarkoor/db.go @@ -28,6 +28,7 @@ var dbCompactFlags struct { extraArgs []string timeout string noInspect bool + noVerify bool } var dbCmd = &cobra.Command{ @@ -60,6 +61,7 @@ func init() { f.StringArrayVar(&dbCompactFlags.extraArgs, "extra-arg", nil, "argument appended to the compaction command (repeatable)") f.StringVar(&dbCompactFlags.timeout, "timeout", config.DefaultDBCompactionTimeout, "time limit for the whole compaction") f.BoolVar(&dbCompactFlags.noInspect, "no-inspect", false, "skip the database inspection before and after") + f.BoolVar(&dbCompactFlags.noVerify, "no-verify", false, "skip the client's check of the compacted datadir (geth's journal check)") _ = dbCompactCmd.MarkFlagRequired("client") _ = dbCompactCmd.MarkFlagRequired("datadir") @@ -82,6 +84,7 @@ func runDBCompact(cmd *cobra.Command, _ []string) error { } inspect := !dbCompactFlags.noInspect + verify := !dbCompactFlags.noVerify cfg := &config.DBCompactionConfig{ Enabled: true, Prepare: dbCompactFlags.prepare, @@ -89,6 +92,7 @@ func runDBCompact(cmd *cobra.Command, _ []string) error { ExtraArgs: dbCompactFlags.extraArgs, Timeout: dbCompactFlags.timeout, Inspect: &inspect, + Verify: &verify, } ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) diff --git a/docs/configuration.md b/docs/configuration.md index 58089c223..1eb28a671 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1160,7 +1160,7 @@ RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databas ###### geth: the journal check -geth's path scheme keeps the last 128 blocks as in-memory diff layers and older, not yet flushed state in a write buffer (up to 256 MiB). A clean stop writes both to `/geth/triedb/merkle.journal`, and the next start loads them back into memory, so that state is never read from the database a compaction just rewrote. After the compaction the runner parses the journal (layout version 3, written by geth v1.17.7) and fails the phase if the write buffer is not empty or there are more than 128 diff layers. To pass, run geth with `--cache.gc=0` (a 0-byte buffer) for at least 128 blocks before it stops. `verify: false` skips the check; a datadir on a container volume is not checked. +geth's path scheme keeps the last 128 blocks as in-memory diff layers and older, not yet flushed state in a write buffer (up to 256 MiB). A clean stop writes both to `/geth/triedb/merkle.journal`, and the next start loads them back into memory, so that state is never read from the database a compaction just rewrote. After the compaction the runner parses the journal (layout version 3, written by geth v1.17.7) and fails the phase if the write buffer is not empty or there are more than 128 diff layers. To pass, run geth with `--cache.gc=0` (a 0-byte buffer) for at least 128 blocks before it stops. The check is opt-in (`verify: true`) in a run and on by default in `benchmarkoor db compact` (`--no-verify` skips it); a datadir on a container volume is not checked. ###### Outside a run: `benchmarkoor db compact` @@ -1210,7 +1210,7 @@ runner: | `prepare` | []string | No | - | Client preparation steps to run before each compaction, in order (see [Preparation steps](#preparation-steps-prepare)) | | `timeout` | string | No | `3h` | Cap for one phase's work — every preparation step, the compaction, and both inspections (Go duration). Applies per phase | | `image` | string | No | the instance image; the `ldb` image for nethermind and besu | Image of the compaction container. The default keeps the tool version and the client version identical | -| `verify` | bool | No | `true` | Run the client's check of the compacted datadir (geth's journal check) | +| `verify` | bool | No | `false` | Run the client's check of the compacted datadir (geth's journal check) | | `extra_args` | []string | No | - | Extra arguments for the compaction command, e.g. `--cache=16384` | | `continue_on_error` | bool | No | `false` | Downgrade a compaction failure to a warning. A failed compaction makes the results incomparable, so the run fails by default | | `skip_if_marked` | bool | No | `true` with `persist`, else `false` | Skip a phase the datadir marker already names (see below) | diff --git a/pkg/config/config.go b/pkg/config/config.go index d6175e7b1..037cd1258 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1532,7 +1532,9 @@ type DBCompactionConfig struct { // Verify runs the client's check of the compacted datadir. geth's fails // when triedb/merkle.journal holds the state of more than the last 128 - // blocks, which geth would load back into memory on start. Default: true. + // blocks, which geth would load back into memory on start. Default: false, + // since a node stopped with a non-empty write buffer (any default-cache run) + // fails it; benchmarkoor db compact turns it on. Verify *bool `yaml:"verify,omitempty" mapstructure:"verify" json:"verify,omitempty"` // ExtraArgs appends arguments to the compaction command, e.g. @@ -1688,10 +1690,10 @@ func (c *DBCompactionConfig) InspectEnabled() bool { } // VerifyEnabled reports whether to run the client's check of the compacted -// datadir. Defaults to true. +// datadir. Defaults to false. func (c *DBCompactionConfig) VerifyEnabled() bool { if c == nil || c.Verify == nil { - return true + return false } return *c.Verify diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index 4b6b737b0..a598dec29 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -764,8 +764,8 @@ func TestRunDBCompactionContainers_ToolImage(t *testing.T) { } // TestRunDBCompactionContainers_Verify checks that a failed verification fails -// the phase before the inspection after it, that verify: false skips it, and -// that a volume datadir is not verified. +// the phase before the inspection after it, that it is opt-in, and that a +// volume datadir is not verified. func TestRunDBCompactionContainers_Verify(t *testing.T) { var checked []string @@ -783,6 +783,12 @@ func TestRunDBCompactionContainers_Verify(t *testing.T) { r, resultsDir := dbCompactionTestRunner(t, mgr) req := dbCompactionTestRequest(resultsDir) + require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log), "unset: off") + assert.Empty(t, checked) + + mgr.ran = nil + on := true + req.Cfg.Verify = &on err := r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log) require.ErrorContains(t, err, "journal holds 1354 blocks") assert.Equal(t, []string{resultsDir}, checked, "the host path of the datadir") @@ -792,7 +798,7 @@ func TestRunDBCompactionContainers_Verify(t *testing.T) { req.Cfg.Verify = &off require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) - req.Cfg.Verify = nil + req.Cfg.Verify = &on req.Mount = docker.Mount{Type: "volume", Source: "vol", Target: "/data"} require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) assert.Len(t, checked, 1) From cc9861ef05267e2ee9e2587946f880ce43b3edc1 Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 8 Oct 2026 19:03:42 +0200 Subject: [PATCH 05/13] feat(client): log each column family's levels and SST bytes before and 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. --- docs/configuration.md | 2 +- pkg/client/rocksdb_compact.sh | 21 +++++++++++++++++---- pkg/client/rocksdb_test.go | 15 +++++++++++---- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 1eb28a671..abfae3d5f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1151,7 +1151,7 @@ Supported clients are **geth**, **erigon**, **nethermind** and **besu**; every o ###### nethermind and besu: RocksDB `ldb` -Neither client ships an offline compaction command, so their databases are compacted with RocksDB's own `ldb`, run from a separate image (`ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1`, built from [`Dockerfile.rocksdb-ldb`](../Dockerfile.rocksdb-ldb); `image` overrides it). Every directory under the datadir holding an `OPTIONS-*` file is a database — nethermind has one per store (state, code, blocks, receipts, ...), besu one with a column family per segment — and each of its column families is compacted with `--try_load_options`, i.e. with the options the client wrote it with. `extra_args` reach every `ldb compact`. +Neither client ships an offline compaction command, so their databases are compacted with RocksDB's own `ldb`, run from a separate image (`ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1`, built from [`Dockerfile.rocksdb-ldb`](../Dockerfile.rocksdb-ldb); `image` overrides it). Every directory under the datadir holding an `OPTIONS-*` file is a database — nethermind has one per store (state, code, blocks, receipts, ...), besu one with a column family per segment — and each of its column families is compacted with `--try_load_options`, i.e. with the options the client wrote it with. `extra_args` reach every `ldb compact`. Each family is logged before and after with its non-empty levels and SST bytes (`L0=12/310MB L6=41/9870MB sst_bytes=…` → `L6=…`) and its duration, so the run output shows what the compaction changed, not only that it ran. RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databases open with it too. Nethermind 2.1.0 and Besu 26.9.0 each reopened a compacted mainnet-genesis datadir and served genesis block and state from it. Two guards make a wrong compaction fail rather than pass: diff --git a/pkg/client/rocksdb_compact.sh b/pkg/client/rocksdb_compact.sh index ffa871755..654553626 100644 --- a/pkg/client/rocksdb_compact.sh +++ b/pkg/client/rocksdb_compact.sh @@ -23,6 +23,16 @@ label() { esac } +# levels : the family's non-empty levels as L=/, then +# its SST bytes. Logged before and after, so the run output shows what the +# compaction did, not only that it ran. +levels() { + ldb --db="$1" --try_load_options --column_family="$2" get_property rocksdb.levelstats 2 && $2 > 0 { printf "L%s=%s/%sMB ", $1, $2, $3 }' + ldb --db="$1" --try_load_options --column_family="$2" get_property rocksdb.total-sst-files-size &2; exit 1; } @@ -54,15 +64,18 @@ echo "$dbs" | while IFS= read -r db; do *) cf=$cfs; cfs= ;; esac - echo "compacting $db column_family=$(label "$cf")" + echo "compacting $db column_family=$(label "$cf") $(levels "$db" "$cf")" + start=$(date +%s) ldb --db="$db" --try_load_options --column_family="$cf" "$@" compact &2; exit 1; } + after=$(levels "$db" "$cf") + case $after in + *"L0="*) echo "$db column_family=$(label "$cf"): level 0 not empty after compaction: $after" >&2; exit 1 ;; + esac + echo "compacted $db column_family=$(label "$cf") $after duration=$(( $(date +%s) - start ))s" [ -n "$cfs" ] || break done diff --git a/pkg/client/rocksdb_test.go b/pkg/client/rocksdb_test.go index 8b78fed79..5f7da2399 100644 --- a/pkg/client/rocksdb_test.go +++ b/pkg/client/rocksdb_test.go @@ -30,13 +30,19 @@ func TestRocksDBClientsCompactWithLdb(t *testing.T) { } // stubLdb answers list_column_families with `listing` (raw bytes, as ldb -// prints them), get_property with `level0`, and records every compact argv, +// prints them), get_property levelstats with $LDB_LEVEL0 files at level 0 and +// two at level 6, and records every compact argv, // NUL-separated with one line per call, in $LDB_CALLS. const stubLdb = `#!/bin/sh for a in "$@"; do case $a in list_column_families) printf '%s' "$LDB_LISTING"; exit 0 ;; - get_property) echo "rocksdb.num-files-at-level0: $LDB_LEVEL0"; exit 0 ;; + get_property) + case "$*" in + *levelstats*) printf 'rocksdb.levelstats: Level Files Size(MB)\n--------------------\n 0 %s 0\n 6 2 10\n' "$LDB_LEVEL0" ;; + *total-sst-files-size*) echo "rocksdb.total-sst-files-size: 4096" ;; + esac + exit 0 ;; compact) for b in "$@"; do printf '%s\0' "$b"; done >>"$LDB_CALLS"; echo >>"$LDB_CALLS"; exit 0 ;; esac done @@ -117,7 +123,8 @@ func TestRocksDBCompactScript_ParsesColumnFamilies(t *testing.T) { assert.Equal(t, besu, s.compacted(t)) assert.Contains(t, out, "column_family=0x0a") - assert.Contains(t, out, "column_family=default") + assert.Contains(t, out, "column_family=default L6=2/10MB sst_bytes=4096\n", "levels before") + assert.Regexp(t, `compacted \S+ column_family=default L6=2/10MB sst_bytes=4096 duration=\d+s`, out, "levels and duration after") } func TestRocksDBCompactScript_EveryDatabase(t *testing.T) { @@ -162,7 +169,7 @@ func TestRocksDBCompactScript_Failures(t *testing.T) { out, err := s.run(t) require.Error(t, err) - assert.Contains(t, out, "rocksdb.num-files-at-level0: 2 after compaction") + assert.Contains(t, out, "level 0 not empty after compaction: L0=2/0MB L6=2/10MB sst_bytes=4096") }) t.Run("a merge operator ldb does not have", func(t *testing.T) { From ae87520e36b27880b2c983ef3490818f2c19bea2 Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 11 Oct 2026 12:00:58 +0200 Subject: [PATCH 06/13] feat(client): compact reth with reth db copy --compact; name why ethrex 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 /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'. --- cmd/benchmarkoor/db.go | 12 ---------- pkg/client/client.go | 6 +++++ pkg/client/ethrex.go | 7 ++++-- pkg/client/reth.go | 39 ++++++++++++++++++++++++++++---- pkg/runner/db_compaction.go | 2 +- pkg/runner/db_compaction_test.go | 21 ++++++++++++++++- 6 files changed, 67 insertions(+), 20 deletions(-) diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go index d01f05182..62eaeece5 100644 --- a/cmd/benchmarkoor/db.go +++ b/cmd/benchmarkoor/db.go @@ -13,12 +13,6 @@ import ( "github.com/spf13/cobra" ) -// dbCompactionUnneeded names the clients whose database an offline -// compaction has nothing to do for. -var dbCompactionUnneeded = map[client.ClientType]string{ - client.ClientReth: "MDBX, persisted every block", -} - var dbCompactFlags struct { client string datadir string @@ -69,12 +63,6 @@ func init() { func runDBCompact(cmd *cobra.Command, _ []string) error { ct := client.ClientType(dbCompactFlags.client) - if why, ok := dbCompactionUnneeded[ct]; ok { - log.WithField("client", ct).Infof("Nothing to compact: %s", why) - - return nil - } - if !client.SupportsDBCompaction(ct) { return fmt.Errorf("client %q has no offline database compaction", ct) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 1b4a2721c..6052d7974 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -82,6 +82,12 @@ type DBMaintenanceCommands struct { // offline compactor and is compacted by a separate tool. CompactImage string + // OwnEntrypoint runs Compact in the client's image with Compact[0] as the + // entrypoint instead of as an argument to the client's: the command is a + // shell around the client binary (reth's copy-and-swap). CompactImage + // implies it. + OwnEntrypoint bool + // Verify checks the datadir after the compaction, given its host path, and // returns a one-line summary of what it found. Optional. Verify func(hostDataDir string) (string, error) diff --git a/pkg/client/ethrex.go b/pkg/client/ethrex.go index 3be818014..9f23ab7d5 100644 --- a/pkg/client/ethrex.go +++ b/pkg/client/ethrex.go @@ -98,8 +98,11 @@ func (s *ethrexSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands returns nil; benchmarkoor has no offline -// compaction command for Ethrex yet. +// DBMaintenanceCommands returns nil: Ethrex (29.x) is one RocksDB with a +// column family, TRANSACTION_LOCATIONS, under its own merge operator +// (tx_locations_merge). ldb has only a string-append operator and would fold +// that family's operands wrongly, so the generic ldb compaction refuses the +// database, and ethrex ships no offline compactor of its own. func (s *ethrexSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { return nil } diff --git a/pkg/client/reth.go b/pkg/client/reth.go index f71836106..3e7c79d07 100644 --- a/pkg/client/reth.go +++ b/pkg/client/reth.go @@ -106,8 +106,39 @@ func (s *rethSpec) SnapshotPrepareArgs() []string { } } -// DBMaintenanceCommands returns nil; benchmarkoor has no offline -// compaction command for Reth yet. -func (s *rethSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { - return nil +// DBMaintenanceCommands compacts Reth's MDBX database with `reth db copy +// --compact` (mdbx_copy -c, bundled since Reth 2.7.0): the database is +// rewritten without its free pages into /db.compact and swapped in. +// The static files are append-only segments and are left alone. +// +// MDBX never shrinks: the pages a replay frees stay in mdbx.dat, and in every +// archive of it. The copy needs that much free space under the datadir, so the +// script fails before copying when the filesystem has less than the file's size. +// +// `reth db stats` is the inspection: tables, entries, sizes and the freelist. +func (s *rethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { + return &DBMaintenanceCommands{ + Compact: []string{"sh", "-c", rethCompactScript, "reth-compact", dataDir}, + OwnEntrypoint: true, + Inspect: []string{"db", "stats", "--datadir=" + dataDir}, + } } + +// rethCompactScript is the copy-and-swap around `reth db copy --compact`, in +// the reth image's /bin/sh. $1 is the datadir. The destination must be an +// existing empty directory (mdbx_env_copy in subdir mode); database.version +// lives next to mdbx.dat and is carried over. +const rethCompactScript = `set -eu +d=$1 +dat=$d/db/mdbx.dat +[ -f "$dat" ] || { echo "no $dat: not a reth datadir" >&2; exit 1; } +size=$(stat -c %s "$dat") +free=$(df -B1 --output=avail "$d" | tail -n 1) +[ "$free" -gt "$size" ] || { echo "need $size bytes free under $d for the compacted copy, have $free" >&2; exit 1; } +rm -rf "$d/db.compact" && mkdir "$d/db.compact" +reth db copy --compact --datadir="$d" "$d/db.compact" +[ -f "$d/db.compact/mdbx.dat" ] || { echo "reth db copy wrote no $d/db.compact/mdbx.dat" >&2; exit 1; } +[ ! -f "$d/db/database.version" ] || cp "$d/db/database.version" "$d/db.compact/" +mv "$d/db" "$d/db.old" && mv "$d/db.compact" "$d/db" && rm -rf "$d/db.old" +echo "reth-compact: mdbx.dat $size -> $(stat -c %s "$d/db/mdbx.dat") bytes" +` diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index 1f8eabe74..985c306f6 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -435,7 +435,7 @@ func (r *runner) runDBCompactionContainers( log.WithField("timeout", timeout).Info("Compacting the database") entrypoint, command := req.Instance.Entrypoint, dbCompactionCommand(cmds, req.Cfg) - if cmds.CompactImage != "" { + if cmds.CompactImage != "" || cmds.OwnEntrypoint { entrypoint, command = command[:1], command[1:] } diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index a598dec29..7549ff586 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -189,9 +189,28 @@ func TestErigonDBMaintenanceCommands(t *testing.T) { assert.True(t, client.SupportsDBCompaction(client.ClientErigon)) } +// TestRethDBMaintenanceCommands: reth compacts with a copy-and-swap shell +// around `reth db copy --compact`, so the script is the entrypoint. +func TestRethDBMaintenanceCommands(t *testing.T) { + spec, err := client.NewRegistry().Get(client.ClientReth) + require.NoError(t, err) + + cmds := spec.DBMaintenanceCommands("/data") + require.NotNil(t, cmds) + + assert.True(t, cmds.OwnEntrypoint) + require.Len(t, cmds.Compact, 5) + assert.Equal(t, "sh", cmds.Compact[0]) + assert.Contains(t, cmds.Compact[2], "reth db copy --compact") + assert.Equal(t, "/data", cmds.Compact[4]) + assert.Equal(t, []string{"db", "stats", "--datadir=/data"}, cmds.Inspect) + assert.Empty(t, cmds.CompactImage, "runs in the client's own image") + assert.True(t, client.SupportsDBCompaction(client.ClientReth)) +} + func TestSupportsDBCompaction_UnsupportedClients(t *testing.T) { for _, other := range []client.ClientType{ - client.ClientReth, client.ClientNimbus, client.ClientEthrex, + client.ClientNimbus, client.ClientEthrex, } { assert.False(t, client.SupportsDBCompaction(other), string(other)) } From ec92d69933cfeacb5ac209b98edcddbbc8b46e4f Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 11 Oct 2026 13:30:13 +0200 Subject: [PATCH 07/13] fix(client): reth's --datadir goes on db, not on stats/copy 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. --- pkg/client/reth.go | 5 +++-- pkg/runner/db_compaction_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/client/reth.go b/pkg/client/reth.go index 3e7c79d07..8a9f35b21 100644 --- a/pkg/client/reth.go +++ b/pkg/client/reth.go @@ -116,11 +116,12 @@ func (s *rethSpec) SnapshotPrepareArgs() []string { // script fails before copying when the filesystem has less than the file's size. // // `reth db stats` is the inspection: tables, entries, sizes and the freelist. +// --datadir belongs to `db`, not to its subcommands (clap rejects it there). func (s *rethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { return &DBMaintenanceCommands{ Compact: []string{"sh", "-c", rethCompactScript, "reth-compact", dataDir}, OwnEntrypoint: true, - Inspect: []string{"db", "stats", "--datadir=" + dataDir}, + Inspect: []string{"db", "--datadir=" + dataDir, "stats"}, } } @@ -136,7 +137,7 @@ size=$(stat -c %s "$dat") free=$(df -B1 --output=avail "$d" | tail -n 1) [ "$free" -gt "$size" ] || { echo "need $size bytes free under $d for the compacted copy, have $free" >&2; exit 1; } rm -rf "$d/db.compact" && mkdir "$d/db.compact" -reth db copy --compact --datadir="$d" "$d/db.compact" +reth db --datadir="$d" copy --compact "$d/db.compact" [ -f "$d/db.compact/mdbx.dat" ] || { echo "reth db copy wrote no $d/db.compact/mdbx.dat" >&2; exit 1; } [ ! -f "$d/db/database.version" ] || cp "$d/db/database.version" "$d/db.compact/" mv "$d/db" "$d/db.old" && mv "$d/db.compact" "$d/db" && rm -rf "$d/db.old" diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index 7549ff586..5247ac4ac 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -201,9 +201,9 @@ func TestRethDBMaintenanceCommands(t *testing.T) { assert.True(t, cmds.OwnEntrypoint) require.Len(t, cmds.Compact, 5) assert.Equal(t, "sh", cmds.Compact[0]) - assert.Contains(t, cmds.Compact[2], "reth db copy --compact") + assert.Contains(t, cmds.Compact[2], "reth db --datadir=\"$d\" copy --compact") assert.Equal(t, "/data", cmds.Compact[4]) - assert.Equal(t, []string{"db", "stats", "--datadir=/data"}, cmds.Inspect) + assert.Equal(t, []string{"db", "--datadir=/data", "stats"}, cmds.Inspect) assert.Empty(t, cmds.CompactImage, "runs in the client's own image") assert.True(t, client.SupportsDBCompaction(client.ClientReth)) } From 1765da1b2e83dbcdc199c18a60ba94c5d203c7ed Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 11 Oct 2026 13:33:06 +0200 Subject: [PATCH 08/13] fix(client): let mdbx_env_copy create reth's destination directory It fails with EEXIST (error 17) on a pre-created one (2026-10-11 build). --- pkg/client/reth.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/client/reth.go b/pkg/client/reth.go index 8a9f35b21..7e0484061 100644 --- a/pkg/client/reth.go +++ b/pkg/client/reth.go @@ -126,9 +126,9 @@ func (s *rethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands } // rethCompactScript is the copy-and-swap around `reth db copy --compact`, in -// the reth image's /bin/sh. $1 is the datadir. The destination must be an -// existing empty directory (mdbx_env_copy in subdir mode); database.version -// lives next to mdbx.dat and is carried over. +// the reth image's /bin/sh. $1 is the datadir. mdbx_env_copy creates the +// destination directory itself and fails with EEXIST on an existing one; +// database.version lives next to mdbx.dat and is carried over. const rethCompactScript = `set -eu d=$1 dat=$d/db/mdbx.dat @@ -136,7 +136,7 @@ dat=$d/db/mdbx.dat size=$(stat -c %s "$dat") free=$(df -B1 --output=avail "$d" | tail -n 1) [ "$free" -gt "$size" ] || { echo "need $size bytes free under $d for the compacted copy, have $free" >&2; exit 1; } -rm -rf "$d/db.compact" && mkdir "$d/db.compact" +rm -rf "$d/db.compact" reth db --datadir="$d" copy --compact "$d/db.compact" [ -f "$d/db.compact/mdbx.dat" ] || { echo "reth db copy wrote no $d/db.compact/mdbx.dat" >&2; exit 1; } [ ! -f "$d/db/database.version" ] || cp "$d/db/database.version" "$d/db.compact/" From 3df7227f0d9bd1e629435b4acec4e4080c1a3bcf Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 11 Oct 2026 13:36:15 +0200 Subject: [PATCH 09/13] fix(client): reth's compacted copy is one file; swap it in as db/mdbx.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. --- pkg/client/reth.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/client/reth.go b/pkg/client/reth.go index 7e0484061..c05641c2b 100644 --- a/pkg/client/reth.go +++ b/pkg/client/reth.go @@ -107,8 +107,8 @@ func (s *rethSpec) SnapshotPrepareArgs() []string { } // DBMaintenanceCommands compacts Reth's MDBX database with `reth db copy -// --compact` (mdbx_copy -c, bundled since Reth 2.7.0): the database is -// rewritten without its free pages into /db.compact and swapped in. +// --compact` (mdbx_copy -c, bundled since Reth 2.7.0): the database file is +// rewritten without its free pages to /db.compact and swapped in. // The static files are append-only segments and are left alone. // // MDBX never shrinks: the pages a replay frees stay in mdbx.dat, and in every @@ -126,9 +126,10 @@ func (s *rethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands } // rethCompactScript is the copy-and-swap around `reth db copy --compact`, in -// the reth image's /bin/sh. $1 is the datadir. mdbx_env_copy creates the -// destination directory itself and fails with EEXIST on an existing one; -// database.version lives next to mdbx.dat and is carried over. +// the reth image's /bin/sh. $1 is the datadir. Reth opens MDBX in no-subdir +// mode (/db/mdbx.dat), so mdbx_env_copy writes one file at the +// destination path and fails with EEXIST if it exists; the file then replaces +// db/mdbx.dat and the stale db/mdbx.lck goes. const rethCompactScript = `set -eu d=$1 dat=$d/db/mdbx.dat @@ -136,10 +137,9 @@ dat=$d/db/mdbx.dat size=$(stat -c %s "$dat") free=$(df -B1 --output=avail "$d" | tail -n 1) [ "$free" -gt "$size" ] || { echo "need $size bytes free under $d for the compacted copy, have $free" >&2; exit 1; } -rm -rf "$d/db.compact" +rm -f "$d/db.compact" reth db --datadir="$d" copy --compact "$d/db.compact" -[ -f "$d/db.compact/mdbx.dat" ] || { echo "reth db copy wrote no $d/db.compact/mdbx.dat" >&2; exit 1; } -[ ! -f "$d/db/database.version" ] || cp "$d/db/database.version" "$d/db.compact/" -mv "$d/db" "$d/db.old" && mv "$d/db.compact" "$d/db" && rm -rf "$d/db.old" -echo "reth-compact: mdbx.dat $size -> $(stat -c %s "$d/db/mdbx.dat") bytes" +[ -s "$d/db.compact" ] || { echo "reth db copy wrote no $d/db.compact" >&2; exit 1; } +mv "$dat" "$dat.old" && mv "$d/db.compact" "$dat" && rm -f "$dat.old" "$d/db/mdbx.lck" +echo "reth-compact: mdbx.dat $size -> $(stat -c %s "$dat") bytes" ` From d47c060e8f7d15c07987f0c5f9725acaa1c39b97 Mon Sep 17 00:00:00 2001 From: Stefan Date: Mon, 12 Oct 2026 10:36:45 +0200 Subject: [PATCH 10/13] db: fail compaction when RocksDB properties cannot be read --- cmd/benchmarkoor/db.go | 4 ++-- docs/configuration.md | 7 +++++-- pkg/client/rocksdb_compact.sh | 11 ++++++++--- pkg/client/rocksdb_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go index 62eaeece5..476554651 100644 --- a/cmd/benchmarkoor/db.go +++ b/cmd/benchmarkoor/db.go @@ -34,10 +34,10 @@ var dbCompactCmd = &cobra.Command{ Use: "compact", Short: "Compact a stopped client's datadir, as db_compaction does in a run", Long: `Compact a stopped client's datadir with the same containers a run's -db_compaction phase uses: the client's own command for geth and erigon, RocksDB's +db_compaction phase uses: the client's own command for geth, erigon and reth, RocksDB's ldb over every column family for nethermind and besu. geth's datadir then fails if triedb/merkle.journal holds the state of more than the last 128 blocks. -reth needs nothing (MDBX, persisted every block).`, +reth copies and swaps its MDBX file, requiring free space for the compacted copy.`, Args: cobra.NoArgs, RunE: runDBCompact, } diff --git a/docs/configuration.md b/docs/configuration.md index abfae3d5f..8fce5850f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1137,16 +1137,19 @@ runner: The `db_compaction` option compacts the client database before the run measures anything. Compaction needs exclusive access to the database, so the client is never running while it happens: the runner runs an offline command — the client's own, or RocksDB's `ldb` for clients without one — in a one-shot container against the datadir. -Supported clients are **geth**, **erigon**, **nethermind** and **besu**; every other client fails validation with a clear message. reth needs no compaction: MDBX is persisted every block. +Supported clients are **geth**, **erigon**, **reth**, **nethermind** and **besu**; every other client fails validation with a clear message. | Client | Compaction | Inspection | Check after | |--------|------------|------------|-------------| | geth | `geth db compact` | `geth db inspect` | `triedb/merkle.journal` holds at most the last 128 blocks | | erigon | `erigon db compact` | `erigon seg du --verbose` | - | +| reth | `reth db copy --compact`, then swap the MDBX file | - | copy must finish before the original is replaced | | nethermind, besu | RocksDB `ldb compact`, every column family of every database | - | level 0 of every column family is empty | `erigon db compact` rewrites every mdbx database of the datadir without its free pages. It needs **Erigon 3.7.0-dev or newer** (September 2026); an older binary, including every pinned glamsterdam-devnet image, fails the step with `command db not found`. +Reth compaction needs **Reth 2.7.0 or newer** and enough free disk for a compacted copy of `db/mdbx.dat`. It writes the copy to a separate file before swapping it into place; static files are unchanged. + > **On erigon, `before_benchmarks` needs `--exec.no-prune` on the instance.** That phase stops and restarts the client, and erigon refuses to reopen a datadir whose receipt domain it pruned past the snapshot files: `[snapshots] gap between snapshot files and DB for domain receipt: files end at txNum 0 but the DB was pruned up to 390625`. A synthetic snapshot has no snapshot files, so the client's own pruning is enough to create the gap. `--exec.no-prune` disables the state-aggregator pruning that advances the marker. Erigon documents the flag for diagnostic and perf-comparison use, which is what a benchmark is, and it removes housekeeping the benchmark does not want anyway. `before_pre_runs` needs no flag: it compacts before the client ever boots, so nothing restarts. ###### nethermind and besu: RocksDB `ldb` @@ -1170,7 +1173,7 @@ The same containers and checks run against a stopped client's datadir without a benchmarkoor db compact --client --datadir [--prepare ]... ``` -It exits non-zero on any failure, the geth journal check included. reth is a no-op with a log line; a client without offline compaction (ethrex) is an error. `--image` picks the client image (default: the client's default image), `--compaction-image` the compaction container's (as `image`), and `--extra-arg`, `--timeout` and `--no-inspect` mirror `extra_args`, `timeout` and `inspect`. No marker is written: that belongs to a run. The journal check reads the datadir from the host, so the CLI needs read access to it. +It exits non-zero on any failure, the geth journal check included. reth copies and swaps its MDBX file; a client without offline compaction (ethrex) is an error. `--image` picks the client image (default: the client's default image), `--compaction-image` the compaction container's (as `image`), and `--extra-arg`, `--timeout` and `--no-inspect` mirror `extra_args`, `timeout` and `inspect`. No marker is written: that belongs to a run. The journal check reads the datadir from the host, so the CLI needs read access to it. ###### Preparation steps (`prepare`) diff --git a/pkg/client/rocksdb_compact.sh b/pkg/client/rocksdb_compact.sh index 654553626..534bc9783 100644 --- a/pkg/client/rocksdb_compact.sh +++ b/pkg/client/rocksdb_compact.sh @@ -27,9 +27,13 @@ label() { # its SST bytes. Logged before and after, so the run output shows what the # compaction did, not only that it ran. levels() { - ldb --db="$1" --try_load_options --column_family="$2" get_property rocksdb.levelstats 2 && $2 > 0 { printf "L%s=%s/%sMB ", $1, $2, $3 }' - ldb --db="$1" --try_load_options --column_family="$2" get_property rocksdb.total-sst-files-size &2; exit 1 + fi ;; + esac case "$*" in *levelstats*) printf 'rocksdb.levelstats: Level Files Size(MB)\n--------------------\n 0 %s 0\n 6 2 10\n' "$LDB_LEVEL0" ;; *total-sst-files-size*) echo "rocksdb.total-sst-files-size: 4096" ;; @@ -74,6 +80,8 @@ func newLdbStub(t *testing.T, listing string, dbs ...string) *ldbStub { "LDB_LISTING="+listing, "LDB_LEVEL0=0", "LDB_CALLS="+s.calls, + "LDB_FAIL_PROPERTY=", + "LDB_FAIL_PHASE=", ) return s @@ -146,6 +154,25 @@ func TestRocksDBCompactScript_EveryDatabase(t *testing.T) { } func TestRocksDBCompactScript_Failures(t *testing.T) { + for _, property := range []string{"levelstats", "total-sst-files-size"} { + for _, phase := range []string{"before", "after"} { + t.Run(property+" fails "+phase+" compaction", func(t *testing.T) { + s := newLdbStub(t, "Column families in db: \n{default}\n", "db") + s.env = append(s.env, "LDB_FAIL_PROPERTY="+property, "LDB_FAIL_PHASE="+phase) + + out, err := s.run(t) + require.Error(t, err, out) + assert.Contains(t, out, "property read failed") + assert.NotContains(t, out, "compacted ") + if phase == "before" { + assert.NoFileExists(t, s.calls, "no compaction without readable properties") + } else { + assert.Equal(t, []string{"default"}, s.compacted(t)) + } + }) + } + } + t.Run("no database", func(t *testing.T) { s := newLdbStub(t, "") From f9997f67e377b1e984f2e021cff02baf9e4a5299 Mon Sep 17 00:00:00 2001 From: Stefan Date: Mon, 12 Oct 2026 10:42:17 +0200 Subject: [PATCH 11/13] docs: include Reth stats in compaction inspection table --- docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index 8fce5850f..bf25646e5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1143,7 +1143,7 @@ Supported clients are **geth**, **erigon**, **reth**, **nethermind** and **besu* |--------|------------|------------|-------------| | geth | `geth db compact` | `geth db inspect` | `triedb/merkle.journal` holds at most the last 128 blocks | | erigon | `erigon db compact` | `erigon seg du --verbose` | - | -| reth | `reth db copy --compact`, then swap the MDBX file | - | copy must finish before the original is replaced | +| reth | `reth db copy --compact`, then swap the MDBX file | `reth db stats` | copy must finish before the original is replaced | | nethermind, besu | RocksDB `ldb compact`, every column family of every database | - | level 0 of every column family is empty | `erigon db compact` rewrites every mdbx database of the datadir without its free pages. It needs **Erigon 3.7.0-dev or newer** (September 2026); an older binary, including every pinned glamsterdam-devnet image, fails the step with `command db not found`. From d6995bae377921ca0c303303c3d62da7edc8e9fb Mon Sep 17 00:00:00 2001 From: Stefan Date: Mon, 12 Oct 2026 11:28:18 +0200 Subject: [PATCH 12/13] refactor: restrict compaction CLI to existing client commands --- .../workflows/deploy-docker.rocksdb-ldb.yaml | 72 ------ Dockerfile.rocksdb-ldb | 18 -- cmd/benchmarkoor/db.go | 17 +- docs/configuration.md | 42 ++-- pkg/client/besu.go | 18 +- pkg/client/client.go | 15 -- pkg/client/ethrex.go | 7 +- pkg/client/geth.go | 23 -- pkg/client/geth_journal.go | 229 ------------------ pkg/client/geth_journal_test.go | 220 ----------------- pkg/client/nethermind.go | 9 +- pkg/client/reth.go | 40 +-- pkg/client/rocksdb.go | 21 -- pkg/client/rocksdb_compact.sh | 87 ------- pkg/client/rocksdb_test.go | 224 ----------------- pkg/config/config.go | 23 +- pkg/config/config_test.go | 4 +- pkg/runner/db_compaction.go | 93 ++----- pkg/runner/db_compaction_cli_test.go | 102 ++++++++ pkg/runner/db_compaction_ldb_test.go | 111 --------- pkg/runner/db_compaction_test.go | 115 +-------- 21 files changed, 162 insertions(+), 1328 deletions(-) delete mode 100644 .github/workflows/deploy-docker.rocksdb-ldb.yaml delete mode 100644 Dockerfile.rocksdb-ldb delete mode 100644 pkg/client/geth_journal.go delete mode 100644 pkg/client/geth_journal_test.go delete mode 100644 pkg/client/rocksdb.go delete mode 100644 pkg/client/rocksdb_compact.sh delete mode 100644 pkg/client/rocksdb_test.go create mode 100644 pkg/runner/db_compaction_cli_test.go delete mode 100644 pkg/runner/db_compaction_ldb_test.go diff --git a/.github/workflows/deploy-docker.rocksdb-ldb.yaml b/.github/workflows/deploy-docker.rocksdb-ldb.yaml deleted file mode 100644 index c9f071327..000000000 --- a/.github/workflows/deploy-docker.rocksdb-ldb.yaml +++ /dev/null @@ -1,72 +0,0 @@ -name: Deploy - Docker (RocksDB ldb) - -# The ldb image db_compaction uses for nethermind and besu -# (client.RocksDBLdbImage). Tagged by RocksDB version, so bump both together. -on: - push: - branches: - - master - paths: - - Dockerfile.rocksdb-ldb - - .github/workflows/deploy-docker.rocksdb-ldb.yaml - workflow_dispatch: - -env: - IMAGE: ghcr.io/${{ github.repository }}-rocksdb-ldb - ROCKSDB_VERSION: "11.8.1" - -jobs: - build: - name: Build ${{ matrix.arch }} image - runs-on: ${{ matrix.runner }} - strategy: - matrix: - include: - - arch: amd64 - runner: ubuntu-latest - - arch: arm64 - runner: ubuntu-24.04-arm - permissions: - contents: read - packages: write - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - - name: Log in to GHCR - uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3.3.0 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Build and push - run: | - docker build . --file Dockerfile.rocksdb-ldb \ - --build-arg ROCKSDB_VERSION="$ROCKSDB_VERSION" \ - --platform "linux/${{ matrix.arch }}" \ - --tag "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" - docker run --rm "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" --version | grep -F "RocksDB $ROCKSDB_VERSION" - docker push "$IMAGE:$ROCKSDB_VERSION-${{ matrix.arch }}" - - manifest: - name: Multi-arch manifest - needs: build - runs-on: ubuntu-latest - permissions: - contents: read - packages: write - steps: - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@c47758b77c9736f4b2ef4073d4d51994fabfe349 # v3.7.1 - - - name: Log in to GHCR - uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3.3.0 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Create manifest - run: | - docker buildx imagetools create -t "$IMAGE:$ROCKSDB_VERSION" \ - "$IMAGE:$ROCKSDB_VERSION-amd64" "$IMAGE:$ROCKSDB_VERSION-arm64" diff --git a/Dockerfile.rocksdb-ldb b/Dockerfile.rocksdb-ldb deleted file mode 100644 index f70da56b2..000000000 --- a/Dockerfile.rocksdb-ldb +++ /dev/null @@ -1,18 +0,0 @@ -# RocksDB's ldb, which db_compaction uses to compact nethermind and besu: neither -# ships an offline compaction command. The compaction script (pkg/client/ -# rocksdb_compact.sh) also needs sh, find, sort, grep and od, which the base has. -# Release build: a default `make ldb` links the debug tools library. -FROM debian:bookworm AS build -ARG ROCKSDB_VERSION=11.8.1 -RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \ - ca-certificates git build-essential libsnappy-dev zlib1g-dev libbz2-dev liblz4-dev libzstd-dev \ - && rm -rf /var/lib/apt/lists/* -RUN git clone -q --depth 1 -b "v${ROCKSDB_VERSION}" https://github.com/facebook/rocksdb /rocksdb \ - && cd /rocksdb && DEBUG_LEVEL=0 PORTABLE=1 make -j"$(nproc)" ldb - -FROM debian:bookworm-slim -RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \ - libsnappy1v5 zlib1g libbz2-1.0 liblz4-1 libzstd1 \ - && rm -rf /var/lib/apt/lists/* -COPY --from=build /rocksdb/ldb /usr/local/bin/ldb -ENTRYPOINT ["ldb"] diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go index 476554651..39b2675c7 100644 --- a/cmd/benchmarkoor/db.go +++ b/cmd/benchmarkoor/db.go @@ -18,11 +18,9 @@ var dbCompactFlags struct { datadir string prepare []string image string - toolImage string extraArgs []string timeout string noInspect bool - noVerify bool } var dbCmd = &cobra.Command{ @@ -33,11 +31,9 @@ var dbCmd = &cobra.Command{ var dbCompactCmd = &cobra.Command{ Use: "compact", Short: "Compact a stopped client's datadir, as db_compaction does in a run", - Long: `Compact a stopped client's datadir with the same containers a run's -db_compaction phase uses: the client's own command for geth, erigon and reth, RocksDB's -ldb over every column family for nethermind and besu. geth's datadir then fails -if triedb/merkle.journal holds the state of more than the last 128 blocks. -reth copies and swaps its MDBX file, requiring free space for the compacted copy.`, + Long: `Compact a stopped client's datadir using the client's own offline command. +Supported clients are geth and erigon. Use --image to select the same client +version that wrote the datadir. Other clients return an unsupported error.`, Args: cobra.NoArgs, RunE: runDBCompact, } @@ -47,15 +43,13 @@ func init() { dbCmd.AddCommand(dbCompactCmd) f := dbCompactCmd.Flags() - f.StringVar(&dbCompactFlags.client, "client", "", "client that wrote the datadir (geth, nethermind, besu, erigon, reth, ethrex)") + f.StringVar(&dbCompactFlags.client, "client", "", "client that wrote the datadir (geth or erigon)") f.StringVar(&dbCompactFlags.datadir, "datadir", "", "host path of the datadir; the client must be stopped") f.StringArrayVar(&dbCompactFlags.prepare, "prepare", nil, "preparation step to run before the compaction (repeatable, e.g. erigon's seg-retire)") f.StringVar(&dbCompactFlags.image, "image", "", "client image (default: the client's default image)") - f.StringVar(&dbCompactFlags.toolImage, "compaction-image", "", "image of the compaction container, as db_compaction.image") f.StringArrayVar(&dbCompactFlags.extraArgs, "extra-arg", nil, "argument appended to the compaction command (repeatable)") f.StringVar(&dbCompactFlags.timeout, "timeout", config.DefaultDBCompactionTimeout, "time limit for the whole compaction") f.BoolVar(&dbCompactFlags.noInspect, "no-inspect", false, "skip the database inspection before and after") - f.BoolVar(&dbCompactFlags.noVerify, "no-verify", false, "skip the client's check of the compacted datadir (geth's journal check)") _ = dbCompactCmd.MarkFlagRequired("client") _ = dbCompactCmd.MarkFlagRequired("datadir") @@ -72,15 +66,12 @@ func runDBCompact(cmd *cobra.Command, _ []string) error { } inspect := !dbCompactFlags.noInspect - verify := !dbCompactFlags.noVerify cfg := &config.DBCompactionConfig{ Enabled: true, Prepare: dbCompactFlags.prepare, - Image: dbCompactFlags.toolImage, ExtraArgs: dbCompactFlags.extraArgs, Timeout: dbCompactFlags.timeout, Inspect: &inspect, - Verify: &verify, } ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) diff --git a/docs/configuration.md b/docs/configuration.md index bf25646e5..eb48c4b1c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1135,45 +1135,30 @@ runner: ##### Database Compaction -The `db_compaction` option compacts the client database before the run measures anything. Compaction needs exclusive access to the database, so the client is never running while it happens: the runner runs an offline command — the client's own, or RocksDB's `ldb` for clients without one — in a one-shot container against the datadir. +The `db_compaction` option compacts the client database before the run measures anything. Compaction needs exclusive access to the database, so the client is never running while it happens: the runner runs the client's own offline command in a one-shot container against the datadir. -Supported clients are **geth**, **erigon**, **reth**, **nethermind** and **besu**; every other client fails validation with a clear message. +Only clients that ship an offline compaction command support this. Today that is **geth** and **erigon**; every other client fails validation with a clear message. -| Client | Compaction | Inspection | Check after | -|--------|------------|------------|-------------| -| geth | `geth db compact` | `geth db inspect` | `triedb/merkle.journal` holds at most the last 128 blocks | -| erigon | `erigon db compact` | `erigon seg du --verbose` | - | -| reth | `reth db copy --compact`, then swap the MDBX file | `reth db stats` | copy must finish before the original is replaced | -| nethermind, besu | RocksDB `ldb compact`, every column family of every database | - | level 0 of every column family is empty | +| Client | Compaction | Inspection | +|--------|------------|------------| +| geth | `geth db compact` | `geth db inspect` | +| erigon | `erigon db compact` | `erigon seg du --verbose` | `erigon db compact` rewrites every mdbx database of the datadir without its free pages. It needs **Erigon 3.7.0-dev or newer** (September 2026); an older binary, including every pinned glamsterdam-devnet image, fails the step with `command db not found`. -Reth compaction needs **Reth 2.7.0 or newer** and enough free disk for a compacted copy of `db/mdbx.dat`. It writes the copy to a separate file before swapping it into place; static files are unchanged. - > **On erigon, `before_benchmarks` needs `--exec.no-prune` on the instance.** That phase stops and restarts the client, and erigon refuses to reopen a datadir whose receipt domain it pruned past the snapshot files: `[snapshots] gap between snapshot files and DB for domain receipt: files end at txNum 0 but the DB was pruned up to 390625`. A synthetic snapshot has no snapshot files, so the client's own pruning is enough to create the gap. `--exec.no-prune` disables the state-aggregator pruning that advances the marker. Erigon documents the flag for diagnostic and perf-comparison use, which is what a benchmark is, and it removes housekeeping the benchmark does not want anyway. `before_pre_runs` needs no flag: it compacts before the client ever boots, so nothing restarts. -###### nethermind and besu: RocksDB `ldb` - -Neither client ships an offline compaction command, so their databases are compacted with RocksDB's own `ldb`, run from a separate image (`ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1`, built from [`Dockerfile.rocksdb-ldb`](../Dockerfile.rocksdb-ldb); `image` overrides it). Every directory under the datadir holding an `OPTIONS-*` file is a database — nethermind has one per store (state, code, blocks, receipts, ...), besu one with a column family per segment — and each of its column families is compacted with `--try_load_options`, i.e. with the options the client wrote it with. `extra_args` reach every `ldb compact`. Each family is logged before and after with its non-empty levels and SST bytes (`L0=12/310MB L6=41/9870MB sst_bytes=…` → `L6=…`) and its duration, so the run output shows what the compaction changed, not only that it ran. - -RocksDB 11.8.1 is the version nethermind binds; besu's rocksdbjni 10.6.2 databases open with it too. Nethermind 2.1.0 and Besu 26.9.0 each reopened a compacted mainnet-genesis datadir and served genesis block and state from it. Two guards make a wrong compaction fail rather than pass: - -- A database whose `OPTIONS` names a merge operator `ldb` does not have (e.g. nethermind's log index, off by default) is refused: `ldb` would merge its operands with a string-append operator instead. -- `ldb compact` falls back to the default column family for a name it does not know, so after each family its level 0 must be empty. - -###### geth: the journal check - -geth's path scheme keeps the last 128 blocks as in-memory diff layers and older, not yet flushed state in a write buffer (up to 256 MiB). A clean stop writes both to `/geth/triedb/merkle.journal`, and the next start loads them back into memory, so that state is never read from the database a compaction just rewrote. After the compaction the runner parses the journal (layout version 3, written by geth v1.17.7) and fails the phase if the write buffer is not empty or there are more than 128 diff layers. To pass, run geth with `--cache.gc=0` (a 0-byte buffer) for at least 128 blocks before it stops. The check is opt-in (`verify: true`) in a run and on by default in `benchmarkoor db compact` (`--no-verify` skips it); a datadir on a container volume is not checked. - ###### Outside a run: `benchmarkoor db compact` -The same containers and checks run against a stopped client's datadir without a config file, e.g. when producing a snapshot: +Run the same client-provided maintenance commands against a stopped host datadir without a benchmark config: ```bash -benchmarkoor db compact --client --datadir [--prepare ]... +benchmarkoor db compact --client geth --datadir /path/to/datadir --image ethereum/client-go:v1.17.7 ``` -It exits non-zero on any failure, the geth journal check included. reth copies and swaps its MDBX file; a client without offline compaction (ethrex) is an error. `--image` picks the client image (default: the client's default image), `--compaction-image` the compaction container's (as `image`), and `--extra-arg`, `--timeout` and `--no-inspect` mirror `extra_args`, `timeout` and `inspect`. No marker is written: that belongs to a run. The journal check reads the datadir from the host, so the CLI needs read access to it. +Only Geth and Erigon are supported. Other clients return an error; benchmarkoor does not use generic database tools or replace database files itself. Select the image version that wrote the datadir with `--image`; the default is the client's default image. The client must be stopped before running this command. + +`--prepare`, `--extra-arg`, `--timeout` and `--no-inspect` use the same settings as a run's compaction phase. Container output goes to stdout. Preparation or compaction failures return a non-zero exit status; inspection failures remain warnings, as in a run. No run marker or report is written. ###### Preparation steps (`prepare`) @@ -1193,6 +1178,8 @@ On a **real synced erigon datadir** this is the pairing erigon documents, and it > **Do not enable `seg-retire` for a state-actor or otherwise synthetic snapshot.** Its history is far shorter than one step, so the retire finds nothing to freeze but prunes anyway, and erigon then refuses to reopen its own datadir with the gap error above. Verified in CI on a snapshot advanced to block 39: `retiring blocks from=0 to=39`, `Build state history snapshots` (nothing to build), `Prune state history` (marker advances regardless). The compaction alone is worth running there — it took that datadir's `chaindata` from 2.0GB to 32MB. +Besu was checked and cannot be supported yet: as of Besu 26.6.1, `besu storage` has no compaction subcommand. `trie-log prune` deletes trie logs below the retention limit instead of rewriting the database, which is a different operation and removes history the Bonsai rollback needs. + ```yaml runner: client: @@ -1212,8 +1199,7 @@ runner: | `inspect` | bool | No | `true` | Run the client database inspection before and after each compaction. A failed inspection is logged and never fails the run | | `prepare` | []string | No | - | Client preparation steps to run before each compaction, in order (see [Preparation steps](#preparation-steps-prepare)) | | `timeout` | string | No | `3h` | Cap for one phase's work — every preparation step, the compaction, and both inspections (Go duration). Applies per phase | -| `image` | string | No | the instance image; the `ldb` image for nethermind and besu | Image of the compaction container. The default keeps the tool version and the client version identical | -| `verify` | bool | No | `false` | Run the client's check of the compacted datadir (geth's journal check) | +| `image` | string | No | the instance image | Image of the compaction container. The default keeps the tool version and the client version identical | | `extra_args` | []string | No | - | Extra arguments for the compaction command, e.g. `--cache=16384` | | `continue_on_error` | bool | No | `false` | Downgrade a compaction failure to a warning. A failed compaction makes the results incomparable, so the run fails by default | | `skip_if_marked` | bool | No | `true` with `persist`, else `false` | Skip a phase the datadir marker already names (see below) | diff --git a/pkg/client/besu.go b/pkg/client/besu.go index d42797c09..154ceeb87 100644 --- a/pkg/client/besu.go +++ b/pkg/client/besu.go @@ -117,18 +117,18 @@ func (s *besuSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands compacts Besu's database with RocksDB's ldb: Besu -// ships no offline compaction command. +// DBMaintenanceCommands returns nil: Besu ships no offline compaction command. // // Checked against Besu 26.6.1. `besu storage` offers revert-variables, // rocksdb {usage, x-stats}, trie-log {count, prune, export, import}, // revert-metadata and prune-pre-merge-blocks, and no --Xplugin-rocksdb-* -// option compacts either. `trie-log prune` DELETES trie logs below the -// retention limit rather than rewriting the SSTs, which takes away the history -// the Bonsai rollback needs between tests. +// option compacts either. `trie-log prune` is the nearest thing operators +// reach for, but it DELETES trie logs below the retention limit rather than +// rewriting the SSTs — a different operation, and one that takes away the +// history the Bonsai rollback needs between tests. // -// Besu keeps one RocksDB directory with a column family per segment, named by -// a single byte (0x01..0x12). -func (s *besuSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { - return rocksDBCompactCommands(dataDir) +// `besu --data-path= storage rocksdb usage` would serve as the Inspect +// side, but Compact is what makes a client supported, so this stays nil. +func (s *besuSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { + return nil } diff --git a/pkg/client/client.go b/pkg/client/client.go index 6052d7974..a5dee442d 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -76,21 +76,6 @@ type DBMaintenanceCommands struct { // compaction. Optional: a client that can compact but not inspect leaves // it nil. Inspect []string - - // CompactImage runs Compact in this image instead of the client's, with - // Compact[0] as the entrypoint. It is set for a client that ships no - // offline compactor and is compacted by a separate tool. - CompactImage string - - // OwnEntrypoint runs Compact in the client's image with Compact[0] as the - // entrypoint instead of as an argument to the client's: the command is a - // shell around the client binary (reth's copy-and-swap). CompactImage - // implies it. - OwnEntrypoint bool - - // Verify checks the datadir after the compaction, given its host path, and - // returns a one-line summary of what it found. Optional. - Verify func(hostDataDir string) (string, error) } // Spec provides client-specific container configuration. diff --git a/pkg/client/ethrex.go b/pkg/client/ethrex.go index 9f23ab7d5..3be818014 100644 --- a/pkg/client/ethrex.go +++ b/pkg/client/ethrex.go @@ -98,11 +98,8 @@ func (s *ethrexSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands returns nil: Ethrex (29.x) is one RocksDB with a -// column family, TRANSACTION_LOCATIONS, under its own merge operator -// (tx_locations_merge). ldb has only a string-append operator and would fold -// that family's operands wrongly, so the generic ldb compaction refuses the -// database, and ethrex ships no offline compactor of its own. +// DBMaintenanceCommands returns nil; benchmarkoor has no offline +// compaction command for Ethrex yet. func (s *ethrexSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { return nil } diff --git a/pkg/client/geth.go b/pkg/client/geth.go index 4ab0266fe..dfbeb4d78 100644 --- a/pkg/client/geth.go +++ b/pkg/client/geth.go @@ -1,7 +1,5 @@ package client -import "fmt" - type gethSpec struct{} // NewGethSpec creates a new Geth client specification. @@ -122,30 +120,9 @@ func (s *gethSpec) SnapshotPrepareArgs() []string { // lock, so a running node makes it fail. The commands take --datadir rather // than inheriting the one in DefaultCommand, since a datadir config may mount // the data somewhere other than /data. -// -// Verify fails a datadir whose path-scheme journal holds more than the last -// GethMaxDiffLayers blocks: geth loads that state into memory on start, so -// the compaction never reaches it. func (s *gethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { return &DBMaintenanceCommands{ Compact: []string{"db", "compact", "--datadir=" + dataDir}, Inspect: []string{"db", "inspect", "--datadir=" + dataDir}, - Verify: verifyGethJournal, - } -} - -func verifyGethJournal(hostDataDir string) (string, error) { - j, err := CheckGethJournal(hostDataDir) - if err != nil { - return "", err - } - - if j == nil { - return "no " + GethJournalFile + ": no state is loaded into memory on start", nil } - - return fmt.Sprintf( - "%s: %d diff layers (blocks %d-%d), write buffer empty", - GethJournalFile, j.DiffLayers, j.FirstBlock, j.LastBlock, - ), nil } diff --git a/pkg/client/geth_journal.go b/pkg/client/geth_journal.go deleted file mode 100644 index 0c5187a7b..000000000 --- a/pkg/client/geth_journal.go +++ /dev/null @@ -1,229 +0,0 @@ -package client - -import ( - "bufio" - "errors" - "fmt" - "io" - "os" - "path/filepath" - - "github.com/ethereum/go-ethereum/rlp" -) - -const ( - // GethJournalFile is where geth's path-scheme state database writes its - // in-memory layers on a clean stop, relative to the datadir. The next start - // loads them back into memory. - GethJournalFile = "geth/triedb/merkle.journal" - - // gethJournalVersion is the only journal layout ReadGethJournal parses - // (triedb/pathdb journalVersion, written by geth v1.17.7). - gethJournalVersion = 3 - - // GethMaxDiffLayers is the number of recent blocks geth keeps as diff - // layers (triedb/pathdb maxDiffLayers). - GethMaxDiffLayers = 128 -) - -// GethJournal summarises a path-scheme journal: the diff layers, one per -// block, and the disk layer's write buffer, which holds the state of every -// older block that has not been flushed to the database yet. -type GethJournal struct { - DiffLayers int - FirstBlock uint64 - LastBlock uint64 - - // What the write buffer holds: tries with dirty nodes, accounts, and - // accounts with dirty storage slots. All zero when it is empty. - BufferTries int - BufferAccounts int - BufferStorages int -} - -// BufferEmpty reports whether everything below the diff layers is on disk. -func (j *GethJournal) BufferEmpty() bool { - return j.BufferTries == 0 && j.BufferAccounts == 0 && j.BufferStorages == 0 -} - -// CheckGethJournal fails when the journal in dataDir holds the state of more -// than the last GethMaxDiffLayers blocks, i.e. when the write buffer below the -// diff layers is not empty: geth would serve that state from memory, so a -// compaction never reaches it. A datadir without a journal passes, and the -// returned journal is nil. -func CheckGethJournal(dataDir string) (*GethJournal, error) { - path := filepath.Join(dataDir, GethJournalFile) - - j, err := ReadGethJournal(path) - if errors.Is(err, os.ErrNotExist) { - return nil, nil - } - - if err != nil { - return nil, err - } - - if j.DiffLayers > GethMaxDiffLayers || !j.BufferEmpty() { - return j, fmt.Errorf( - "%s holds the state of more than the last %d blocks, which geth loads back into memory"+ - " on start: %d diff layers (blocks %d-%d) over a write buffer of %d tries,"+ - " %d accounts and %d storages; flush it by running geth with --cache.gc=0 for"+ - " at least %d blocks before stopping it", - path, GethMaxDiffLayers, j.DiffLayers, j.FirstBlock, j.LastBlock, - j.BufferTries, j.BufferAccounts, j.BufferStorages, GethMaxDiffLayers, - ) - } - - return j, nil -} - -// ReadGethJournal parses a path-scheme journal (triedb/pathdb/journal.go): -// the version, the disk root, the disk layer (root, state id, nodes, states) -// and then one entry per diff layer, oldest first. -func ReadGethJournal(path string) (*GethJournal, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - - defer func() { _ = f.Close() }() - - s := rlp.NewStream(bufio.NewReaderSize(f, 1<<20), 0) - - version, err := s.Uint64() - if err != nil { - return nil, fmt.Errorf("%s: reading version: %w", path, err) - } - - if version != gethJournalVersion { - return nil, fmt.Errorf( - "%s: journal version %d, only %d can be checked", path, version, gethJournalVersion, - ) - } - - j := &GethJournal{} - - // Disk root, then the disk layer's root and state id. - for _, what := range []string{"disk root", "disk layer root"} { - if _, err := s.Bytes(); err != nil { - return nil, fmt.Errorf("%s: reading %s: %w", path, what, err) - } - } - - if _, err := s.Uint64(); err != nil { - return nil, fmt.Errorf("%s: reading disk layer id: %w", path, err) - } - - if j.BufferTries, err = listLen(s); err != nil { - return nil, fmt.Errorf("%s: reading buffered nodes: %w", path, err) - } - - if j.BufferAccounts, j.BufferStorages, err = readStateSet(s); err != nil { - return nil, fmt.Errorf("%s: reading buffered states: %w", path, err) - } - - for { - if _, err := s.Bytes(); errors.Is(err, io.EOF) { - return j, nil - } else if err != nil { - return nil, fmt.Errorf("%s: reading diff layer %d root: %w", path, j.DiffLayers, err) - } - - block, err := s.Uint64() - if err != nil { - return nil, fmt.Errorf("%s: reading diff layer %d block: %w", path, j.DiffLayers, err) - } - - if err := skipDiffLayerBody(s); err != nil { - return nil, fmt.Errorf("%s: reading diff layer %d (block %d): %w", path, j.DiffLayers, block, err) - } - - if j.DiffLayers == 0 { - j.FirstBlock = block - } - - j.LastBlock = block - j.DiffLayers++ - } -} - -// skipDiffLayerBody consumes a diff layer's nodes, their optional origins -// (a list where the state set's leading bool would be), its state set and -// the state set's origins. -func skipDiffLayerBody(s *rlp.Stream) error { - if _, err := s.Raw(); err != nil { - return fmt.Errorf("nodes: %w", err) - } - - kind, _, err := s.Kind() - if err != nil { - return err - } - - if kind == rlp.List { - if _, err := s.Raw(); err != nil { - return fmt.Errorf("node origins: %w", err) - } - } - - if _, _, err := readStateSet(s); err != nil { - return err - } - - for _, what := range []string{"account origins", "storage origins"} { - if _, err := s.Raw(); err != nil { - return fmt.Errorf("%s: %w", what, err) - } - } - - return nil -} - -// readStateSet consumes a stateSet (raw-key flag, accounts, storages) and -// returns how many accounts and storages it holds. -func readStateSet(s *rlp.Stream) (accounts, storages int, err error) { - if _, err := s.Bool(); err != nil { - return 0, 0, fmt.Errorf("raw storage key flag: %w", err) - } - - // struct{ AddrHashes []common.Hash; Accounts [][]byte } - raw, err := s.Raw() - if err != nil { - return 0, 0, fmt.Errorf("accounts: %w", err) - } - - fields, _, err := rlp.SplitList(raw) - if err != nil { - return 0, 0, fmt.Errorf("accounts: %w", err) - } - - _, hashes, _, err := rlp.Split(fields) - if err != nil { - return 0, 0, fmt.Errorf("account hashes: %w", err) - } - - if accounts, err = rlp.CountValues(hashes); err != nil { - return 0, 0, fmt.Errorf("account hashes: %w", err) - } - - if storages, err = listLen(s); err != nil { - return 0, 0, fmt.Errorf("storages: %w", err) - } - - return accounts, storages, nil -} - -// listLen consumes a list and returns its number of elements. -func listLen(s *rlp.Stream) (int, error) { - raw, err := s.Raw() - if err != nil { - return 0, err - } - - content, _, err := rlp.SplitList(raw) - if err != nil { - return 0, err - } - - return rlp.CountValues(content) -} diff --git a/pkg/client/geth_journal_test.go b/pkg/client/geth_journal_test.go deleted file mode 100644 index 8acd3b40f..000000000 --- a/pkg/client/geth_journal_test.go +++ /dev/null @@ -1,220 +0,0 @@ -package client - -import ( - "bytes" - "os" - "path/filepath" - "testing" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/rlp" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// The types below mirror what geth v1.17.7's triedb/pathdb journal() writes, -// so the tests build journals the way geth does. - -type testJournalNode struct { - Path []byte - Blob []byte -} - -type testJournalNodes struct { - Owner common.Hash - Nodes []testJournalNode -} - -type testAccounts struct { - AddrHashes []common.Hash - Accounts [][]byte -} - -type testStorage struct { - AddrHash common.Hash - Keys []common.Hash - Vals [][]byte -} - -type testOriginAccounts struct { - Addresses []common.Address - Accounts [][]byte -} - -type testOriginStorage struct { - Address common.Address - Keys []common.Hash - Vals [][]byte -} - -type testLayerState struct { - nodes []testJournalNodes - accounts testAccounts - storages []testStorage -} - -func dirtyState(i byte) testLayerState { - return testLayerState{ - nodes: []testJournalNodes{{Nodes: []testJournalNode{{Path: []byte{i}, Blob: []byte{0xc0}}}}}, - accounts: testAccounts{AddrHashes: []common.Hash{{i}}, Accounts: [][]byte{{i}}}, - storages: []testStorage{{AddrHash: common.Hash{i}, Keys: []common.Hash{{1}}, Vals: [][]byte{{2}}}}, - } -} - -func emptyState() testLayerState { - return testLayerState{nodes: []testJournalNodes{}, accounts: testAccounts{}, storages: []testStorage{}} -} - -type testJournal struct { - version uint64 - buffer testLayerState - diffs int - noOrigin bool // pre-origin layout: no node origins after the nodes -} - -func (j testJournal) encode(t *testing.T) []byte { - t.Helper() - - var b bytes.Buffer - - enc := func(v any) { require.NoError(t, rlp.Encode(&b, v)) } - - state := func(s testLayerState) { - enc(false) - enc(s.accounts) - enc(s.storages) - } - - enc(j.version) - enc(common.Hash{0xd1}) // disk root - enc(common.Hash{0xd1}) // disk layer root - enc(uint64(1000)) // disk layer state id - enc(j.buffer.nodes) - state(j.buffer) - - for i := range j.diffs { - s := dirtyState(byte(i)) - - enc(common.Hash{byte(i), 0xff}) - enc(uint64(500 + i)) - enc(s.nodes) - - if !j.noOrigin { - enc(s.nodes) - } - - state(s) - enc(testOriginAccounts{Addresses: []common.Address{{byte(i)}}, Accounts: [][]byte{{}}}) - enc([]testOriginStorage{{Address: common.Address{byte(i)}, Keys: []common.Hash{{1}}, Vals: [][]byte{{}}}}) - } - - return b.Bytes() -} - -func writeJournal(t *testing.T, data []byte) string { - t.Helper() - - dir := t.TempDir() - path := filepath.Join(dir, GethJournalFile) - require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) - require.NoError(t, os.WriteFile(path, data, 0o644)) - - return dir -} - -func TestCheckGethJournal(t *testing.T) { - t.Run("the last 128 blocks over an empty buffer pass", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 128}.encode(t)) - - j, err := CheckGethJournal(dir) - require.NoError(t, err) - assert.Equal(t, &GethJournal{DiffLayers: 128, FirstBlock: 500, LastBlock: 627}, j) - }) - - t.Run("fewer blocks than that pass", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 3}.encode(t)) - - j, err := CheckGethJournal(dir) - require.NoError(t, err) - assert.Equal(t, 3, j.DiffLayers) - }) - - t.Run("a disk layer alone passes", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState()}.encode(t)) - - j, err := CheckGethJournal(dir) - require.NoError(t, err) - assert.Zero(t, j.DiffLayers) - }) - - t.Run("an unflushed write buffer fails", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: dirtyState(7), diffs: 128}.encode(t)) - - j, err := CheckGethJournal(dir) - require.Error(t, err) - assert.Contains(t, err.Error(), "128 diff layers (blocks 500-627) over a write buffer of 1 tries, 1 accounts and 1 storages") - assert.Equal(t, 1, j.BufferTries) - }) - - t.Run("a buffer holding only accounts fails", func(t *testing.T) { - buffer := emptyState() - buffer.accounts = dirtyState(1).accounts - dir := writeJournal(t, testJournal{version: 3, buffer: buffer, diffs: 1}.encode(t)) - - _, err := CheckGethJournal(dir) - require.Error(t, err) - }) - - t.Run("more than 128 diff layers fail", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 129}.encode(t)) - - _, err := CheckGethJournal(dir) - require.ErrorContains(t, err, "129 diff layers") - }) - - t.Run("diff layers without node origins parse", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 5, noOrigin: true}.encode(t)) - - j, err := CheckGethJournal(dir) - require.NoError(t, err) - assert.Equal(t, 5, j.DiffLayers) - }) - - t.Run("no journal passes", func(t *testing.T) { - j, err := CheckGethJournal(t.TempDir()) - require.NoError(t, err) - assert.Nil(t, j) - }) - - t.Run("another journal version is not guessed at", func(t *testing.T) { - dir := writeJournal(t, testJournal{version: 2, buffer: emptyState()}.encode(t)) - - _, err := CheckGethJournal(dir) - require.ErrorContains(t, err, "journal version 2, only 3 can be checked") - }) - - t.Run("a truncated journal fails", func(t *testing.T) { - data := testJournal{version: 3, buffer: emptyState(), diffs: 2}.encode(t) - dir := writeJournal(t, data[:len(data)-5]) - - _, err := CheckGethJournal(dir) - require.ErrorContains(t, err, "diff layer 1") - }) -} - -func TestGethVerifiesItsJournal(t *testing.T) { - spec, err := NewRegistry().Get(ClientGeth) - require.NoError(t, err) - - verify := spec.DBMaintenanceCommands("/data").Verify - require.NotNil(t, verify) - - summary, err := verify(t.TempDir()) - require.NoError(t, err) - assert.Contains(t, summary, "no geth/triedb/merkle.journal") - - dir := writeJournal(t, testJournal{version: 3, buffer: emptyState(), diffs: 128}.encode(t)) - summary, err = verify(dir) - require.NoError(t, err) - assert.Equal(t, "geth/triedb/merkle.journal: 128 diff layers (blocks 500-627), write buffer empty", summary) -} diff --git a/pkg/client/nethermind.go b/pkg/client/nethermind.go index 833f53e70..db7599b8d 100644 --- a/pkg/client/nethermind.go +++ b/pkg/client/nethermind.go @@ -106,9 +106,8 @@ func (s *nethermindSpec) SnapshotPrepareArgs() []string { return nil } -// DBMaintenanceCommands compacts Nethermind's databases with RocksDB's ldb: -// Nethermind ships no offline compaction command. Each database (state, code, -// blocks, receipts, ...) is its own RocksDB directory under the datadir. -func (s *nethermindSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { - return rocksDBCompactCommands(dataDir) +// DBMaintenanceCommands returns nil; benchmarkoor has no offline +// compaction command for Nethermind yet. +func (s *nethermindSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { + return nil } diff --git a/pkg/client/reth.go b/pkg/client/reth.go index c05641c2b..f71836106 100644 --- a/pkg/client/reth.go +++ b/pkg/client/reth.go @@ -106,40 +106,8 @@ func (s *rethSpec) SnapshotPrepareArgs() []string { } } -// DBMaintenanceCommands compacts Reth's MDBX database with `reth db copy -// --compact` (mdbx_copy -c, bundled since Reth 2.7.0): the database file is -// rewritten without its free pages to /db.compact and swapped in. -// The static files are append-only segments and are left alone. -// -// MDBX never shrinks: the pages a replay frees stay in mdbx.dat, and in every -// archive of it. The copy needs that much free space under the datadir, so the -// script fails before copying when the filesystem has less than the file's size. -// -// `reth db stats` is the inspection: tables, entries, sizes and the freelist. -// --datadir belongs to `db`, not to its subcommands (clap rejects it there). -func (s *rethSpec) DBMaintenanceCommands(dataDir string) *DBMaintenanceCommands { - return &DBMaintenanceCommands{ - Compact: []string{"sh", "-c", rethCompactScript, "reth-compact", dataDir}, - OwnEntrypoint: true, - Inspect: []string{"db", "--datadir=" + dataDir, "stats"}, - } +// DBMaintenanceCommands returns nil; benchmarkoor has no offline +// compaction command for Reth yet. +func (s *rethSpec) DBMaintenanceCommands(_ string) *DBMaintenanceCommands { + return nil } - -// rethCompactScript is the copy-and-swap around `reth db copy --compact`, in -// the reth image's /bin/sh. $1 is the datadir. Reth opens MDBX in no-subdir -// mode (/db/mdbx.dat), so mdbx_env_copy writes one file at the -// destination path and fails with EEXIST if it exists; the file then replaces -// db/mdbx.dat and the stale db/mdbx.lck goes. -const rethCompactScript = `set -eu -d=$1 -dat=$d/db/mdbx.dat -[ -f "$dat" ] || { echo "no $dat: not a reth datadir" >&2; exit 1; } -size=$(stat -c %s "$dat") -free=$(df -B1 --output=avail "$d" | tail -n 1) -[ "$free" -gt "$size" ] || { echo "need $size bytes free under $d for the compacted copy, have $free" >&2; exit 1; } -rm -f "$d/db.compact" -reth db --datadir="$d" copy --compact "$d/db.compact" -[ -s "$d/db.compact" ] || { echo "reth db copy wrote no $d/db.compact" >&2; exit 1; } -mv "$dat" "$dat.old" && mv "$d/db.compact" "$dat" && rm -f "$dat.old" "$d/db/mdbx.lck" -echo "reth-compact: mdbx.dat $size -> $(stat -c %s "$dat") bytes" -` diff --git a/pkg/client/rocksdb.go b/pkg/client/rocksdb.go deleted file mode 100644 index 8d50c4ee7..000000000 --- a/pkg/client/rocksdb.go +++ /dev/null @@ -1,21 +0,0 @@ -package client - -import _ "embed" - -// RocksDBLdbImage is the RocksDB `ldb` tool that compacts nethermind and besu, -// built from Dockerfile.rocksdb-ldb. 11.8.1 is the RocksDB nethermind binds; -// besu's rocksdbjni 10.6.2 databases open with it too. -const RocksDBLdbImage = "ghcr.io/ethpandaops/benchmarkoor-rocksdb-ldb:11.8.1" - -//go:embed rocksdb_compact.sh -var rocksDBCompactScript string - -// rocksDBCompactCommands compacts every column family of every RocksDB -// database under dataDir with ldb, using the options each was written with. -// db_compaction.extra_args reach every `ldb compact`. -func rocksDBCompactCommands(dataDir string) *DBMaintenanceCommands { - return &DBMaintenanceCommands{ - Compact: []string{"sh", "-c", rocksDBCompactScript, "rocksdb-compact", dataDir}, - CompactImage: RocksDBLdbImage, - } -} diff --git a/pkg/client/rocksdb_compact.sh b/pkg/client/rocksdb_compact.sh deleted file mode 100644 index 534bc9783..000000000 --- a/pkg/client/rocksdb_compact.sh +++ /dev/null @@ -1,87 +0,0 @@ -# Compacts every column family of every RocksDB database under a datadir, with -# the options each database was written with, for clients that ship no offline -# compactor (nethermind, besu). The client must be stopped. -# -# sh -c "$(cat rocksdb_compact.sh)" rocksdb-compact [ldb option]... -# -# Extra arguments are passed to every `ldb compact`. -set -eu - -[ $# -ge 1 ] || { echo "usage: rocksdb-compact [ldb option]..." >&2; exit 2; } -root=$1 -shift - -nl=' -' - -# A column family name is printed as hex when it is not printable: besu names -# its families by single bytes (0x01..0x12), one of which is a newline. -label() { - case $1 in - *[![:print:]]* | '') printf '0x%s' "$(printf %s "$1" | od -An -tx1 | tr -d ' \n')" ;; - *) printf %s "$1" ;; - esac -} - -# levels : the family's non-empty levels as L=/, then -# its SST bytes. Logged before and after, so the run output shows what the -# compaction did, not only that it ran. -levels() { - # Capture each command separately: POSIX pipelines hide an ldb failure - # behind awk/sed's successful exit status. - level_stats=$(ldb --db="$1" --try_load_options --column_family="$2" get_property rocksdb.levelstats 2 && $2 > 0 { printf "L%s=%s/%sMB ", $1, $2, $3 }' - printf '%s\n' "$sst_bytes" | - sed 's/.*: /sst_bytes=/' -} - -dbs=$(find "$root" -type f -name 'OPTIONS-*' -exec dirname {} \; | sort -u) -[ -n "$dbs" ] || { echo "no RocksDB database (no OPTIONS-* file) under $root" >&2; exit 1; } - -echo "$dbs" | while IFS= read -r db; do - # ldb opens a family that has no merge operator with a string-append one - # (and records it in OPTIONS), so a family with a merge operator of its own - # would have its operands merged wrongly. - opts=$(ls "$db"/OPTIONS-* | sort -V | tail -n 1) - foreign=$(grep -E '^[[:space:]]*merge_operator=' "$opts" | - grep -vxE '[[:space:]]*merge_operator=(nullptr|\{id=StringAppendOperator;delimiter=\\:;\})' || true) - if [ -n "$foreign" ]; then - echo "$db: refusing to compact: $opts sets a merge operator ldb does not have:$nl$foreign" >&2 - exit 1 - fi - - # ldb prints `Column families in : ` and then `{a, b, c}`, names raw, - # and exits 0 even when it cannot read the database. - out=$(ldb --db="$db" list_column_families &2; exit 1 ;; - esac - cfs=${out#*": $nl{"} - cfs=${cfs%"}"} - - while :; do - case $cfs in - *", "*) cf=${cfs%%", "*}; cfs=${cfs#*", "} ;; - *) cf=$cfs; cfs= ;; - esac - - before=$(levels "$db" "$cf") - echo "compacting $db column_family=$(label "$cf") $before" - start=$(date +%s) - ldb --db="$db" --try_load_options --column_family="$cf" "$@" compact &2; exit 1 ;; - esac - echo "compacted $db column_family=$(label "$cf") $after duration=$(( $(date +%s) - start ))s" - - [ -n "$cfs" ] || break - done -done diff --git a/pkg/client/rocksdb_test.go b/pkg/client/rocksdb_test.go deleted file mode 100644 index 73b34f565..000000000 --- a/pkg/client/rocksdb_test.go +++ /dev/null @@ -1,224 +0,0 @@ -package client - -import ( - "bytes" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRocksDBClientsCompactWithLdb(t *testing.T) { - for _, ct := range []ClientType{ClientNethermind, ClientBesu} { - spec, err := NewRegistry().Get(ct) - require.NoError(t, err) - - cmds := spec.DBMaintenanceCommands("/data") - require.NotNil(t, cmds, ct) - - assert.Equal(t, RocksDBLdbImage, cmds.CompactImage, ct) - assert.Equal(t, []string{"sh", "-c", rocksDBCompactScript, "rocksdb-compact", "/data"}, cmds.Compact, ct) - assert.Empty(t, cmds.Prepare, ct) - assert.Empty(t, cmds.Inspect, ct) - assert.Nil(t, cmds.Verify, ct) - assert.True(t, SupportsDBCompaction(ct), ct) - } -} - -// stubLdb answers list_column_families with `listing` (raw bytes, as ldb -// prints them), get_property levelstats with $LDB_LEVEL0 files at level 0 and -// two at level 6, and records every compact argv, -// NUL-separated with one line per call, in $LDB_CALLS. -const stubLdb = `#!/bin/sh -for a in "$@"; do - case $a in - list_column_families) printf '%s' "$LDB_LISTING"; exit 0 ;; - get_property) - case "$*" in - *"$LDB_FAIL_PROPERTY"*) - if [ -n "$LDB_FAIL_PROPERTY" ] && { [ "$LDB_FAIL_PHASE" = before ] || [ -f "$LDB_CALLS" ]; }; then - echo "property read failed" >&2; exit 1 - fi ;; - esac - case "$*" in - *levelstats*) printf 'rocksdb.levelstats: Level Files Size(MB)\n--------------------\n 0 %s 0\n 6 2 10\n' "$LDB_LEVEL0" ;; - *total-sst-files-size*) echo "rocksdb.total-sst-files-size: 4096" ;; - esac - exit 0 ;; - compact) for b in "$@"; do printf '%s\0' "$b"; done >>"$LDB_CALLS"; echo >>"$LDB_CALLS"; exit 0 ;; - esac -done -exit 3 -` - -type ldbStub struct { - datadir string - calls string - env []string -} - -func newLdbStub(t *testing.T, listing string, dbs ...string) *ldbStub { - t.Helper() - - bin := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(bin, "ldb"), []byte(stubLdb), 0o755)) - - s := &ldbStub{datadir: t.TempDir(), calls: filepath.Join(t.TempDir(), "calls")} - - for _, db := range dbs { - dir := filepath.Join(s.datadir, db) - require.NoError(t, os.MkdirAll(dir, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "OPTIONS-000007"), []byte("[CFOptions \"default\"]\n merge_operator=nullptr\n"), 0o644)) - } - - s.env = append(os.Environ(), - "PATH="+bin+":"+os.Getenv("PATH"), - "LDB_LISTING="+listing, - "LDB_LEVEL0=0", - "LDB_CALLS="+s.calls, - "LDB_FAIL_PROPERTY=", - "LDB_FAIL_PHASE=", - ) - - return s -} - -func (s *ldbStub) run(t *testing.T, args ...string) (string, error) { - t.Helper() - - cmd := exec.Command("sh", append([]string{"-c", rocksDBCompactScript, "rocksdb-compact", s.datadir}, args...)...) - cmd.Env = s.env - - out, err := cmd.CombinedOutput() - - return string(out), err -} - -// compacted returns the --column_family of every compact call, in order. -func (s *ldbStub) compacted(t *testing.T) []string { - t.Helper() - - data, err := os.ReadFile(s.calls) - require.NoError(t, err) - - var cfs []string - - for _, call := range bytes.Split(bytes.TrimSuffix(data, []byte("\x00\n")), []byte("\x00\n")) { - for _, arg := range strings.Split(string(call), "\x00") { - if cf, ok := strings.CutPrefix(arg, "--column_family="); ok { - cfs = append(cfs, cf) - } - } - } - - return cfs -} - -// TestRocksDBCompactScript_ParsesColumnFamilies feeds the script ldb's real -// output shape: a header line, then `{a, b, c}` with names printed raw. Besu -// names its families by single bytes, so one of them is a newline and another -// a tab. -func TestRocksDBCompactScript_ParsesColumnFamilies(t *testing.T) { - besu := []string{"default", "\x01", "\x09", "\x0a", "\x12"} - s := newLdbStub(t, "Column families in /data/database: \n{"+strings.Join(besu, ", ")+"}\n", "database") - - out, err := s.run(t) - require.NoError(t, err, out) - - assert.Equal(t, besu, s.compacted(t)) - assert.Contains(t, out, "column_family=0x0a") - assert.Contains(t, out, "column_family=default L6=2/10MB sst_bytes=4096\n", "levels before") - assert.Regexp(t, `compacted \S+ column_family=default L6=2/10MB sst_bytes=4096 duration=\d+s`, out, "levels and duration after") -} - -func TestRocksDBCompactScript_EveryDatabase(t *testing.T) { - s := newLdbStub(t, "Column families in x: \n{default, Blocks, with space}\n", - "nethermind_db/mainnet/state/0", "nethermind_db/mainnet/receipts") - - out, err := s.run(t, "--max_open_files=-1") - require.NoError(t, err, out) - - assert.Equal(t, []string{ - "default", "Blocks", "with space", // receipts - "default", "Blocks", "with space", // state/0 - }, s.compacted(t)) - - data, err := os.ReadFile(s.calls) - require.NoError(t, err) - assert.Contains(t, string(data), "--max_open_files=-1\x00compact\x00", "extra args reach ldb") - assert.Contains(t, string(data), "--try_load_options\x00") -} - -func TestRocksDBCompactScript_Failures(t *testing.T) { - for _, property := range []string{"levelstats", "total-sst-files-size"} { - for _, phase := range []string{"before", "after"} { - t.Run(property+" fails "+phase+" compaction", func(t *testing.T) { - s := newLdbStub(t, "Column families in db: \n{default}\n", "db") - s.env = append(s.env, "LDB_FAIL_PROPERTY="+property, "LDB_FAIL_PHASE="+phase) - - out, err := s.run(t) - require.Error(t, err, out) - assert.Contains(t, out, "property read failed") - assert.NotContains(t, out, "compacted ") - if phase == "before" { - assert.NoFileExists(t, s.calls, "no compaction without readable properties") - } else { - assert.Equal(t, []string{"default"}, s.compacted(t)) - } - }) - } - } - - t.Run("no database", func(t *testing.T) { - s := newLdbStub(t, "") - - out, err := s.run(t) - require.Error(t, err) - assert.Contains(t, out, "no RocksDB database") - }) - - t.Run("unreadable database", func(t *testing.T) { - // ldb prints the error to stderr and exits 0. - s := newLdbStub(t, "", "db") - - out, err := s.run(t) - require.Error(t, err) - assert.Contains(t, out, "cannot list column families") - }) - - t.Run("level 0 not empty after compaction", func(t *testing.T) { - s := newLdbStub(t, "Column families in db: \n{default}\n", "db") - s.env = append(s.env, "LDB_LEVEL0=2") - - out, err := s.run(t) - require.Error(t, err) - assert.Contains(t, out, "level 0 not empty after compaction: L0=2/0MB L6=2/10MB sst_bytes=4096") - }) - - t.Run("a merge operator ldb does not have", func(t *testing.T) { - s := newLdbStub(t, "Column families in db: \n{default}\n", "db") - opts := filepath.Join(s.datadir, "db", "OPTIONS-000012") - require.NoError(t, os.WriteFile(opts, []byte(" merge_operator=nullptr\n merge_operator=LogIndexMergeOperator\n"), 0o644)) - - out, err := s.run(t) - require.Error(t, err) - assert.Contains(t, out, "merge_operator=LogIndexMergeOperator") - assert.NoFileExists(t, s.calls, "nothing is compacted") - }) -} - -// ldb itself records its string-append operator in OPTIONS, so a database it -// has compacted before compacts again. -func TestRocksDBCompactScript_AcceptsLdbsOwnMergeOperator(t *testing.T) { - s := newLdbStub(t, "Column families in db: \n{default}\n", "db") - opts := filepath.Join(s.datadir, "db", "OPTIONS-000012") - require.NoError(t, os.WriteFile(opts, []byte(" merge_operator={id=StringAppendOperator;delimiter=\\:;}\n"), 0o644)) - - out, err := s.run(t) - require.NoError(t, err, out) - assert.Equal(t, []string{"default"}, s.compacted(t)) -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 037cd1258..7a2a80d1e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1486,8 +1486,7 @@ const ( // the global one; it does not merge field by field. // // Only clients whose spec returns compaction commands support this. Today -// that is geth, erigon, nethermind and besu; the last two have no offline -// compactor of their own and are compacted with RocksDB's ldb. +// that is geth and erigon. type DBCompactionConfig struct { Enabled bool `yaml:"enabled" mapstructure:"enabled" json:"enabled"` @@ -1526,17 +1525,9 @@ type DBCompactionConfig struct { // Image overrides the image of the compaction container. Empty uses the // instance image, which keeps the tool version and the client version - // identical, or for nethermind and besu the RocksDB ldb image - // (client.RocksDBLdbImage). + // identical. Image string `yaml:"image,omitempty" mapstructure:"image" json:"image,omitempty"` - // Verify runs the client's check of the compacted datadir. geth's fails - // when triedb/merkle.journal holds the state of more than the last 128 - // blocks, which geth would load back into memory on start. Default: false, - // since a node stopped with a non-empty write buffer (any default-cache run) - // fails it; benchmarkoor db compact turns it on. - Verify *bool `yaml:"verify,omitempty" mapstructure:"verify" json:"verify,omitempty"` - // ExtraArgs appends arguments to the compaction command, e.g. // ["--cache=16384"] for geth. ExtraArgs []string `yaml:"extra_args,omitempty" mapstructure:"extra_args" json:"extra_args,omitempty"` @@ -1689,16 +1680,6 @@ func (c *DBCompactionConfig) InspectEnabled() bool { return *c.Inspect } -// VerifyEnabled reports whether to run the client's check of the compacted -// datadir. Defaults to false. -func (c *DBCompactionConfig) VerifyEnabled() bool { - if c == nil || c.Verify == nil { - return false - } - - return *c.Verify -} - // PrepareSteps returns the preparation steps to run before each compaction, in // the configured order. Nil means none, which is the default. func (c *DBCompactionConfig) PrepareSteps() []string { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index bcffc2929..9d8e580db 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -3763,7 +3763,7 @@ func TestValidateDBCompaction(t *testing.T) { }, { name: "unsupported client", - client: "ethrex", + client: "besu", cfg: &DBCompactionConfig{Enabled: true}, wantErr: true, errSubstr: "not supported for client", @@ -3997,7 +3997,7 @@ func TestValidateDBCompaction_SkipsInactiveInstances(t *testing.T) { Client: ClientConfig{ Config: ClientDefaults{DBCompaction: &DBCompactionConfig{Enabled: true}}, }, - Instances: []ClientInstance{{ID: "ethrex", Client: "ethrex"}}, + Instances: []ClientInstance{{ID: "besu", Client: "besu"}}, }, } diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index 985c306f6..7fd119a34 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -168,21 +168,6 @@ func (req *dbCompactionRequest) image() string { return req.ImageName } -// compactImage returns the image of the compaction container: the tool image -// of a client compacted by a separate tool, unless db_compaction.image names -// another. -func (req *dbCompactionRequest) compactImage(cmds *client.DBMaintenanceCommands) string { - if req.Cfg != nil && req.Cfg.Image != "" { - return req.Cfg.Image - } - - if cmds.CompactImage != "" { - return cmds.CompactImage - } - - return req.ImageName -} - // hostPath returns the host path of the datadir mount, or "" when // the datadir is a container volume. The marker and the size measurements need // a path the runner can read. @@ -242,7 +227,7 @@ func (r *runner) runDBCompaction( report := &dbCompactionReport{ Phase: req.Phase, Client: req.Instance.Client, - Image: req.compactImage(cmds), + Image: req.image(), RunID: req.RunID, StartedAt: started.UTC().Format(time.RFC3339), Persisted: req.Persisting, @@ -395,18 +380,11 @@ func (r *runner) runDBCompactionContainers( ctx, cancel := context.WithTimeout(ctx, req.Cfg.EffectiveTimeout()) defer cancel() - compactImage := req.compactImage(cmds) - if compactImage != req.ImageName { - if err := r.containerMgr.PullImage(ctx, compactImage, "if-not-present"); err != nil { - return fmt.Errorf("pulling compaction image: %w", err) - } - } - if req.Cfg.InspectEnabled() && len(cmds.Inspect) > 0 { log.Info("Inspecting the database before compaction") if err := r.runDBMaintenanceContainer( - ctx, req, "inspect-before", req.image(), req.Instance.Entrypoint, cmds.Inspect, + ctx, req, "inspect-before", cmds.Inspect, filepath.Join(phaseDir, "inspect-before.txt"), ); err != nil { log.WithError(err).Warn("Database inspection before compaction failed") @@ -425,7 +403,7 @@ func (r *runner) runDBCompactionContainers( }).Info("Preparing the database for compaction") if err := r.runDBMaintenanceContainer( - ctx, req, step.Name, req.image(), req.Instance.Entrypoint, step.Args, + ctx, req, step.Name, step.Args, filepath.Join(phaseDir, step.Name+".log"), ); err != nil { return fmt.Errorf("preparation step %q: %w", step.Name, err) @@ -434,27 +412,18 @@ func (r *runner) runDBCompactionContainers( log.WithField("timeout", timeout).Info("Compacting the database") - entrypoint, command := req.Instance.Entrypoint, dbCompactionCommand(cmds, req.Cfg) - if cmds.CompactImage != "" || cmds.OwnEntrypoint { - entrypoint, command = command[:1], command[1:] - } - if err := r.runDBMaintenanceContainer( - ctx, req, "compact", compactImage, entrypoint, command, + ctx, req, "compact", dbCompactionCommand(cmds, req.Cfg), filepath.Join(phaseDir, "compact.log"), ); err != nil { return err } - if err := verifyDBCompaction(req, cmds, log); err != nil { - return err - } - if req.Cfg.InspectEnabled() && len(cmds.Inspect) > 0 { log.Info("Inspecting the database after compaction") if err := r.runDBMaintenanceContainer( - ctx, req, "inspect-after", req.image(), req.Instance.Entrypoint, cmds.Inspect, + ctx, req, "inspect-after", cmds.Inspect, filepath.Join(phaseDir, "inspect-after.txt"), ); err != nil { log.WithError(err).Warn("Database inspection after compaction failed") @@ -464,32 +433,6 @@ func (r *runner) runDBCompactionContainers( return nil } -// verifyDBCompaction runs the client's check of the compacted datadir. It -// needs the datadir on the host, so a container volume is not checked. -func verifyDBCompaction( - req *dbCompactionRequest, cmds *client.DBMaintenanceCommands, log logrus.FieldLogger, -) error { - if cmds.Verify == nil || !req.Cfg.VerifyEnabled() { - return nil - } - - hostPath := req.hostPath() - if hostPath == "" { - log.Warn("Not verifying the compacted database: the datadir is a container volume") - - return nil - } - - summary, err := cmds.Verify(hostPath) - if err != nil { - return fmt.Errorf("verifying the compacted database: %w", err) - } - - log.WithField("result", summary).Info("Verified the compacted database") - - return nil -} - // runDBMaintenanceContainer runs one database command in a one-shot container // on the datadir mount and writes its output to outputFile. // @@ -498,8 +441,8 @@ func verifyDBCompaction( func (r *runner) runDBMaintenanceContainer( ctx context.Context, req *dbCompactionRequest, - step, image string, - entrypoint, command []string, + step string, + command []string, outputFile string, ) error { name := fmt.Sprintf( @@ -508,8 +451,8 @@ func (r *runner) runDBMaintenanceContainer( spec := &docker.ContainerSpec{ Name: name, - Image: image, - Entrypoint: entrypoint, + Image: req.image(), + Entrypoint: req.Instance.Entrypoint, Command: command, Mounts: []docker.Mount{req.Mount}, NetworkName: r.cfg.ContainerNetwork, @@ -533,7 +476,7 @@ func (r *runner) runDBMaintenanceContainer( }() _, _ = fmt.Fprintf( - out, "# %s %s\n# %v\n\n", image, step, append(append([]string{}, entrypoint...), command...), + out, "# %s %s\n# %v\n\n", req.image(), step, command, ) var stdout, stderr io.Writer = out, out @@ -1018,14 +961,14 @@ type DatadirCompaction struct { Image string // Cfg carries the db_compaction settings that apply outside a run: - // prepare, extra_args, image, inspect, verify and timeout. + // prepare, extra_args, image, inspect and timeout. Cfg *config.DBCompactionConfig } // CompactDatadir runs what a db_compaction phase runs — the inspections, the -// preparation steps, the compaction and the verification, each in the same -// container — against a host datadir. Container output goes to stdout. No -// marker or report is written: those belong to a run. +// preparation steps and the compaction — against a host datadir, using the +// client image. Container output goes to stdout. No marker or report is +// written: those belong to a run. func CompactDatadir( ctx context.Context, log *logrus.Logger, mgr docker.ContainerManager, c *DatadirCompaction, ) error { @@ -1084,12 +1027,8 @@ func CompactDatadir( Mount: docker.Mount{Type: "bind", Source: dataDir, Target: spec.DataDir()}, } - usesClientImage := req.compactImage(cmds) == image || len(steps) > 0 || - (c.Cfg.InspectEnabled() && len(cmds.Inspect) > 0) - if usesClientImage && c.Cfg.Image == "" { - if err := mgr.PullImage(ctx, image, "if-not-present"); err != nil { - return fmt.Errorf("pulling client image: %w", err) - } + if err := mgr.PullImage(ctx, req.image(), "if-not-present"); err != nil { + return fmt.Errorf("pulling client image: %w", err) } started := time.Now() diff --git a/pkg/runner/db_compaction_cli_test.go b/pkg/runner/db_compaction_cli_test.go new file mode 100644 index 000000000..dfd766c97 --- /dev/null +++ b/pkg/runner/db_compaction_cli_test.go @@ -0,0 +1,102 @@ +package runner + +import ( + "context" + "fmt" + "io" + "testing" + + "github.com/ethpandaops/benchmarkoor/pkg/client" + "github.com/ethpandaops/benchmarkoor/pkg/config" + "github.com/ethpandaops/benchmarkoor/pkg/docker" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type cliCompactionMgr struct { + fakeDBMaintenanceMgr + pulled []string + specs []*docker.ContainerSpec + pullErr error +} + +func (m *cliCompactionMgr) PullImage(_ context.Context, image, _ string) error { + m.pulled = append(m.pulled, image) + + return m.pullErr +} + +func (m *cliCompactionMgr) RunInitContainer( + ctx context.Context, spec *docker.ContainerSpec, stdout, stderr io.Writer, +) error { + m.specs = append(m.specs, spec) + + return m.fakeDBMaintenanceMgr.RunInitContainer(ctx, spec, stdout, stderr) +} + +func TestCompactDatadir_ClientCommands(t *testing.T) { + for _, ct := range []client.ClientType{client.ClientGeth, client.ClientErigon} { + t.Run(string(ct), func(t *testing.T) { + dir := t.TempDir() + mgr := &cliCompactionMgr{} + cfg := &config.DBCompactionConfig{ExtraArgs: []string{"--example-option"}} + require.NoError(t, CompactDatadir(context.Background(), logrus.New(), mgr, &DatadirCompaction{ + Client: ct, DataDir: dir, Image: "client:pinned", Cfg: cfg, + })) + assert.Equal(t, []string{"client:pinned"}, mgr.pulled) + require.Len(t, mgr.specs, 3) + + for _, spec := range mgr.specs { + assert.Equal(t, "client:pinned", spec.Image) + assert.Empty(t, spec.Entrypoint, "use the client image's entrypoint") + require.Len(t, spec.Mounts, 1) + assert.Equal(t, docker.Mount{Type: "bind", Source: dir, Target: "/data"}, spec.Mounts[0]) + } + + assert.Equal(t, []string{"db", "compact", "--datadir=/data", "--example-option"}, mgr.specs[1].Command) + assert.NotContains(t, mgr.specs[0].Command, "--example-option") + assert.NotContains(t, mgr.specs[2].Command, "--example-option") + }) + } +} + +func TestCompactDatadir_RejectsUnsupportedClients(t *testing.T) { + for _, ct := range []client.ClientType{client.ClientBesu, client.ClientNethermind, client.ClientReth, client.ClientEthrex} { + t.Run(string(ct), func(t *testing.T) { + mgr := &cliCompactionMgr{} + err := CompactDatadir(context.Background(), logrus.New(), mgr, &DatadirCompaction{Client: ct}) + require.ErrorContains(t, err, "no database compaction command") + assert.Empty(t, mgr.pulled) + assert.Empty(t, mgr.specs) + }) + } +} + +func TestCompactDatadir_Failures(t *testing.T) { + for _, stage := range []string{"pull", "compact"} { + t.Run(stage, func(t *testing.T) { + mgr := &cliCompactionMgr{} + if stage == "pull" { + mgr.pullErr = fmt.Errorf("image unavailable") + } else { + mgr.failCommand = "db" + } + + inspect := false + err := CompactDatadir(context.Background(), logrus.New(), mgr, &DatadirCompaction{ + Client: client.ClientGeth, DataDir: t.TempDir(), Image: "client:pinned", + Cfg: &config.DBCompactionConfig{Inspect: &inspect}, + }) + require.Error(t, err) + + if stage == "pull" { + require.ErrorContains(t, err, "image unavailable") + assert.Empty(t, mgr.specs) + } else { + require.ErrorContains(t, err, "exit status 1") + require.Len(t, mgr.specs, 1) + } + }) + } +} diff --git a/pkg/runner/db_compaction_ldb_test.go b/pkg/runner/db_compaction_ldb_test.go deleted file mode 100644 index 244c66da7..000000000 --- a/pkg/runner/db_compaction_ldb_test.go +++ /dev/null @@ -1,111 +0,0 @@ -package runner - -import ( - "bytes" - "context" - "fmt" - "os" - "regexp" - "testing" - - "github.com/ethpandaops/benchmarkoor/pkg/client" - "github.com/ethpandaops/benchmarkoor/pkg/config" - "github.com/ethpandaops/benchmarkoor/pkg/docker" - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// ldbTestSetup creates two databases with besu's single-byte family names -// (0x01, 0x09 and 0x0a among them) next to nethermind-style ones. -const ldbTestSetup = `set -e -nl=' -' -for db in /data/a/rdb /data/b; do - mkdir -p $db - ldb --db=$db --create_if_missing put k0 v0 >/dev/null - for cf in "$(printf '\001')" "$(printf '\011')" Blocks "with space" "$nl"; do - ldb --db=$db create_column_family "$cf" >/dev/null - done - # Every ldb run flushes on close, so each put leaves its own level-0 file. - for i in 1 2 3; do - for cf in default "$(printf '\001')" Blocks "$nl"; do - ldb --db=$db --try_load_options --column_family="$cf" put k$i "v$i$cf" >/dev/null - done - done -done -` - -const ldbTestDump = `set -e -nl=' -' -for db in /data/a/rdb /data/b; do - for cf in default "$(printf '\001')" "$(printf '\011')" Blocks "with space" "$nl"; do - echo "$db $(printf %s "$cf" | od -An -tx1 | tr -d ' \n')" - ldb --db=$db --try_load_options --column_family="$cf" get_property rocksdb.num-files-at-level0 - ldb --db=$db --try_load_options --column_family="$cf" --value_hex scan - done -done -` - -var levelZero = regexp.MustCompile(`(rocksdb\.num-files-at-level0: )\d+`) - -// TestCompactDatadir_RealLdb runs the nethermind/besu compaction against real -// RocksDB databases with the ldb image named by BENCHMARKOOR_TEST_LDB_IMAGE -// (built from Dockerfile.rocksdb-ldb), through the docker daemon. -func TestCompactDatadir_RealLdb(t *testing.T) { - image := os.Getenv("BENCHMARKOOR_TEST_LDB_IMAGE") - if image == "" { - t.Skip("BENCHMARKOOR_TEST_LDB_IMAGE is not set") - } - - ctx := context.Background() - log := logrus.New() - - mgr, err := docker.NewManager(log) - require.NoError(t, err) - require.NoError(t, mgr.Start(ctx)) - - t.Cleanup(func() { _ = mgr.Stop() }) - - dir := t.TempDir() - mnt := docker.Mount{Type: "bind", Source: dir, Target: "/data"} - - sh := func(name, script string) string { - var out bytes.Buffer - - require.NoError(t, mgr.RunInitContainer(ctx, &docker.ContainerSpec{ - Name: fmt.Sprintf("benchmarkoor-ldb-test-%s-%d", name, os.Getpid()), - Image: image, - Entrypoint: []string{"sh"}, - Command: []string{"-c", script}, - Mounts: []docker.Mount{mnt}, - }, &out, &out), out.String()) - - return out.String() - } - - t.Cleanup(func() { sh("chown", fmt.Sprintf("chown -R %d:%d /data", os.Getuid(), os.Getgid())) }) - - sh("setup", ldbTestSetup) - before := sh("before", ldbTestDump) - require.Contains(t, before, "/data/b 0a\nrocksdb.num-files-at-level0: 2\n", "the newline family has data in level 0") - - compact := func() { - require.NoError(t, CompactDatadir(ctx, log, mgr, &DatadirCompaction{ - Client: client.ClientNethermind, - DataDir: dir, - Cfg: &config.DBCompactionConfig{Enabled: true, Image: image}, - })) - } - - compact() - - after := sh("after", ldbTestDump) - assert.Equal(t, levelZero.ReplaceAllString(before, "${1}0"), after, - "every family keeps its data and has nothing left in level 0") - - // ldb records its own merge operator in OPTIONS; a second pass accepts it. - compact() - assert.Equal(t, after, sh("again", ldbTestDump)) -} diff --git a/pkg/runner/db_compaction_test.go b/pkg/runner/db_compaction_test.go index 5247ac4ac..ad95a2741 100644 --- a/pkg/runner/db_compaction_test.go +++ b/pkg/runner/db_compaction_test.go @@ -189,28 +189,10 @@ func TestErigonDBMaintenanceCommands(t *testing.T) { assert.True(t, client.SupportsDBCompaction(client.ClientErigon)) } -// TestRethDBMaintenanceCommands: reth compacts with a copy-and-swap shell -// around `reth db copy --compact`, so the script is the entrypoint. -func TestRethDBMaintenanceCommands(t *testing.T) { - spec, err := client.NewRegistry().Get(client.ClientReth) - require.NoError(t, err) - - cmds := spec.DBMaintenanceCommands("/data") - require.NotNil(t, cmds) - - assert.True(t, cmds.OwnEntrypoint) - require.Len(t, cmds.Compact, 5) - assert.Equal(t, "sh", cmds.Compact[0]) - assert.Contains(t, cmds.Compact[2], "reth db --datadir=\"$d\" copy --compact") - assert.Equal(t, "/data", cmds.Compact[4]) - assert.Equal(t, []string{"db", "--datadir=/data", "stats"}, cmds.Inspect) - assert.Empty(t, cmds.CompactImage, "runs in the client's own image") - assert.True(t, client.SupportsDBCompaction(client.ClientReth)) -} - func TestSupportsDBCompaction_UnsupportedClients(t *testing.T) { for _, other := range []client.ClientType{ - client.ClientNimbus, client.ClientEthrex, + client.ClientBesu, client.ClientNethermind, + client.ClientReth, client.ClientNimbus, client.ClientEthrex, } { assert.False(t, client.SupportsDBCompaction(other), string(other)) } @@ -377,22 +359,13 @@ type fakeDBMaintenanceMgr struct { // failCommand fails any container whose command starts with this word. failCommand string - ran []string - specs []*docker.ContainerSpec - pulled []string -} - -func (f *fakeDBMaintenanceMgr) PullImage(_ context.Context, image, _ string) error { - f.pulled = append(f.pulled, image) - - return nil + ran []string } func (f *fakeDBMaintenanceMgr) RunInitContainer( _ context.Context, spec *docker.ContainerSpec, stdout, _ io.Writer, ) error { f.ran = append(f.ran, strings.Join(spec.Command, " ")) - f.specs = append(f.specs, spec) _, _ = fmt.Fprintln(stdout, "fake container output") @@ -740,85 +713,3 @@ func TestDBCompactionMarkerRecordsTheOverriddenImage(t *testing.T) { entry := marker.Phases[config.DBCompactionBeforePreRuns] assert.Equal(t, "erigontech/erigon:v3.7.0", entry.Image) } - -// TestRunDBCompactionContainers_ToolImage pins how a client compacted by a -// separate tool runs: the compaction in the tool image with Compact[0] as the -// entrypoint and extra_args after the rest, everything else in the client -// image, and the tool image pulled first. -func TestRunDBCompactionContainers_ToolImage(t *testing.T) { - mgr := &fakeDBMaintenanceMgr{} - r, resultsDir := dbCompactionTestRunner(t, mgr) - - cmds := &client.DBMaintenanceCommands{ - Compact: []string{"sh", "-c", "script", "rocksdb-compact", "/data"}, - Inspect: []string{"storage", "rocksdb", "usage"}, - CompactImage: "ldb:11.8.1", - } - - req := dbCompactionTestRequest(resultsDir) - req.Instance.Entrypoint = []string{"/opt/besu/bin/besu"} - req.Cfg.ExtraArgs = []string{"--max_open_files=-1"} - - require.NoError(t, r.runDBCompactionContainers( - context.Background(), req, cmds, nil, resultsDir, r.log, - )) - - assert.Equal(t, []string{"ldb:11.8.1"}, mgr.pulled) - require.Len(t, mgr.specs, 3) - - compact := mgr.specs[1] - assert.Equal(t, "ldb:11.8.1", compact.Image) - assert.Equal(t, []string{"sh"}, compact.Entrypoint) - assert.Equal(t, []string{"-c", "script", "rocksdb-compact", "/data", "--max_open_files=-1"}, compact.Command) - - for _, inspect := range []*docker.ContainerSpec{mgr.specs[0], mgr.specs[2]} { - assert.Equal(t, req.ImageName, inspect.Image) - assert.Equal(t, req.Instance.Entrypoint, inspect.Entrypoint) - } - - // db_compaction.image overrides the tool image too. - req.Cfg.Image = "my-ldb:dev" - assert.Equal(t, "my-ldb:dev", req.compactImage(cmds)) - assert.Equal(t, req.ImageName, (&dbCompactionRequest{ImageName: req.ImageName}).compactImage(&client.DBMaintenanceCommands{})) -} - -// TestRunDBCompactionContainers_Verify checks that a failed verification fails -// the phase before the inspection after it, that it is opt-in, and that a -// volume datadir is not verified. -func TestRunDBCompactionContainers_Verify(t *testing.T) { - var checked []string - - cmds := &client.DBMaintenanceCommands{ - Compact: []string{"db", "compact"}, - Inspect: []string{"db", "inspect"}, - Verify: func(dir string) (string, error) { - checked = append(checked, dir) - - return "", fmt.Errorf("journal holds 1354 blocks") - }, - } - - mgr := &fakeDBMaintenanceMgr{} - r, resultsDir := dbCompactionTestRunner(t, mgr) - req := dbCompactionTestRequest(resultsDir) - - require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log), "unset: off") - assert.Empty(t, checked) - - mgr.ran = nil - on := true - req.Cfg.Verify = &on - err := r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log) - require.ErrorContains(t, err, "journal holds 1354 blocks") - assert.Equal(t, []string{resultsDir}, checked, "the host path of the datadir") - assert.Equal(t, []string{"db inspect", "db compact"}, mgr.ran) - - off := false - req.Cfg.Verify = &off - require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) - - req.Cfg.Verify = &on - req.Mount = docker.Mount{Type: "volume", Source: "vol", Target: "/data"} - require.NoError(t, r.runDBCompactionContainers(context.Background(), req, cmds, nil, resultsDir, r.log)) - assert.Len(t, checked, 1) -} From 5b1ada31d3133a0751ae44cf95caf44a28afd3bd Mon Sep 17 00:00:00 2001 From: Stefan Date: Mon, 12 Oct 2026 12:52:40 +0200 Subject: [PATCH 13/13] fix(cli): reject empty compaction datadir and non-positive timeout --- cmd/benchmarkoor/db.go | 11 +++++++- cmd/benchmarkoor/db_test.go | 40 ++++++++++++++++++++++++++++ pkg/runner/db_compaction.go | 4 +++ pkg/runner/db_compaction_cli_test.go | 11 ++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 cmd/benchmarkoor/db_test.go diff --git a/cmd/benchmarkoor/db.go b/cmd/benchmarkoor/db.go index 39b2675c7..a98d1efb4 100644 --- a/cmd/benchmarkoor/db.go +++ b/cmd/benchmarkoor/db.go @@ -61,10 +61,19 @@ func runDBCompact(cmd *cobra.Command, _ []string) error { return fmt.Errorf("client %q has no offline database compaction", ct) } - if _, err := time.ParseDuration(dbCompactFlags.timeout); err != nil { + if dbCompactFlags.datadir == "" { + return fmt.Errorf("--datadir must not be empty") + } + + timeout, err := time.ParseDuration(dbCompactFlags.timeout) + if err != nil { return fmt.Errorf("--timeout: %w", err) } + if timeout <= 0 { + return fmt.Errorf("--timeout must be positive") + } + inspect := !dbCompactFlags.noInspect cfg := &config.DBCompactionConfig{ Enabled: true, diff --git a/cmd/benchmarkoor/db_test.go b/cmd/benchmarkoor/db_test.go new file mode 100644 index 000000000..921e21280 --- /dev/null +++ b/cmd/benchmarkoor/db_test.go @@ -0,0 +1,40 @@ +package main + +import ( + "context" + "path/filepath" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func TestDBCompactRejectsInvalidInputs(t *testing.T) { + original := dbCompactFlags + t.Cleanup(func() { dbCompactFlags = original }) + + // Invalid input must be rejected without a reachable container engine. + t.Setenv("DOCKER_HOST", "unix://"+filepath.Join(t.TempDir(), "missing.sock")) + + for _, tc := range []struct { + name string + datadir string + timeout string + want string + }{ + {"empty datadir", "", "1m", "--datadir must not be empty"}, + {"zero timeout", t.TempDir(), "0", "--timeout must be positive"}, + {"negative timeout", t.TempDir(), "-1s", "--timeout must be positive"}, + {"malformed timeout", t.TempDir(), "invalid", "--timeout:"}, + } { + t.Run(tc.name, func(t *testing.T) { + dbCompactFlags.client = "geth" + dbCompactFlags.datadir = tc.datadir + dbCompactFlags.timeout = tc.timeout + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + require.ErrorContains(t, runDBCompact(cmd, nil), tc.want) + }) + } +} diff --git a/pkg/runner/db_compaction.go b/pkg/runner/db_compaction.go index 7fd119a34..9914f16a8 100644 --- a/pkg/runner/db_compaction.go +++ b/pkg/runner/db_compaction.go @@ -987,6 +987,10 @@ func CompactDatadir( return err } + if c.DataDir == "" { + return fmt.Errorf("datadir must not be empty") + } + dataDir, err := filepath.Abs(c.DataDir) if err != nil { return err diff --git a/pkg/runner/db_compaction_cli_test.go b/pkg/runner/db_compaction_cli_test.go index dfd766c97..aa68b7784 100644 --- a/pkg/runner/db_compaction_cli_test.go +++ b/pkg/runner/db_compaction_cli_test.go @@ -100,3 +100,14 @@ func TestCompactDatadir_Failures(t *testing.T) { }) } } + +func TestCompactDatadir_RejectsEmptyDatadir(t *testing.T) { + mgr := &cliCompactionMgr{} + err := CompactDatadir(context.Background(), logrus.New(), mgr, &DatadirCompaction{ + Client: client.ClientGeth, Image: "client:pinned", + Cfg: &config.DBCompactionConfig{}, + }) + require.ErrorContains(t, err, "datadir must not be empty") + assert.Empty(t, mgr.pulled) + assert.Empty(t, mgr.specs) +}