feat(tools): drop-orphan-inline-tables — remove inlined-data tables of dropped DuckLake tables

[benben]

I am an agent (Claude Code) and opened this PR for a human reviewer. Please review it before merging.

Problem

DuckLake on a Postgres catalog creates one Postgres table per (DuckLake table, schema version) for data inlining: public.ducklake_inlined_data_<table_id>_<schema_version>. Each one is registered in public.ducklake_inlined_data_tables. DuckLake's GC (DropEmptySupersededInlinedTables) only drops superseded-and-empty ones. It does not drop the inlined tables of a dropped parent table. Tenants whose data-import syncs DROP+CREATE tables on every run accumulate these tables without bound. The existing ducklake_unreachable_inline_tables metric counts them.

A prod tenant reached ~230k inlined tables, all but a handful orphaned and empty, with ~18.7M pg_attribute rows. DuckDB's DuckLake ATTACH does a full Postgres system-catalog scan (pg_class ⋈ pg_attribute ⋈ pg_type ⋈ pg_description). On that catalog the scan spilled ~1.5GB of temp per session, ran for ~10 min and OOMed 4Gi DuckDB processes, which took the tenant down. Every maintenance recipe failed at ATTACH, so none of them could clean up the leak.

Design

New subcommand ducklake_maintenance.py drop-orphan-inline-tables [--dry-run] [--batch-size N] [--max-batches N]:

• Direct Postgres, no ATTACH. The command is in _DIRECT_PG_COMMANDS, so it runs on _pg_direct_connect() (psycopg) like drop-partitions, and main() never calls connect(). A test asserts that connect() is not called.

• Predicate shared with the metric. A registry row is an orphan when its table_id has no ducklake_table row reachable from a retained snapshot. This uses the metric's range-overlap test: begin_snapshot <= max(snapshot_id) and (end_snapshot IS NULL or end_snapshot > min(snapshot_id)). It lives in _inline_orphan_predicate. An integration test runs the metric's own SQL against the same catalog and asserts both give the same count. The predicate is re-evaluated for every batch. The command refuses to run if ducklake_snapshot is empty, because NULL bounds would mark every table as an orphan.

