Skip to content

Commit 1377138

Browse files
committed
fix: serialize is_keyword_blocked recomputes with a fenced lease
The full-table `is_keyword_blocked` recomputes (media and stream) run with no coordination from four trigger paths: the startup version check in both the API (main.rs) and the worker (worker.rs), the two keyword file-sync functions, and admin keyword edits. Even a minimal deployment runs the API and the worker as separate processes, so a keyword-version change triggers at least two concurrent full-table regex sweeps over the same rows; admin edits spawn additional unguarded sweeps on top, and multi-replica deployments multiply all of this by the replica count. Because completion is recorded only at the END of a sweep, any process restart mid-sweep repeats the whole sweep from scratch. On large stream tables the overlapping sweeps contend on the same tuples and can dominate database CPU with redundant regex evaluation. Route every trigger through a single entry point, kw_recompute_single_flight(pool, KwRecomputeKind), which converges the column to the CURRENT keyword state under a deployment-wide lease (the raw recompute fns are now private so no future caller can bypass it): * Each attempt reloads the keyword state from the DB and targets THAT version, so replicas running different builds converge to whatever the DB says now and can never downgrade each other's results. * The lease claim is a single-statement atomic upsert on the existing keyword_sync_state table (no schema change) that takes the lease only when absent or stale (180s). This works behind PgBouncer in transaction pooling mode — where session pg_advisory_lock would land on an arbitrary pooled backend and silently fail to exclude anyone — and holds no pool connection while the sweep runs, so a DB_POOL_SIZE=1 pool cannot be starved either. * The lease row stores a unique owner token (hostname + nanos). Renewal (every 60s) and release are fenced on it; on fence-out the holder aborts its sweep immediately. If renewals cannot be CONFIRMED for 120s the holder also aborts — before its 180s lease can go stale under it — so two sweeps never overlap. * Completion markers are published ONLY by the single-flight helper via an atomic owner-fenced upsert (the sweeps return success/failure and no longer write markers themselves), so a holder that silently lost its lease can never overwrite a successor's completion state. * Losing contenders retry (sleep 180s, bounded attempts) instead of skipping, so a process that arrives while an older version is being swept still gets the newer version applied without waiting for an unrelated restart. * After every claim the completion marker is rechecked, and after a successful sweep the holder loops once more and exits only when the marker agrees with the LIVE keyword state — an edit that reverts to a previously recorded version mid-sweep (ABA) is converged by the active holder rather than stranded until some later restart. Consolidating the callers also fixes two standalone correctness bugs: * The file-sync paths computed their completion version from the file lists alone, disagreeing with the DB-derived tag whenever admin-managed keyword rows exist — guaranteeing a redundant full re-sweep at every startup for any instance with admin-added keywords. * load_keyword_filter_cache coerces a transient SELECT failure into empty lists; a sweep run at that moment would clear every blocked flag and record a fabricated completion version. The sweep path now uses a new fallible loader (try_load_keyword_filter_cache) and retries instead of sweeping with silently-empty state. Known bounded residual (deliberate): the per-batch UPDATEs inside a sweep are not individually fenced on lease ownership. A holder descheduled past the 180s lease expiry exactly between batches can land at most one stale batch over a successor's result before its next renewal fences it out; the rows self-heal on the next keyword change or restart (the marker itself is fence-protected). Fencing every batch would couple the sweep SQL to the lease layer for marginal benefit. The batching and regex logic of the sweeps themselves are unchanged. cargo check --bins clean; clippy introduces no new warnings.
1 parent 6f3e446 commit 1377138

2 files changed

Lines changed: 447 additions & 106 deletions

File tree

backend/src/routes/admin_keyword_filters.rs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ use serde_json::json;
2525
use sha2::Sha256;
2626

2727
use crate::state::{
28-
AppState, load_keyword_filter_cache, recompute_keyword_blocked,
29-
recompute_stream_keyword_blocked,
28+
AppState, KwRecomputeKind, kw_recompute_single_flight, load_keyword_filter_cache,
3029
};
3130

3231
// ─── Auth helpers ─────────────────────────────────────────────────────────────
@@ -146,23 +145,18 @@ pub struct WhitelistRow {
146145
async fn reload_cache(state: &AppState) {
147146
let mut new_cache = load_keyword_filter_cache(&state.pool).await;
148147
new_cache.nsfw_filter_enabled = state.config.poster_nsfw_enabled;
149-
let media_ver = new_cache.media_version_tag();
150-
let stream_ver = new_cache.version_tag();
151-
let keywords = new_cache.keywords.clone();
152-
let stream_kws = new_cache.stream_keywords.clone();
153-
let whitelist = new_cache.whitelist.clone();
154-
let whitelist2 = whitelist.clone();
155148
if let Ok(mut w) = state.keyword_filters.write() {
156149
*w = new_cache;
157150
}
151+
// Converge the blocked flags via the deployment-wide single-flight lease —
152+
// an admin keyword edit must not launch unguarded full-table sweeps
153+
// alongside whatever other processes are doing.
158154
let pool = state.pool.clone();
159155
let pool2 = pool.clone();
160-
tokio::spawn(async move {
161-
recompute_keyword_blocked(&pool, media_ver, &keywords, &whitelist).await
162-
});
163-
tokio::spawn(async move {
164-
recompute_stream_keyword_blocked(&pool2, stream_ver, &stream_kws, &whitelist2).await
165-
});
156+
tokio::spawn(async move { kw_recompute_single_flight(&pool, KwRecomputeKind::Media).await });
157+
tokio::spawn(
158+
async move { kw_recompute_single_flight(&pool2, KwRecomputeKind::Stream).await },
159+
);
166160
}
167161

168162
// ─── Handlers ────────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)