Skip to content

Add per-index compaction controls - #475

Open
tjgreen42 wants to merge 5 commits into
background-compaction-1-enginefrom
background-compaction-2-api
Open

Add per-index compaction controls#475
tjgreen42 wants to merge 5 commits into
background-compaction-1-enginefrom
background-compaction-2-api

Conversation

@tjgreen42

@tjgreen42 tjgreen42 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Expose scheduler-independent SQL controls for inspecting and compacting one
BM25 index. This layer makes compaction usable by administrators and future
scheduler adapters without introducing a dependency on pg_durable or any
other job runner.

Why

The storage layer can compact automatically during a spill, but background
workers and operators need a stable way to inspect compaction debt and process
it in bounded transactions. Those entry points must preserve the same
capacity, locking, ownership, recovery, and relation-lifecycle guarantees as
inline compaction.

APIs

  • bm25_level_counts(regclass) returns the persisted segment count for each
    of the eight LSM levels.
  • bm25_needs_compaction(regclass) reports whether L0 through L6 contain
    threshold compaction debt.
  • bm25_compact(regclass) runs the full cascade under one per-index exclusive
    lock, after preflighting the complete cascade before its first merge.
  • bm25_compact_step(regclass) runs at most one merge batch and reports
    whether a batch ran, allowing callers to split a cascade across
    transactions.

Safety and behavior

  • Both mutating APIs require ownership of the physical BM25 index.
  • Partitioned parent indexes are rejected because they have no physical index
    storage; callers operate on each partition's index instead.
  • Permanent and unlogged indexes cannot be compacted in read-only
    transactions, and no index can be compacted during recovery.
  • A backend may compact its own temporary index in a read-only transaction.
  • Full compaction rejects a terminal L7 capacity conflict before changing the
    index. Stepped compaction preflights each individual batch.
  • Fresh-install and upgrade SQL expose the same functions and privileges.

Test coverage

Regression coverage exercises debt detection, full and stepped cascades,
ownership checks, partitioned and temporary relations, read-only and recovery
restrictions, terminal L7 behavior, and fresh/upgrade SQL parity. A source
guard preserves the ownership check before compaction side effects.

Stack position

This is the second independently reviewable layer of GitHub stack #479. It
builds on the capacity-safe engine in #474; later layers add transaction
dispatch and optional scheduler integration.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline ownership-check-ordering finding, I also checked the merge.c refactor: tp_compaction_next_action / tp_compaction_apply_count_transition reproduce the exact prior tp_compaction_candidate-based level selection and destination-capacity error behavior used by tp_maybe_compact_level and tp_compact_step — no semantic change there.

Extended reasoning...

The reported finding (RowExclusiveLock acquired in tp_open_bm25_index before the object_ownercheck, reachable by any PUBLIC caller via bm25_compact/bm25_compact_step which have no REVOKE EXECUTE) is confirmed by reading src/access/compaction.c lines 29-65 and the SQL install scripts (no REVOKE for the new functions). Given that finding, I additionally reviewed the src/segment/merge.c refactor since the PR conventions explicitly called out verifying that the new tp_compaction_next_action/tp_compaction_preflight/tp_compaction_apply_count_transition helpers preserve the prior tp_compaction_candidate semantics used by tp_maybe_compact_level and tp_compact_step. Comparing the old and new code paths, tp_compaction_candidate itself is unchanged, and the new wrapper simply hoists the same destination-capacity check (same ERRCODE_PROGRAM_LIMIT_EXCEEDED) earlier, before tp_merge_level_segments would have raised it anyway — so the automatic compaction paths behave identically. This is a concrete area I checked and ruled out beyond the inline finding, worth noting since it was flagged as a specific risk in the review conventions.

Comment thread src/access/compaction.c
@tjgreen42
tjgreen42 force-pushed the background-compaction-2-api branch 3 times, most recently from 46f165a to 18569f8 Compare August 30, 2026 06:08
@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.62500% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/access/compaction.c 86.36% 12 Missing ⚠️

📢 Thoughts on this report? Let us know!

@tjgreen42
tjgreen42 force-pushed the background-compaction-2-api branch from 18569f8 to 4514813 Compare August 31, 2026 01:41
@tjgreen42
tjgreen42 force-pushed the background-compaction-2-api branch from 4514813 to 39f47d5 Compare August 31, 2026 03:19
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.

1 participant