Skip to content

perf(scheduler): index soft-deleted schedules for the cleaning daemon - #6924

Open
pfreixes wants to merge 2 commits into
masterfrom
pau/schedules-deleted-at-index
Open

perf(scheduler): index soft-deleted schedules for the cleaning daemon#6924
pfreixes wants to merge 2 commits into
masterfrom
pau/schedules-deleted-at-index

Conversation

@pfreixes

@pfreixes pfreixes commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Why

The cleaning daemon's schedules hard-delete burns ~427 ms of CPU every ~8s to delete at most one row. Measured on cloud prod:

Limit  (actual time=427.549..427.551 rows=1 loops=1)
  Buffers: shared hit=253987
  ->  Index Scan using schedules_pkey on schedules
        Filter: (deleted_at < (now() - '5 days'::interval))
        Rows Removed by Filter: 256444

The query filters on deleted_at but orders by id, so the planner walks schedules_pkey in id order filtering as it goes. It estimated rows=18 and gambled on hitting a match early; it stepped over 256,444 live schedules instead. That is ~1.94 GiB of buffer accesses to return a single row, entirely shared hit, so it is pure CPU on a fully cached table — roughly 6% of a core, permanently.

It also degrades over time: as older deleted schedules get cleaned, the front of the id ordering fills with live rows and the walk gets longer.

How

  • Indexed on id, not deleted_at. The query's ORDER BY is on id, so a deleted_at index would have to gather every eligible row and sort it to pick one. A partial index on id walks in id order over soft-deleted rows only and stops at the first eligible one.
  • The partial predicate bounds the work. schedules currently holds 2,578 soft-deleted rows (111 of them eligible) against 256k+ live rows, so the walk is bounded by 2,577 in the worst case however the ids interleave — a 100x reduction even pathologically.
  • No cost on the hot path. scheduleNextExecution updates schedules ~67/s. Neither id nor deleted_at changes on those updates, so HOT updates are preserved and this index is never touched by them. It is only written when a schedule is actually soft-deleted, which is rare.
  • Not a throughput problem. 111 eligible rows at ~1 per 8s clears in ~15 minutes, so the LIMIT 1 and its cascade-safety reasoning stay exactly as they are. This is wasted scanning, not a backlog.
  • Migrations: cloud prod runs CREATE INDEX CONCURRENTLY manually, then inserts the migration row so the auto-runner skips it. The scheduler migration runs inline at orchestrator boot (app.ts calls dbClient.migrate() before the server starts) and knex's migration lock serializes every rolling pod behind it, so a slow build stalls the deploy. Worse, if a startup probe kills the pod mid-build, Postgres leaves an invalid index and the next boot's IF NOT EXISTS matches on name, silently skips, and marks the migration applied — a green deploy with no speedup and nothing pointing at why. Self-hosted auto-runs it on deploy, which is fine on their small tables.

Commands

(Schema-qualified for the operator runbook — the migration file itself uses unqualified table names and relies on search_path.)

1. Pre-flight — confirm the recorded migration name format, and that nothing long-lived is open (CONCURRENTLY waits for every transaction older than each of its two phases):

SELECT name FROM nango_scheduler.migrations ORDER BY id DESC LIMIT 5;
-- expect names ending in .js (prod loads compiled migrations from dist/db/migrations)

SELECT pid, state, now() - xact_start AS xact_age, left(query, 80)
FROM pg_stat_activity
WHERE xact_start IS NOT NULL AND now() - xact_start > interval '5 seconds'
ORDER BY xact_age DESC;
-- expect: empty, or nothing long-lived

