perf(blocklist): seek instead of scan, serve from a swapped copy, decay every 6h

deploy/hushield-recompute.timer

@@ -1,11 +1,23 @@
 [Unit]
-Description=Run HuShield score decay and trust recomputation every 15 minutes
+Description=Run HuShield score decay and trust recomputation every 6 hours
 Documentation=https://github.com/Hushield/hushield
 
 [Timer]
-# First run shortly after boot, then every 15 minutes.
+# First run shortly after boot, then every 6 hours.
+#
+# This was 15 minutes, which was fine when the table held a handful of rows.
+# After seeding 732k numbers a pass measured 58m52s, so the timer fired, found
+# the unit still active, skipped, and the pass effectively ran back-to-back
+# forever -- saturating MySQL at ~618 selects/sec and competing with the
+# blocklist reads devices sync from.
+#
+# 6 hours is not a compromise. This pass exists ONLY to apply time decay: new
+# community reports and admin overrides recompute their own number
+# synchronously inside the request transaction (api/reports.go,
+# api/admin_overrides.go), so nothing user-facing waits on it. With a 30-day
+# half-life a score moves 0.58% in 6 hours, well inside any threshold.
 OnBootSec=2min
-OnUnitActiveSec=15min
+OnUnitActiveSec=6h
 
 # Persistent means a run missed while the host was down fires once on the next
 # boot rather than being skipped. Scores decay with wall-clock time, so a gap in

internal/api/admin_overrides.go

@@ -122,7 +122,7 @@ func (h *adminOverridesHandler) writeOverride(ctx context.Context, e164, mode, r
 		return "", err
 	}
 
-	status, err := store.RecomputeNumber(ctx, tx, phoneNumberID, now)
+	status, err := store.RecomputeNumberServing(ctx, tx, phoneNumberID, now)
 	if err != nil {
 		return "", err
 	}

internal/api/blocklist.go

