Skip to content

Commit 4f5dab3

Browse files
committed
index fixups for the prod DB
1 parent 6bf7bb6 commit 4f5dab3

3 files changed

Lines changed: 91 additions & 0 deletions

File tree

‎ext/central-controller-docker/migrations/0006_oidc_config.up.sql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
ALTER TABLE sso_expiry RENAME COLUMN member_id TO device_id;
2+
-- NOTE (2026-08-03): sso_expiry_network_member_ix is dropped again in 0008. It looks
3+
-- correct but could never serve the ON DELETE CASCADE from network_memberships_ctl --
4+
-- sso_expiry's columns are bpchar against a varchar parent, so the RI trigger compares
5+
-- network_id::text and no bare-column index can match it. Do not re-add a bare-column
6+
-- index here expecting it to help; see 0008_sso_expiry_indexes.up.sql for the full story.
27
CREATE INDEX IF NOT EXISTS sso_expiry_network_member_ix ON public.sso_expiry (network_id, device_id);
38

49
CREATE TABLE IF NOT EXISTS oidc_config (
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
-- Reverse of 0008: drop the two new indexes and restore sso_expiry_network_member_ix
2+
-- exactly as 0006 created it.
3+
--
4+
-- WARNING: this reinstates the defect 0008 fixed. With sso_expiry_ri_ix gone, the
5+
-- ON DELETE CASCADE from network_memberships_ctl falls back to a full sequential scan of
6+
-- sso_expiry for every member deleted; with sso_expiry_lookup_ix gone the netconf-path
7+
-- nonce lookups go back to heap-fetching every accumulated row per member; and with
8+
-- sso_expiry_creation_time_ix gone the retention pruner full-scans the table every run.
9+
-- See the header of 0008_sso_expiry_indexes.up.sql for the measurements.
10+
11+
DROP INDEX IF EXISTS sso_expiry_creation_time_ix;
12+
DROP INDEX IF EXISTS sso_expiry_lookup_ix;
13+
DROP INDEX IF EXISTS sso_expiry_ri_ix;
14+
15+
CREATE INDEX IF NOT EXISTS sso_expiry_network_member_ix ON public.sso_expiry (network_id, device_id);
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
-- sso_expiry index overhaul (2026-08-03).
2+
--
3+
-- WHY: sso_expiry.network_id / device_id are CHARACTER(16) / CHARACTER(10) (bpchar), while
4+
-- the parent columns they reference -- network_memberships_ctl.network_id / device_id --
5+
-- are character varying(22). There is no varchar = bpchar operator, so when the FK in 0005
6+
-- was created PostgreSQL coerced both sides to text and stored texteq as the constraint's
7+
-- comparison operator. The ON DELETE CASCADE trigger consequently emits:
8+
--
9+
-- DELETE FROM ONLY public.sso_expiry x
10+
-- WHERE $1::pg_catalog.text OPERATOR(pg_catalog.=) x.network_id::pg_catalog.text
11+
-- AND $2::pg_catalog.text OPERATOR(pg_catalog.=) x.device_id::pg_catalog.text
12+
--
13+
-- bpchar->text is a real function call (it strips trailing blanks), not a binary-coercible
14+
-- relabel, so a btree index on the bare columns can NEVER match that qual. This is why
15+
-- sso_expiry_network_member_ix (added in 0006, on exactly the right two columns) was never
16+
-- once used by the cascade: every single member delete seq-scanned the entire table.
17+
--
18+
-- Measured in prod 2026-08-01: 64,974 buffers / 1.33 s per deleted member, at idle. A user
19+
-- bulk-deleting ~2,100 members from one network pinned the 4-vCPU AlloyDB instance at 100%
20+
-- CPU for 27 minutes and paged on controller_db_commit_latency_ms.
21+
--
22+
-- Note the asymmetry: the reverse RI direction (checking the parent exists on INSERT into
23+
-- sso_expiry) stays fast at ~17 buffers, because varchar->text IS binary-coercible and the
24+
-- planner strips it as a RelabelType. Same operator, same cast syntax, opposite outcomes.
25+
--
26+
-- WHAT:
27+
-- sso_expiry_ri_ix -- built on the cast expressions so the cascade can match it.
28+
-- Prod: 61,268 -> ~15 buffers, 1,450 ms -> 0.4 ms per delete.
29+
-- sso_expiry_lookup_ix -- adds nonce_expiration as a third key column so the netconf-path
30+
-- nonce lookups (CentralDB.cpp, "SELECT nonce FROM sso_expiry
31+
-- WHERE network_id = $1 AND device_id = $2 AND ... <= ...") can
32+
-- range-seek past expired rows instead of heap-fetching every
33+
-- accumulated row for that member. A member that never completes
34+
-- SSO accrues one row per 5 minutes and rows are retained ~30
35+
-- days (sso_expiry doubles as the SSO log), so those groups run
36+
-- to thousands of rows.
37+
-- Prod: 4,850 -> 5 buffers, 16 ms -> 0.03 ms per lookup, which
38+
-- returned ~1.2 of the 4 vCPUs.
39+
-- sso_expiry_creation_time_ix
40+
-- -- the retention pruner in central-v2/apps/controller-manager/
41+
-- core/controller/ssocleanup.go ("DELETE FROM sso_expiry WHERE
42+
-- creation_time < NOW() - INTERVAL '32 days'") had no index on
43+
-- creation_time and full-scanned the table on every run, roughly
44+
-- every 5 minutes, forever.
45+
-- Prod: 58,000 -> ~900 buffers, 880 ms -> 1.0 ms per run.
46+
-- sso_expiry_network_member_ix is dropped: it is a strict prefix of sso_expiry_lookup_ix,
47+
-- so it serves nothing the new index does not, while costing
48+
-- maintenance on every insert and every pruner delete.
49+
--
50+
-- LOCKING: prod already has all three changes, applied by hand on 2026-08-03 using
51+
-- CREATE INDEX CONCURRENTLY / DROP INDEX CONCURRENTLY, so nothing blocked. This file
52+
-- deliberately uses plain, non-concurrent DDL: golang-migrate sends the whole file as a
53+
-- single Exec, which PostgreSQL runs as an implicit transaction block, and
54+
-- CREATE INDEX CONCURRENTLY cannot run inside a transaction block. That is the right
55+
-- trade-off for the databases this file will actually run against -- dev, staging, and
56+
-- freshly provisioned controllers -- where sso_expiry is small or empty.
57+
--
58+
-- Do NOT run this file against a large populated database. CREATE INDEX takes a SHARE lock
59+
-- (blocking writes to sso_expiry for the duration of each build) and DROP INDEX takes
60+
-- ACCESS EXCLUSIVE. At prod scale, do it by hand with CONCURRENTLY instead.
61+
62+
CREATE INDEX IF NOT EXISTS sso_expiry_ri_ix
63+
ON public.sso_expiry ((network_id::text), (device_id::text));
64+
65+
CREATE INDEX IF NOT EXISTS sso_expiry_lookup_ix
66+
ON public.sso_expiry (network_id, device_id, nonce_expiration);
67+
68+
CREATE INDEX IF NOT EXISTS sso_expiry_creation_time_ix
69+
ON public.sso_expiry (creation_time);
70+
71+
DROP INDEX IF EXISTS sso_expiry_network_member_ix;

0 commit comments

Comments
 (0)