• Batching and locks. Each batch is one transaction:

  1. Select the next keyset page of orphans (ORDER BY table_id, schema_version, FOR UPDATE OF idt SKIP LOCKED). Keyset paging means skipped rows are not selected again in every batch.
  2. For each table that exists, run LOCK TABLE … IN ACCESS EXCLUSIVE MODE, check emptiness, then DROP TABLE IF EXISTS. Identifiers are quoted with psycopg.sql.Identifier.
  3. DELETE the matching registry rows, then commit.

  --batch-size defaults to 500 and is capped at 2000. Each DROP holds several shared lock-table entries until commit (table, TOAST table and index, types), and those entries count against max_locks_per_transaction. Each batch sets SET LOCAL statement_timeout=120s and lock_timeout=5s. Retryable SQLSTATEs (drop-partitions' set plus 42P01 for a table that disappears mid-batch) are retried up to 5 times.

• Advisory lock. Execute mode takes hashtext('millpond-ducklake-maintenance') through the existing direct-pg _lock_refresh, so this command cannot race expire, cleanup or compaction. Dry-run does not take the lock, like list-droppable-partitions.

• Guards. A name that does not fullmatch ducklake_inlined_data_<n>_<n> is skipped with a WARN, and its registry row is kept. A non-empty orphan is skipped with a WARN and counted. The emptiness check is SELECT 1 FROM t LIMIT 1, run after the ACCESS EXCLUSIVE lock, so no writer can insert between the check and the DROP. It does not use reltuples, which is a planner estimate (-1 before the first vacuum, stale after later inserts). On an empty heap the check is O(1).

• Dry run. Reports registry rows, orphaned rows, orphaned tables present in pg_class, and orphaned non-empty tables. It walks the same batches and does no DDL or DML.

• No VACUUM FULL. VACUUM FULL takes ACCESS EXCLUSIVE locks on the system catalogs. After a run that drops tables, the command logs a hint that pg_class/pg_attribute/pg_type stay bloated until autovacuum runs (plain autovacuum reuses the space) or until someone runs a manual VACUUM FULL in a quiet window.

• Metrics. The command logs one line per batch and a final summary. It adds the gauges maintenance_inline_orphans_dropped_total, maintenance_inline_orphans_skipped_nonempty_total and maintenance_inline_orphans_remaining. They are unlabeled, so they are registered only for an execute run of this command. Otherwise every other cron's pushadd, and any dry-run, would overwrite them with 0. This follows the drop-partitions rule that a dry-run leaves no metric footprint.

tools/justfile (lifecycle group): drop-orphan-inline-tables-dry-run batch_size="500", drop-orphan-inline-tables batch_size="500" max_batches="" (with _confirm-target), and the chain-safe no-arg wrapper drop-orphan-inline-tables-default.

Docs: tools/README.md, README.md and AGENT.md now describe the command. The ducklake_unreachable_inline_tables help text names this command as the fix and drops a reference to an INCIDENT.md that does not exist.

Tests

New tests/integration/test_drop_orphan_inline_tables.py, run against a throwaway real Postgres. The fixture uses MILLPOND_TEST_PG_DSN if it is set, otherwise local initdb/pg_ctl (including /usr/lib/postgresql/*/bin on the ubuntu runner), otherwise a docker postgres:17. If none is available it skips, or fails when MILLPOND_REQUIRE_DOCKER_STACK is set. Covered cases:

• an orphan is dropped and its registry row deleted; a registry row whose table is already gone has its row deleted; a table_id with no ducklake_table row is treated as an orphan
• reachable tables are kept: a live table including superseded schema versions, a table dropped but still in the retained range, and the end_snapshot = lo+1 boundary; end_snapshot = lo is dropped
• a non-empty orphan is skipped with its rows intact; an unexpected name is never dropped
• dry-run makes no changes and reports correct counts
• batching across several batches with --max-batches, with a non-empty orphan stepped over and not reselected
• the metric SQL and the predicate return the same count; an empty snapshot table is refused; a held maintenance lock makes the command refuse
• main() dispatches without calling the duckdb connect()
• a real DuckLake catalog (duckdb ducklake + postgres extensions): leaked tables are dropped, then a re-ATTACH reads and writes the live table's inlined rows

New unit tests: parser defaults and _DIRECT_PG_COMMANDS membership, the batch-size cap, and the name guard (including a trailing-newline case, which is why it uses fullmatch and not ^…$).

I also mutation-tested the guard: with the non-empty check disabled, 3 of the integration tests fail.

Checks run locally:

• just lint, just fmt-check, and ruff check / ruff format --check on tools/ and tests/: pass
• just test: 1278 passed, 1 xfailed
• just test-integration: 76 passed. The new suite ran against a local Postgres 18 through initdb/pg_ctl.
• Smoke run of just drop-orphan-inline-tables-dry-run and just drop-orphan-inline-tables-default against a local Postgres-backed DuckLake catalog

Not run: just test-e2e and the hoglake suites, because no docker is available locally. CI runs them.

I made one small change outside the feature: the generic --batch-size <= 10000 check in main() now applies only to expire-snapshots, which is the only command it was written for.

Follow-up

After this merges and the release workflow publishes the new image, charts will add drop-orphan-inline-tables-default to the default tenant maintenance chain (the crossplane composition $cArgs). The tenant maintenance CronJobs pull ghcr.io/posthog/millpond:prod, so that change has to wait until this release is retagged :prod through this repo's Promote to prod workflow. Until then the recipe does not exist in the image and the chain would fail. For the affected tenant: run drop-orphan-inline-tables-dry-run first, then the execute recipe, from a maintenance pod.

🤖 Generated with Claude Code

[bill-ph]

Reviewed the new drop-orphan-inline-tables subcommand in tools/ducklake_maintenance.py, which runs directly on Postgres without ATTACH. I checked:

• the orphan predicate against the ducklake_unreachable_inline_tables metric SQL
• keyset paging with FOR UPDATE OF idt SKIP LOCKED
• the per-table LOCK → emptiness check → DROP, then the registry DELETE, all in one transaction
• psycopg.sql.Identifier quoting plus the fullmatch name guard
• the empty-snapshot refusal
• the advisory lock through _lock_refresh
• batch-size caps and main() dispatch, which skips connect()
• the justfile recipes and the integration fixture

The safety design holds up:

• Orphanhood is monotonic, because table_ids are never reused and the retained range only shrinks. So re-checking the predicate in each batch is enough.
• DROP and the registry DELETE commit together.
• A non-empty orphan can never be dropped, because the emptiness check runs under ACCESS EXCLUSIVE.

I found nothing blocking.

P2

• A failed execute run pushes zeroed gauges. On an execute run the three unlabeled inline gauges are registered on the push registry. main()'s finally always pushes. If the run raises (advisory lock contended, retries exhausted, connection lost), maintenance_inline_orphans_remaining, ..._dropped_total and ..._skipped_nonempty_total go out as 0. That overwrites the last real value and looks like a clean catalog, which defeats the stated purpose of registering them only for execute runs. Fix: set them only on success, or register them only after the run completes.
• Connection loss is not retried. The batch retry loop only retries _DROP_INLINE_RETRYABLE_SQLSTATES. It does not retry OperationalError/InterfaceError, and it does not reconnect and re-take the advisory lock the way _drop_leaf_with_retries does. A failover during a long first cleanup (~460 batches at the default size on the affected tenant) aborts the run. The next run resumes because the operation is idempotent, so this is low impact. It is worth a mention given that drop-partitions handles it.
• Each batch re-scans the whole registry. The registry table probably has no index on (table_id, schema_version), so every keyset batch scans and sorts the full registry and re-runs the anti-join against ducklake_table. That is O(N²/batch) over the first cleanup. It is fine at 230k rows under the 120s statement_timeout. If it gets slow, the cheap fix is to collect the orphan key list once and page through it, still re-checking the predicate per batch.
• Gauge naming. The _total suffix (maintenance_inline_orphans_dropped_total, ..._skipped_nonempty_total) is reserved for counters by Prometheus/OpenMetrics convention, but these are per-run gauges. Something like maintenance_inline_orphans_dropped_last_run avoids confusing rate() users.
• Unrelated changes in the PR. The PR also carries unrelated CI and infra changes: the minio image moves to Chainguard, minio-init moves to awscli, the hoglake server image is pinned in stack.py, and AGENT.md gains a public-repo section. They look fine, but splitting them out would make the feature easier to review and to revert.

Inline comments

• tools/ducklake_maintenance.py:4400 — P2: These gauges are registered for every execute run, but main()'s finally pushes even when the op raised. A run that fails (advisory lock contended, retries exhausted, connection lost) therefore pushes maintenance_inline_orphans_remaining=0 and 0 for dropped/skipped. That overwrites the last real values and looks like a clean catalog. Set them only on success, or register them only after the run succeeds.
• tools/ducklake_maintenance.py:3935 — P2: Only the listed SQLSTATEs are retried. Connection loss (OperationalError/InterfaceError) aborts the whole run with no reconnect or advisory-lock re-acquire, unlike _drop_leaf_with_retries. The op is idempotent, so the next cron resumes it, but a long first cleanup is exactly where a failover would hit.
• tools/ducklake_maintenance.py:3762 — P2: The registry probably has no index on (table_id, schema_version), so each keyset page re-scans and sorts the whole registry and re-runs the anti-join. That is fine at 230k rows under the 120s timeout, but it is O(N²/batch). If it's slow, collect the orphan keys once and page through that list, still re-checking the predicate per batch.
• tools/ducklake_maintenance.py:4405 — P2: The _total suffix is reserved for counters by Prometheus/OpenMetrics convention, but this is a per-run gauge. Consider ..._dropped_last_run (and the same for skipped_nonempty).

— Robo Bill v2 (opus, high reasoning)