@@ -35,6 +35,13 @@ type blocklistResponse struct {
 	Entries []blocklistEntryResponse `json:"entries"`
 	Count   int                      `json:"count"`
 	Cursor  string                   `json:"cursor"`
+	// Total is how many rows the live serving slot can serve, so a client
+	// paging a full sync has a denominator for progress. It is the cached
+	// count published at the last slot rebuild, NOT a live COUNT(*) -- see
+	// store.ServableCount. It counts servable rows only, so a delta carrying
+	// "unblock" tombstones can apply more entries than Total; a client must
+	// clamp rather than assume the fraction stays under 1.
+	Total int64 `json:"total"`
 }
 
 const (
@@ -71,10 +78,20 @@ func (h *blocklistHandler) handleList(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// A failure here must not fail the sync: the entries are already loaded and
+	// a missing denominator only costs the client its percentage, so log it and
+	// serve Total=0, which the client treats as "no total available".
+	total, err := store.ServableCount(r.Context(), h.db)
+	if err != nil {
+		logInternalError(requestID, "read servable count", err)
+		total = 0
+	}
+
 	resp := blocklistResponse{
 		Entries: make([]blocklistEntryResponse, 0, len(entries)),
 		Count:   len(entries),
 		Cursor:  formatCursor(nextSec, nextID),
+		Total:   total,
 	}
 	for _, e := range entries {
 		resp.Entries = append(resp.Entries, blocklistEntryResponse{

internal/api/reports.go

@@ -148,7 +148,7 @@ func (h *reportsHandler) writeReport(ctx context.Context, deviceID uint64, e164
 		return "", err
 	}
 
-	status, err := store.RecomputeNumber(ctx, tx, phoneNumberID, now)
+	status, err := store.RecomputeNumberServing(ctx, tx, phoneNumberID, now)
 	if err != nil {
 		return "", err
 	}

internal/db/migrate_test.go

@@ -64,7 +64,7 @@ func TestMigrate_CreatesAllFiveTablesAndIsIdempotent(t *testing.T) {
 		}
 	}
 
-	const wantMigrations = 6 // 0001_init, 0002_drop_duplicate_number_index, 0003_device_sign_count, 0004_was_blockable, 0005_push_tokens, 0006_trust_weight_default
+	const wantMigrations = 10 // 0001_init, 0002_drop_duplicate_number_index, 0003_device_sign_count, 0004_was_blockable, 0005_push_tokens, 0006_trust_weight_default, 0007_blocklist_keyset_index, 0008_blocklist_serving_swap, 0009_serving_slot_pointer, 0010_serving_servable_count
 
 	var migrationRowCount int
 	if err := sqlDB.QueryRow("SELECT COUNT(*) FROM schema_migrations").Scan(&migrationRowCount); err != nil {

internal/db/migrations/0007_blocklist_keyset_index.down.sql

@@ -0,0 +1 @@
+DROP INDEX idx_phone_numbers_updated_at_id ON phone_numbers;

internal/db/migrations/0007_blocklist_keyset_index.up.sql

@@ -0,0 +1,26 @@
+-- Index the blocklist delta's keyset order so paging is a seek, not a scan.
+--
+-- BlocklistDelta pages by (updated_at, phone_number_id). Before this index --
+-- and before the matching predicate change in internal/store/blocklist.go --
+-- every page was a full table scan plus a filesort:
+--
+--   type: ALL   key: NULL   rows: 715574   Extra: Using where; Using filesort
+--
+-- measured at 0.771s per 500-row page against 732k rows. A full sync is ~1465
+-- pages, so roughly a billion row reads and ~19 minutes of pure DB time.
+--
+-- The key is (updated_at, phone_number_id) rather than extending the existing
+-- (status, updated_at) index with phone_number_id. The base query filters
+-- status IN ('blocked','overridden_block','suspected'), so a status-leading
+-- index produces three separate ranges whose union is NOT in global
+-- (updated_at, phone_number_id) order -- MySQL would still have to sort to
+-- satisfy ORDER BY, which is the cost being removed here. Leading with the
+-- ordering columns instead gives a range seek to the cursor followed by an
+-- in-order walk, with status applied as a residual filter.
+--
+-- That trade is right while nearly every row is blockable. If the table ever
+-- becomes mostly non-blockable -- seeded scores decay, so most rows can end up
+-- 'unknown' -- the residual filter would have to skip many rows to fill a
+-- page, and this choice should be revisited against a composite index.
+CREATE INDEX idx_phone_numbers_updated_at_id
+    ON phone_numbers (updated_at, phone_number_id);

internal/db/migrations/0008_blocklist_serving_swap.down.sql

@@ -0,0 +1,2 @@
+DROP TABLE blocklist_serving_next;
+DROP TABLE blocklist_serving;

internal/db/migrations/0008_blocklist_serving_swap.up.sql

@@ -0,0 +1,59 @@
+-- Split the blocklist into a populated copy and a served copy.
+--
+-- phone_numbers stays the single source of truth: every write -- community
+-- reports, device attestations, admin overrides -- continues to land there, so
+-- nothing a user submits can be lost by a swap. What gets swapped is only the
+-- DERIVED read model the blocklist delta serves.
+--
+-- Why: the decay pass rewrites derived state for every number. At 732k numbers
+-- a pass measured 58m52s, and for that whole hour readers were seeing a table
+-- mid-rewrite. Now the pass populates blocklist_serving_next and swaps it in
+-- with a single atomic RENAME TABLE, so a reader sees either the old snapshot
+-- or the new one, never a torn mixture.
+--
+-- updated_at deliberately has NO "ON UPDATE CURRENT_TIMESTAMP". It is copied
+-- verbatim from phone_numbers because it IS the sync cursor: clients page by
+-- (updated_at, phone_number_id) and persist that cursor. If a copy restamped
+-- it, every device's stored cursor would fall behind the whole table at once
+-- and every client would re-download the entire blocklist after every swap.
+CREATE TABLE blocklist_serving (
+    phone_number_id BIGINT(20) UNSIGNED NOT NULL,
+    number VARCHAR(20) NOT NULL,
+    cached_score DECIMAL(10,4) NOT NULL DEFAULT 0.0000,
+    status ENUM('unknown','suspected','blocked','allowlisted','overridden_block') NOT NULL DEFAULT 'unknown',
+    was_blockable TINYINT(1) NOT NULL DEFAULT 0,
+    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    PRIMARY KEY (phone_number_id),
+    -- The keyset order every delta query pages by. See migration 0007 for why
+    -- this leads with the ordering columns rather than with status.
+    KEY idx_blocklist_serving_updated_at_id (updated_at, phone_number_id),
+    -- The neighbour-spoof query's LIKE '+1NPANXX%' prefix match.
+    KEY idx_blocklist_serving_number (number),
+    -- The removal/tombstone query.
+    KEY idx_blocklist_serving_was_blockable (was_blockable, updated_at, phone_number_id)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+
+CREATE TABLE blocklist_serving_next (
+    phone_number_id BIGINT(20) UNSIGNED NOT NULL,
+    number VARCHAR(20) NOT NULL,
+    cached_score DECIMAL(10,4) NOT NULL DEFAULT 0.0000,
+    status ENUM('unknown','suspected','blocked','allowlisted','overridden_block') NOT NULL DEFAULT 'unknown',
+    was_blockable TINYINT(1) NOT NULL DEFAULT 0,
+    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    PRIMARY KEY (phone_number_id),
+    KEY idx_blocklist_serving_updated_at_id (updated_at, phone_number_id),
+    KEY idx_blocklist_serving_number (number),
+    KEY idx_blocklist_serving_was_blockable (was_blockable, updated_at, phone_number_id)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+
+-- Seed both copies so the blocklist serves correctly from the moment this
+-- migration lands, rather than returning an empty delta until the first swap.
+INSERT INTO blocklist_serving
+    (phone_number_id, number, cached_score, status, was_blockable, updated_at)
+SELECT phone_number_id, number, cached_score, status, was_blockable, updated_at
+FROM phone_numbers;
+
+INSERT INTO blocklist_serving_next
+    (phone_number_id, number, cached_score, status, was_blockable, updated_at)
+SELECT phone_number_id, number, cached_score, status, was_blockable, updated_at
+FROM phone_numbers;

internal/db/migrations/0009_serving_slot_pointer.down.sql

@@ -0,0 +1,15 @@
+DROP TABLE blocklist_serving_slot;
+
+ALTER TABLE blocklist_serving_a
+    RENAME INDEX idx_blocklist_serving_a_updated_at_id TO idx_blocklist_serving_updated_at_id,
+    RENAME INDEX idx_blocklist_serving_a_number        TO idx_blocklist_serving_number,
+    RENAME INDEX idx_blocklist_serving_a_was_blockable TO idx_blocklist_serving_was_blockable;
+
+ALTER TABLE blocklist_serving_b
+    RENAME INDEX idx_blocklist_serving_b_updated_at_id TO idx_blocklist_serving_updated_at_id,
+    RENAME INDEX idx_blocklist_serving_b_number        TO idx_blocklist_serving_number,
+    RENAME INDEX idx_blocklist_serving_b_was_blockable TO idx_blocklist_serving_was_blockable;
+
+RENAME TABLE
+    blocklist_serving_a TO blocklist_serving,
+    blocklist_serving_b TO blocklist_serving_next;

internal/db/migrations/0009_serving_slot_pointer.up.sql

@@ -0,0 +1,44 @@
+-- Switch the serving copy by pointer instead of by renaming tables.
+--
+-- 0008 published a rebuilt snapshot with an atomic RENAME TABLE. That is
+-- correct, but RENAME takes an EXCLUSIVE metadata lock: it waits for in-flight
+-- queries on those tables and every new reader queues behind it. Nothing is
+-- being physically moved here -- both copies just sit there -- so the switch
+-- should be a pointer change, not DDL.
+--
+-- Two fixed slots, and one singleton row saying which one is live. The decay
+-- pass populates whichever slot is NOT live and then updates the pointer, a
+-- single-row UPDATE that no reader can block and that takes no DDL lock.
+RENAME TABLE
+    blocklist_serving      TO blocklist_serving_a,
+    blocklist_serving_next TO blocklist_serving_b;
+
+-- Index names are per-table, so both copies carry the same names. Rename them
+-- per slot anyway: with the tables no longer swapping identities, an EXPLAIN
+-- naming idx_blocklist_serving_a_* tells you which slot you actually read.
+ALTER TABLE blocklist_serving_a
+    RENAME INDEX idx_blocklist_serving_updated_at_id TO idx_blocklist_serving_a_updated_at_id,
+    RENAME INDEX idx_blocklist_serving_number        TO idx_blocklist_serving_a_number,
+    RENAME INDEX idx_blocklist_serving_was_blockable TO idx_blocklist_serving_a_was_blockable;
+
+ALTER TABLE blocklist_serving_b
+    RENAME INDEX idx_blocklist_serving_updated_at_id TO idx_blocklist_serving_b_updated_at_id,
+    RENAME INDEX idx_blocklist_serving_number        TO idx_blocklist_serving_b_number,
+    RENAME INDEX idx_blocklist_serving_was_blockable TO idx_blocklist_serving_b_was_blockable;
+
+-- Singleton by construction: the CHECK pins the primary key to one value, so a
+-- second row cannot be inserted and "which slot is live" can never be
+-- ambiguous. active_slot is an ENUM so the value read back is always one of
+-- exactly two known table suffixes -- the store still validates it before
+-- interpolating, but the column makes an unexpected value impossible to store.
+CREATE TABLE blocklist_serving_slot (
+    slot_id TINYINT UNSIGNED NOT NULL DEFAULT 1,
+    active_slot ENUM('a','b') NOT NULL,
+    swapped_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    PRIMARY KEY (slot_id),
+    CONSTRAINT chk_blocklist_serving_slot_singleton CHECK (slot_id = 1)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+
+-- 'a' is the table 0008 was serving from, so this migration does not change
+-- which rows are live.
+INSERT INTO blocklist_serving_slot (slot_id, active_slot) VALUES (1, 'a');

internal/db/migrations/0010_serving_servable_count.down.sql

@@ -0,0 +1 @@
+ALTER TABLE blocklist_serving_slot DROP COLUMN servable_count;

internal/db/migrations/0010_serving_servable_count.up.sql

@@ -0,0 +1,35 @@
+-- Cache how many rows the live slot can actually serve, so a syncing client
+-- has a denominator for a progress bar.
+--
+-- The alternative was COUNT(*) per request. That is not viable: the delta's
+-- status filter is not the leading column of any index, so the count scans,
+-- and a full sync asks ~733 times. The rebuild in SwapBlocklistServing already
+-- walks every row, so counting there is free and the API reads one row.
+--
+-- Counts only SERVABLE statuses (block/label), matching blocklistBaseQuery.
+-- Two consequences, both deliberate:
+--   - Per-row patches from UpsertServingRow do not adjust this, so between
+--     switches it drifts by however many numbers were reported in that window.
+--     A handful out of 732k is irrelevant to a progress bar and not worth
+--     paying for on every report.
+--   - A delta also carries "unblock" tombstones, which are NOT in this count,
+--     so a removal-heavy sync can apply more entries than the total. The
+--     client clamps its displayed fraction rather than pretending otherwise.
+ALTER TABLE blocklist_serving_slot
+    ADD COLUMN servable_count BIGINT UNSIGNED NOT NULL DEFAULT 0 AFTER active_slot;
+
+-- Backfill from whichever slot is live now, so the bar is right before the
+-- first switch rather than reading zero.
+UPDATE blocklist_serving_slot
+SET servable_count = (
+    SELECT COUNT(*) FROM blocklist_serving_a
+    WHERE status IN ('blocked','overridden_block','suspected')
+)
+WHERE slot_id = 1 AND active_slot = 'a';
+
+UPDATE blocklist_serving_slot
+SET servable_count = (
+    SELECT COUNT(*) FROM blocklist_serving_b
+    WHERE status IN ('blocked','overridden_block','suspected')
+)
+WHERE slot_id = 1 AND active_slot = 'b';