2. Build the index — CONCURRENTLY, no transaction wrapper (CONCURRENTLY can't run inside a txn):

CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_schedules_soft_deleted_id
ON nango_scheduler.schedules USING BTREE (id)
WHERE deleted_at IS NOT NULL;

3. Monitor progress (from a separate psql session while the build is running):

SELECT phase, blocks_done, blocks_total,
       round(100.0 * blocks_done / NULLIF(blocks_total, 0), 1) AS pct
FROM pg_stat_progress_create_index;

4. Verify successindisvalid and indisready must both be t:

SELECT indisvalid, indisready
FROM pg_index
WHERE indexrelid = 'nango_scheduler.idx_schedules_soft_deleted_id'::regclass;
-- expect: t, t

5. Mark migration applied — skip the auto-runner on next deploy:

INSERT INTO nango_scheduler.migrations (name, batch, migration_time)
VALUES (
    '20260728071506_schedules_soft_deleted_index.js',
    (SELECT COALESCE(MAX(batch), 0) + 1 FROM nango_scheduler.migrations),
    NOW()
);

6. Confirm the win — re-run the cleanup query's plan:

EXPLAIN (ANALYZE, BUFFERS)
SELECT "id" FROM "nango_scheduler"."schedules"
WHERE "deleted_at" < NOW() - INTERVAL '5 days'
ORDER BY "id" ASC
LIMIT 1;
-- expect: Index Scan using idx_schedules_soft_deleted_id,
--         Rows Removed by Filter down from 256444 to ~0,
--         Buffers down from 253987 to a few thousand

Emergency stop (if the build needs to be aborted mid-flight)

1. Find the PID (from a separate psql session):

SELECT pid, now() - query_start AS duration, state
FROM pg_stat_activity
WHERE query ILIKE '%CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_schedules_soft_deleted_id%'
  AND pid <> pg_backend_pid();

2. Cancel gracefully (preferred):

SELECT pg_cancel_backend(<pid>);

If cancel doesn't take effect within ~30s (some CREATE INDEX phases ignore SIGINT), escalate to terminate (drops the connection):

SELECT pg_terminate_backend(<pid>);

3. Clean up the invalid leftover — cancelled/terminated builds leave an invalid index in the catalog:

-- Check what's left:
SELECT indisvalid
FROM pg_index
WHERE indexrelid = 'nango_scheduler.idx_schedules_soft_deleted_id'::regclass;
-- if 'f': there's an invalid index that must be dropped before retrying

-- Drop it (CONCURRENTLY so the DROP itself doesn't lock the table):
DROP INDEX CONCURRENTLY IF EXISTS nango_scheduler.idx_schedules_soft_deleted_id;

After the DROP the table is back to a clean state and the build can be retried. Note this also matters if step 5 is skipped: an invalid index plus an unrecorded migration means the next deploy's IF NOT EXISTS matches the invalid index by name and no-ops.

What

  • Add partial index idx_schedules_soft_deleted_id on schedules (id) WHERE deleted_at IS NOT NULL, serving the hardDeleteOlderThanNDays cleanup query in packages/scheduler/lib/models/schedules.ts.

Test plan

  • npm run test:integration --dir=packages/scheduler — 85/85 pass, migration applies to the test database
  • Current plan measured on cloud prod via EXPLAIN (ANALYZE, BUFFERS)
  • Soft-deleted population measured (2,578 total / 111 eligible) to confirm the index bounds the walk
  • Cloud prod: pre-flight checks (migration name format, no long-lived transactions)
  • Cloud prod: CREATE INDEX CONCURRENTLY and confirm indisvalid = t, indisready = t
  • Cloud prod: INSERT INTO nango_scheduler.migrations to mark applied
  • Cloud prod: re-run the plan and confirm Rows Removed by Filter drops from 256,444 to ~0
  • Self-hosted auto-runs the migration on next deploy

Review in cubic

The schedules hard-delete filters on deleted_at but orders by id, so the planner
walked schedules_pkey filtering as it went: 256444 rows removed by filter and
253987 buffers to return a single row, 427ms per call, every ~8s.

Indexed on id rather than deleted_at because the query's ORDER BY is on id; a
deleted_at index would have to gather every eligible row and sort it. The
partial predicate keeps the index to the soft-deleted population, and since
neither id nor deleted_at changes on the hot next_execution_at updates, HOT
updates are preserved and the index is untouched by them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 1 file

Confidence score: 3/5

  • In packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts, creating the index concurrently with IF NOT EXISTS can leave an invalid same-name index after an interrupted build, so later boots may skip rebuilding and hardDeleteOlderThanNDays can run without the intended index; add validity checks/rebuild logic (or drop-invalid-then-create) so startup self-heals this state.
  • The down migration in packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts can take an ACCESS EXCLUSIVE lock on schedules, which may block scheduler reads/writes and turn rollback into an outage under long-lived transactions; switch to a rollback path that avoids long exclusive locking (for example, concurrent-safe drop strategy).
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts">

<violation number="1" location="packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts:11">
P1: An interrupted concurrent build can leave this migration permanently applied with an invalid index: the next scheduler boot sees the same-name relation, `IF NOT EXISTS` skips the build, and `hardDeleteOlderThanNDays` continues scanning live rows. A retry path that checks `pg_index.indisvalid`/`indisready` and drops an invalid same-name index concurrently before creating it would make interrupted deployments recover automatically.</violation>

<violation number="2" location="packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts:18">
P2: Rolling back this migration can block scheduler reads and writes while PostgreSQL acquires an `ACCESS EXCLUSIVE` lock on `schedules`, potentially turning a rollback into an outage under long-lived transactions. Using `DROP INDEX CONCURRENTLY IF EXISTS` is consistent with the concurrent index build and keeps rollback non-blocking.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


export async function up(knex: Knex): Promise<void> {
await knex.raw(
`CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_schedules_soft_deleted_id"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: An interrupted concurrent build can leave this migration permanently applied with an invalid index: the next scheduler boot sees the same-name relation, IF NOT EXISTS skips the build, and hardDeleteOlderThanNDays continues scanning live rows. A retry path that checks pg_index.indisvalid/indisready and drops an invalid same-name index concurrently before creating it would make interrupted deployments recover automatically.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts, line 11:

<comment>An interrupted concurrent build can leave this migration permanently applied with an invalid index: the next scheduler boot sees the same-name relation, `IF NOT EXISTS` skips the build, and `hardDeleteOlderThanNDays` continues scanning live rows. A retry path that checks `pg_index.indisvalid`/`indisready` and drops an invalid same-name index concurrently before creating it would make interrupted deployments recover automatically.</comment>

<file context>
@@ -0,0 +1,19 @@
+
+export async function up(knex: Knex): Promise<void> {
+    await knex.raw(
+        `CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_schedules_soft_deleted_id"
+        ON ${SCHEDULES_TABLE} USING BTREE (id)
+        WHERE deleted_at IS NOT NULL;`
</file context>

}

export async function down(knex: Knex): Promise<void> {
await knex.raw(`DROP INDEX IF EXISTS "idx_schedules_soft_deleted_id";`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Rolling back this migration can block scheduler reads and writes while PostgreSQL acquires an ACCESS EXCLUSIVE lock on schedules, potentially turning a rollback into an outage under long-lived transactions. Using DROP INDEX CONCURRENTLY IF EXISTS is consistent with the concurrent index build and keeps rollback non-blocking.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/scheduler/lib/db/migrations/20260728071506_schedules_soft_deleted_index.ts, line 18:

<comment>Rolling back this migration can block scheduler reads and writes while PostgreSQL acquires an `ACCESS EXCLUSIVE` lock on `schedules`, potentially turning a rollback into an outage under long-lived transactions. Using `DROP INDEX CONCURRENTLY IF EXISTS` is consistent with the concurrent index build and keeps rollback non-blocking.</comment>

<file context>
@@ -0,0 +1,19 @@
+}
+
+export async function down(knex: Knex): Promise<void> {
+    await knex.raw(`DROP INDEX IF EXISTS "idx_schedules_soft_deleted_id";`);
+}
</file context>

Every scheduler migration since 2025-05 leaves down() empty; only the 2024-era
ones carry rollback SQL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pfreixes

Copy link
Copy Markdown
Contributor Author

Thanks — both looked at. The reviewed commit was c79e27830; the branch is now at ad497bc37.

#2 (P2) — down() taking ACCESS EXCLUSIVE: resolved. down() is now empty, so there is no DROP to lock anything. That change landed for a convention reason rather than this one — every scheduler migration since 2025-05 (8 of them) leaves down() empty, and only the 2024-era ones carry rollback SQL — but either way the lock concern is gone.

#1 (P1) — invalid index + IF NOT EXISTS: valid, and handled in the runbook rather than in the migration.

The mechanism is right, with one correction: an interrupted build does not leave the migration applied. knex records a migration only on success, so a pod killed mid-build writes no row. It is the next boot that skips via IF NOT EXISTS, succeeds, and records it — landing on recorded-plus-invalid. Same end state, different path.

Not fixing it in this file, for three reasons:

  • Cloud prod never runs this migration. The index is built manually with CONCURRENTLY and the migration row is inserted so the auto-runner skips it. The Commands section verifies indisvalid/indisready before marking applied, and Emergency stop has the DROP INDEX CONCURRENTLY + retry path for exactly the invalid-leftover case.
  • Self-hosted builds this in well under a second on their table sizes, so the interruption window is effectively nil.
  • The gap is identical for all 17 CREATE INDEX ... IF NOT EXISTS statements already in packages/scheduler/lib/db/migrations/IF NOT EXISTS is 17/17 there. Adding self-healing to this one file would make it the odd one out. If we want it, it belongs in a shared migration helper covering all of them, not here.

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