Skip to content

Hold the idempotency sweeper's advisory lock on one dedicated connection and delete expired rows in batches #1500

Closed

Description

@greatest0fallt1me

Summary

sweepIdempotencyStoreRows takes a session-level pg_try_advisory_lock through pool.query, runs the delete through another pool.query, and unlocks through a third. Each call can use a different pooled connection, so the unlock often runs on a session that does not hold the lock. The lock then leaks on whichever idle connection took it, and every other instance logs "Another instance owns the sweeper lock" forever. The sweep itself is a single unbounded DELETE followed by a full COUNT(*) on every tick. This issue fixes the locking and bounds the work.

Background and evidence

In src/services/idempotencySweeper.ts:

  • Lock, delete and unlock on different connections.
    • pool.query('SELECT pg_try_advisory_lock($1) AS acquired', [KEY]) (lines 26-29): a session lock on an arbitrary pooled client.
    • pool.query('DELETE FROM idempotency_store WHERE expires_at < NOW()::timestamp') (lines 33-35): an unbounded delete, possibly on another client.
    • pool.query('SELECT pg_advisory_unlock($1)', [KEY]) (line 63): likely a third client. It returns false with a warning and leaves the original session holding the lock.
  • Full scan every tick. SELECT COUNT(*)::bigint AS row_count FROM idempotency_store (lines 46-48) runs on every interval for the setIdempotencyStoreRows gauge.
  • Lock leaks. A leaked session lock lives until that pooled connection closes (idleTimeoutMillis in src/db.ts), or indefinitely on a busy pool.
  • Never wired. createIdempotencySweeperJob has no caller outside tests. Fixing this is a precondition for enabling it.

Scope

In scope

  1. Dedicated client. Use one pool.connect() client per sweep. Either:
    • wrap the sweep in a transaction with pg_try_advisory_xact_lock (released automatically on commit or rollback); or
    • keep a session lock, but acquire and release it on the same client inside try/finally, and always call client.release().
  2. Batched delete. Delete in batches (DELETE FROM idempotency_store WHERE ctid IN (SELECT ctid FROM idempotency_store WHERE expires_at < NOW() LIMIT $1)), looping until fewer than the batch size are removed or a per-run time budget is reached. Make both configurable.
  3. Cheap gauge. Replace COUNT(*) with reltuples from pg_class (estimate), or count only on every Nth run. Document the accuracy trade-off in the metric help text in src/metrics.ts.
  4. Wiring. Start the job from src/index.ts, behind a config flag defaulting to on outside test, and register it with createGracefulShutdownHandler subsystems.
  5. Docs. Update docs/billing-idempotency.md (sweeper section).

Out of scope

  • Lease semantics of started rows.
  • Changing retention defaults.

Design constraints

  • Same session. Lock acquisition and release must happen on the same database session, provably, via a test that inspects pg_locks.
  • Short transactions. No single statement may hold row locks for longer than one batch.
  • Shutdown. beginShutdown stops new batches; awaitIdle waits for the current batch only.

Test plan

Use testcontainers Postgres for the lock tests (pg-mem does not implement advisory locks). Unit-test batching logic with a fake client.

# Scenario Expected
1 Sweep with lock available expired rows deleted; lock released (pg_locks shows none afterwards)
2 Two sweeps concurrently (two pools) exactly one deletes; the other skips; no leaked lock afterwards
3 25k expired rows, batch 5k 5 batches; all removed; non-expired rows untouched
4 Time budget reached mid-run stops early; next run continues
5 Delete throws client released; lock released; error logged; next tick runs
6 Gauge updated from the estimate without a full count
7 Shutdown during a batch finishes the batch; no further batches

Regression proof. Row 2 shows a lock still held in pg_locks after the sweep on current main.

Acceptance criteria

  • grep -n "pool.query('SELECT pg_try_advisory_lock" src/services/idempotencySweeper.ts returns nothing.
  • Every test-plan row passes.
  • npm run lint, npm run typecheck, npm run build, npm test and npm run test:integration pass, and CI is green.
  • At least 90% line coverage on idempotencySweeper.ts.

Getting started

git checkout -b fix/idempotency-sweeper-lock
npm ci && npx prisma generate
npm run test:unit -- src/services/idempotencySweeper

Read first: src/services/idempotencySweeper.ts, src/lifecycle/shutdown.ts, src/index.ts, and setIdempotencyStoreRows in src/metrics.ts.

PR requirements

  • Link this issue (Closes #<n>).
  • Paste pg_locks evidence before and after.
  • Paste test output and coverage.
  • Example commit: fix(idempotency): sweep with a same-session lock and bounded batches.

Priority rationale

Medium. It is a contained module, but the bug silently disables cleanup across a fleet and the unbounded delete can lock a hot table. Fixing it requires a correct understanding of Postgres advisory-lock scoping.

Activity

  1. Danielobito009 commented on Oct 18, 2026

    @Danielobito009
    Contributor

    @Danielobito009 has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    I would love to work on this issue

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @Danielobito009 to this issue.

  2. drips-wave commented on Oct 19, 2026

    @drips-wave

    Congratulations, @Danielobito009! 🎉 Your application was accepted by the repo's maintainers, and the issue is due on October 22, 2026.

    🧑‍💻 @Danielobito009: Please resolve the issue such that the repo's maintainers have enough time to review your contribution before the due date. You'll earn Points for completing the issue on-time, which will make you eligible for a share of the Stellar Wave Program's reward pool.

    Warning

    When opening a PR, please link it to this issue to ensure it gets tracked accurately. Points are awarded when this issue is marked as completed by the maintainer.

    🤠 Repo maintainers: Please keep an eye on the contributor's progress and review their work before the due date. You can manage this issue, including adjusting its complexity and points, here.

    🌊 Happy Wave 🌊

  3. grantfox-oss commented on Oct 24, 2026

    @grantfox-oss

    🎉 This issue has been marked as completed on GrantFox!

    @Danielobito009's PR #1535 was approved and merged by @greatest0fallt1me.

    🏆 @Danielobito009: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (83 total points). Track your full progress on GrantFox.

    👏 Great work, @Danielobito009! Keep contributing to CalloraOrg.

  4. drips-wave commented on Oct 24, 2026

    @drips-wave

    This issue has been completed by @Danielobito009 as part of the Stellar Wave Program's 10th Wave 🥳

    😎 @Danielobito009: You earned 100 Points for completing this issue! Once the Wave's rewards are finalized, you'll be eligible for a percentage of the Wave's reward pool based on the percentage of total points you've earned. Learn more here. You can also Leave a review to share your experience working on this issue.

    🧑‍💻 Repo maintainers: How'd the contributor do? Leave a review to share your experience working with them.

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

Metadata

Metadata

Labels

Stellar WaveStellar Wave Program issuebackendBackend service workdatabaseDatabase schema/migrationsperformancePerformance/efficiencypriority:mediumMedium implementation difficulty

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions