Skip to content

feat: pgcolumnar_autovacuum — the maintenance daemon autovacuum cannot reach (#415) - #624

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
ChronicallyJD:feat/415-autovacuum
Aug 14, 2026
Merged

feat: pgcolumnar_autovacuum — the maintenance daemon autovacuum cannot reach (#415)#624
jdatcmd merged 2 commits into
commandprompt:mainfrom
ChronicallyJD:feat/415-autovacuum

Conversation

@ChronicallyJD

Copy link
Copy Markdown
Collaborator

What

pgcolumnar_autovacuum — a maintenance daemon for the online upkeep core autovacuum cannot reach. pgColumnar's compact_rewrite and recluster live in extension functions, not table-AM callbacks, so core autovacuum never runs them; a table's dead rows and clustering decay accumulate until someone runs the verbs by hand. This is the daemon #415 asked for, built on the merged maintenance_due() policy function and the self-gating recluster (Part A, #614, merged).

Shape

Mirrors core autovacuum: a launcher registered from _PG_init (needs shared_preload_libraries, which pgColumnar already requires) wakes every naptime and starts one worker per database; each worker asks maintenance_due() which columnar tables want attention and runs the recommended verb over SPI, each op in its own transaction inside PG_TRY.

Two invariants make it safe unattended

  1. Lazy verbs only. It calls only the ShareUpdateExclusiveLock verbs (compact_rewrite and the now-self-gating recluster), never vacuum/vacuum_sorted/cluster, so by construction it cannot block a reader or a writer — SUEL does not conflict with SELECT/INSERT/UPDATE/DELETE.
  2. Autovacuum's yield. The worker sets PROC_IS_AUTOVACUUM, so core's lock manager cancels its maintenance op the instant a backend queues for a conflicting stronger lock (an ALTER/DROP/TRUNCATE taking AccessExclusiveLock). The cancel aborts only that op; the worker continues. A bounded hiccup, never an indefinite block.

Off by default (pgcolumnar.autovacuum). GUCs: autovacuum, autovacuum_naptime (60s), autovacuum_compact_threshold (0.2), autovacuum_recluster_threshold (0.05).

Proof

test/autovacuum.sh — OFF leaves a deleted-heavy table alone (the control that keeps "it compacted" from being vacuously true), enabling it (SIGHUP) compact_rewrites it (dead rows → 0, survivors intact) and reclusters a decayed table (appended groups → 0), disabling it stops maintenance.

test/autovacuum_yield.sh — the yield, proven deterministically. A dev GUC (pgcolumnar.maintenance_hold_ms) holds SUEL inside compact_rewrite interruptibly; the suite catches the daemon mid-hold (polling pg_locks), requests AccessExclusive, and asserts the lock is granted fast (the daemon yielded within ~deadlock_timeout) not after the full hold, and that the daemon logged the cancel. Removal proof: dropping PROC_IS_AUTOVACUUM makes the lock time out (57014) — the yield is load-bearing, not decoration.

Gate

Full bar, on the rebased-onto-main HEAD: autovacuum + autovacuum_yield preflighted on assert builds pg15/16/17, full matrix (run_all_versions.sh) on pg18a + pg19a — all suites green on both. autovacuum_yield is registered in the matrix.

Notes

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The daemon is faithful to core autovacuum, and the yield is genuinely load-bearing — verified on my lane.

Reviewed the whole daemon (launcher + worker + verb execution) and both suites against 7b08c4b-rebased HEAD, PG17 non-assert (your gate ran the assert builds).

What holds up, checked closely:

  • The two invariants are real, not asserted. Lazy verbs only: the worker calls exactly compact_rewrite and recluster (both SUEL), never the AccessExclusive verbs — grep-confirmed. The compact_rewrite(rel, %g) 2-arg call is valid (max_groups defaults to 0). The yield: av_mark_as_autovacuum sets MyProc->statusFlags |= PROC_IS_AUTOVACUUM and mirrors it into ProcGlobal->statusFlags[pgxactoff] under ProcArrayLock — that is verbatim the core AutoVacWorkerMain pattern, so a lock waiter's deadlock-timeout check finds and cancels the worker.
  • Injection-safe. The table list is built with quote_ident, and every verb SQL wraps it in quote_literal_cstr, so a hostile table name is a string literal cast to regclass, never code.
  • Transaction discipline is right. Each op is its own StartTransactionCommand/PushActiveSnapshot/PG_TRY; the PG_CATCH copies the error message out to TopMemoryContext before FlushErrorState, logs where it failed, and continues — so a cancel or a mid-flight DROP aborts only that op.

The yield removal proof, re-run by me: neutering PROC_IS_AUTOVACUUM makes the AccessExclusive request time out at 57014 instead of being granted, the "granted fast" arm reds (slow(15s)), and the cancel-log arm reds — all three, on that one mutation, while the premises (launcher running, SUEL held mid-hold) stay green. That is exactly the shape a safety claim needs: the arm that would catch the daemon blocking production reds when the yield is removed. autovacuum 11/11, autovacuum_yield 6/6, restored green after the proof.

Two non-blocking notes (robustness, not safety — a follow-up, don't gate this):

  1. sort_key round-trips through a comma-joined string (maintenance_due returns array_to_string(sorted_by, ','), the worker string_to_array(...,',')s it back). A clustering column whose name literally contains a comma splits wrong; recluster then errors on the bogus column and the PG_CATCH skips it — so the table silently never reclusters, but nothing breaks. Passing the name[] through without the string detour closes it.
  2. char q[512] for the verb SQL: snprintf truncates safely, but a very long schema.table + sort-key list would produce truncated SQL that errors and is skipped. psprintf removes the ceiling. Same benign failure mode as (1).

Both are the good failure direction (skip-and-log, never corrupt or block), which is why they're notes rather than a request-changes.

This closes out #415's daemon. Merge when ready.

@jdatcmd

jdatcmd commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Approved and ready to land, but it now conflicts with today's merges and needs a rebase onto main first. The conflicts are all mechanical append-collisions, no semantic overlap, in four files:

The branch is on your fork, so I cannot push the rebase from the jdatcmd session. Please rebase feat/415-autovacuum onto current main and force-push; it will then merge cleanly on the approval already recorded. (You flagged the CHANGELOG collision yourself in your #626 review, so this is expected.)

…ot reach (commandprompt#415)

pgColumnar's online maintenance (compact_rewrite, recluster) lives in extension
functions, not table-AM callbacks, so core autovacuum never runs it and a
table's dead rows and clustering decay accumulate until someone runs the verbs
by hand. This is the daemon commandprompt#415 asked for, on the merged measurement and
maintenance_due() policy function and the self-gating recluster (commandprompt#415 part A,
PR commandprompt#614).

Shape mirrors core autovacuum: a launcher registered from _PG_init (needs
shared_preload_libraries, which pgColumnar already requires) wakes every naptime
and starts one worker per database; each worker asks maintenance_due() which
columnar tables want attention and runs the recommended verb via SPI.

Two invariants make it safe unattended:
 1. It calls ONLY the ShareUpdateExclusiveLock verbs (compact_rewrite and the
    now-self-gating recluster), never vacuum/vacuum_sorted/cluster, so it cannot
    block readers or writers by construction.
 2. Autovacuum's yield: the worker sets PROC_IS_AUTOVACUUM, so core's lock
    manager cancels its maintenance op the instant a backend queues for a
    conflicting stronger lock; each op runs in its own transaction inside
    PG_TRY, so a cancel aborts only that op and the worker continues.

Off by default (pgcolumnar.autovacuum). GUCs: autovacuum, autovacuum_naptime,
autovacuum_compact_threshold (0.2), autovacuum_recluster_threshold (0.05).

test/autovacuum.sh: OFF leaves a deleted-heavy table alone (the control),
enabling it (SIGHUP) compact_rewrites it (dead rows -> 0, survivors intact) and
reclusters a decayed one (appended -> 0), disabling it stops maintenance.

test/autovacuum_yield.sh: the yield, proven deterministically. A dev GUC
(pgcolumnar.maintenance_hold_ms) holds SUEL inside compact_rewrite interruptibly;
the suite catches the daemon mid-hold, requests AccessExclusive, and asserts the
lock is granted FAST (the daemon yielded) not after the hold, and that the daemon
logged the cancel. Removal proof (driver): dropping PROC_IS_AUTOVACUUM makes the
lock time out (57014) -- the yield is load-bearing.

Design in design/ISSUE_415_AUTOVACUUM.md; depends on PR commandprompt#614 for the safe
recluster.
…commandprompt#415)

The daemon section in docs/administration.md landed with the feature. This adds
the four operator GUCs (pgcolumnar.autovacuum, autovacuum_naptime,
autovacuum_compact_threshold, autovacuum_recluster_threshold) to the maintenance
table in the configuration reference, documents the dev/test
pgcolumnar.maintenance_hold_ms under internal settings (it is visible in
pg_settings), and records the daemon under CHANGELOG Added.
ChronicallyJD pushed a commit to ChronicallyJD/pgcolumnar that referenced this pull request Aug 14, 2026
…g changes

Backfills the documentation for the work that merged today, audited
against the SQL surface on main.

sql-reference.md gains an "Object storage" section covering the three
URL schemes (s3://, http://, https://), the separate objstore module
loaded on first remote use, the credential model (ambient AWS_*
environment for the function API; server endpoint/region plus
user-mapping secrets for the FDW; credentials_required and the
ambient-is-a-privilege rule), the pgcolumnar.objstore_allowed_endpoints
allow-list with its unconditional link-local refusal, and export to
s3:// with its multipart nothing-visible-before-complete property. The
Import/export and external-Parquet intros, read_parquet, the FDW,
export_parquet, and export_arrow point at it. parquet_schema's
signature gains the field_id column (commandprompt#388) and describes it (NULL vs 0).

configuration.md gains the allow-list GUC under a new Object storage
subsection, since it is security-relevant and superuser-only.

CHANGELOG Unreleased: object-storage read/write, the credential model,
the allow-list, parquet_schema.field_id (Added); export atomicity and
the batch-fold EXPLAIN report (Fixed).

Docs only. No em/en-dash; every added sentence is within the 25-word
STE limit; docs_style.sh passes 9/9. The pgcolumnar_autovacuum daemon
(commandprompt#624) is documented with its own PR and is not covered here; recluster
and sort_status were documented by commandprompt#625.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ChronicallyJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main (b4d9295, post #626) and force-pushed — head is now 4a81265. The four collisions resolved exactly as you called them, all mechanical:

Re-gated on the rebased tree, not on the pre-rebase approval: preflight autovacuum + autovacuum_yield green on assert 15/16/17, full matrix green on 18a (165 ran) and 19a (167 ran), docs_style 9/9. GitHub CI is running now. The branch is mergeable and your approval carried across the force-push. Yours to land whenever CI is green.

— ChronicallyJD

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved on the rebased HEAD (c20ff3a + 4a81265). Rebase is clean, docs added, re-verified end to end.

The rebase resolved the four append-collisions correctly, confirmed by reading the merged regions:

  • run_all_versions.sh SUITES: both autovacuum/autovacuum_yield and today's objstore_*/export_sink are present and interleaved, none dropped.
  • columnar.h: the four daemon externs and the two objstore externs both survive.
  • The GUC block and CHANGELOG merged without loss.

And the new second commit means the daemon now ships its own docs/administration.md, config-reference GUCs, and CHANGELOG entries, which closes the "daemon docs come separately" note from the #626 review.

Re-ran the full bar on my PG17 non-assert lane against this HEAD:

  • Build clean, -Wshadow -Werror.
  • autovacuum 11/11, autovacuum_yield 6/6.
  • docs_style.sh 9/9 (the daemon config docs and the objstore_allowed_endpoints entry coexist).
  • Yield removal proof re-run: neutering PROC_IS_AUTOVACUUM reds all three yield arms (AccessExclusive times out at 57014, "granted fast" fails, cancel-log fails) while the premises stay green. The safety claim's load-bearing check still fails exactly when the yield is removed.

Everything from my first review stands. The two non-blocking robustness notes (comma-in-column-name string_to_array round-trip; q[512] truncation) remain optional follow-ups. Merging.

@jdatcmd
jdatcmd merged commit 122fd5c into commandprompt:main Aug 14, 2026
10 of 12 checks passed
jdatcmd pushed a commit that referenced this pull request Aug 14, 2026
Ran bench/run_bench.sh (6M, PG18.4 non-assert) on current main 122fd5c. The
storage, query, and mutation numbers are materially unchanged from the 2026-08-12
run: today's work is on the maintenance path and does not touch those code paths.
Refreshed the header stamp and the "What changed" section to record the re-run.

Added an "Online maintenance" section with the two measured changes today:

- recluster self-gate (#614): a redundant recluster on an already-clustered 10M
  row table goes from 14,750 ms (rewrites all 67 groups, layout digest changes) to
  0.96 ms (returns 0, layout byte-identical). Measured as a true before/after by
  building the pre-#614 commit bd983d9 and the current .so and running the same
  fixture on each.
- autovacuum daemon (#624): a foreground scan is 174.3 ms with the daemon off and
  173.9 ms with it on and actively compact_rewriting a separate deleted-heavy
  table. Non-blocking under lock conflict is proven separately by
  test/autovacuum_yield.sh.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgAk1gqeME7DHpJw8xxybu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants