Skip to content

Stale-fragment cleanup: batch size constant, drain rate, and test strength #73

Description

@andinux

Follow-up to 9d0abb3, which bounded the stale-fragment cleanup to 64 candidate groups per pass. Bounding it was necessary: the previous query took one transaction-level advisory lock per stale group in a single statement and held them to commit.

Measured on PostgreSQL 17 with defaults (max_locks_per_transaction 64, max_connections 100, so 6400 shared lock slots), with a table of stale incomplete groups:

Stale groups Previous query Current query
3,000 works, holds 3,000 advisory locks 64 locks
6,000 works, holds 6,000 advisory locks 64 locks
30,000 ERROR: out of shared memory ("You might need to increase max_locks_per_transaction") 64 locks

The lock table is shared cluster-wide, so while one backend grabs thousands of locks other sessions can fail to take any. Three things are worth improving on top of the fix.

1. The batch size is a literal inside the SQL string

LIMIT 64 sits in SQL_PAYLOAD_FRAGMENTS_CLEANUP_STALE (src/postgresql/sql_postgresql.c). It should be a named constant next to the retention and throttle constants in src/cloudsync.c, so the relationship between them is visible.

64 is also exactly PostgreSQL's default max_locks_per_transaction, which reads as "one transaction's budget" but is not: the table is shared across backends (max_locks_per_transaction × (max_connections + max_prepared_transactions)). With many backends applying fragments at the same time, 64 locks each plus their relation and catalog locks can still crowd it. Something like 16 leaves real headroom, at the cost of a slower drain.

2. Draining depends entirely on inbound fragment traffic

Cleanup runs only from the fragment staging path and is throttled to once per 60 s per connection. At 64 groups per pass, a 30,000-group backlog needs roughly 469 passes, so about 8 hours of continuous fragmented-value traffic, and it stops making progress the moment such values stop arriving. Each pass also runs a full GROUP BY over the whole fragments table, so draining N groups costs O(N²/64) of scan work instead of O(N).

Worth considering: run cleanup from other paths too, delete in one statement without per-group locks when no concurrent apply is possible, or prevent large backlogs from forming (a group is only cleaned when it has been incomplete for 24 hours, so a backlog means many abandoned fragmented values).

3. The wording overstates what the fix does

CHANGELOG.md and docs/internal/audit-regressions.md say "subsequent calls drain further batches", which reads as if a backlog clears by itself. It only advances while fragmented values keep arriving, at up to 64 groups per minute per connection. Worth saying that explicitly.

4. The new test does not catch the regression it targets

In test/postgresql/61_fragment_cleanup_backlog.sql, the locks_ok assertion (count(*) <= 65 in pg_locks) passes against the unfixed query as well: the failing statement is rolled back with the cleanup savepoint, and aborting that subtransaction releases the advisory locks it took, so the count is 1. Only progress_ok detects the bug. The hardcoded counts (29937, 29873) also assume exactly one cleanup pass per apply and would fail confusingly if a chunk ever carried two fragments.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions