fix(join): concurrent first joins no longer 500 on the QR-scan burst
`Event::find_or_create` was check-then-insert against a UNIQUE slug, and its only callers are `/join` and `/admin/login` — both of which run before the row exists, at the single most concurrent moment the app ever sees: the QR code goes up and every phone in the room posts `/join` within the same second. All of them miss the SELECT, all of them INSERT, one wins, and the rest get a bare unique violation surfaced as a 500 on the very first screen of the event. There is no retry on that path and nothing in the UI explains it. In the documented timeline the host's T-5 admin login creates the row first, so the blast radius is small — but it is one `down -v` or one `EVENT_SLUG` edit away from being live on the night. `ON CONFLICT (slug) DO UPDATE SET slug = EXCLUDED.slug` — a deliberate no-op write, because `DO NOTHING` returns no row on conflict and would put the loser back at square one. It touches only `slug`, so `name`, `export_epoch` and the lock/release timestamps are never disturbed by a late arrival; a test pins that. The read fast-path stays, so every join after the first is still a plain SELECT and takes no row lock. Tests live in `src/` rather than `tests/` because the crate is a binary and the function is not importable from an integration test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -32,14 +32,32 @@ impl Event {
|
||||
.await
|
||||
}
|
||||
|
||||
/// Insert the event, or return the existing row if another request won the race.
|
||||
///
|
||||
/// `ON CONFLICT`, not a bare INSERT. `slug` is UNIQUE (migration 002), and the only callers are
|
||||
/// `/join` and `/admin/login` — both of which run before the row exists, at the one moment the
|
||||
/// app is most concurrent: the QR code goes up and every phone in the room posts `/join` within
|
||||
/// the same second. A check-then-insert loses that race by construction, and the losers got a
|
||||
/// bare unique violation surfaced as a 500 on the very first screen of the event.
|
||||
///
|
||||
/// `DO UPDATE SET slug = EXCLUDED.slug` is a deliberate no-op write: `DO NOTHING` returns no
|
||||
/// row on conflict, which would put the loser right back at square one. It touches only `slug`,
|
||||
/// so `name`, `export_epoch` and the lock/release timestamps are never disturbed by a late
|
||||
/// arrival.
|
||||
pub async fn create(pool: &PgPool, slug: &str, name: &str) -> Result<Self, sqlx::Error> {
|
||||
sqlx::query_as::<_, Self>("INSERT INTO event (slug, name) VALUES ($1, $2) RETURNING *")
|
||||
.bind(slug)
|
||||
.bind(name)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
sqlx::query_as::<_, Self>(
|
||||
"INSERT INTO event (slug, name) VALUES ($1, $2)
|
||||
ON CONFLICT (slug) DO UPDATE SET slug = EXCLUDED.slug
|
||||
RETURNING *",
|
||||
)
|
||||
.bind(slug)
|
||||
.bind(name)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Reads first so the common case (the row already exists, i.e. every join after the first)
|
||||
/// stays a plain SELECT and never takes a row lock.
|
||||
pub async fn find_or_create(
|
||||
pool: &PgPool,
|
||||
slug: &str,
|
||||
@@ -51,3 +69,71 @@ impl Event {
|
||||
Self::create(pool, slug, name).await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The QR code goes up and every phone posts `/join` in the same second, before the event row
|
||||
/// exists. `find_or_create` reads first, so all of them miss, and all of them insert.
|
||||
///
|
||||
/// With a bare `INSERT`, exactly one wins and the rest get a unique violation on `slug` —
|
||||
/// surfaced as a 500 on the first screen of the event, for everyone but the winner. There is no
|
||||
/// retry on that path and nothing in the UI explains it.
|
||||
#[sqlx::test]
|
||||
async fn concurrent_first_joins_all_get_the_same_event(pool: PgPool) {
|
||||
let racers: Vec<_> = (0..16)
|
||||
.map(|_| {
|
||||
let pool = pool.clone();
|
||||
tokio::spawn(
|
||||
async move { Event::find_or_create(&pool, "wedding", "Hochzeit").await },
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
|
||||
let mut ids = Vec::new();
|
||||
for r in racers {
|
||||
let event = r
|
||||
.await
|
||||
.expect("task panicked")
|
||||
.expect("a concurrent first join must not fail — this is the QR-scan burst");
|
||||
ids.push(event.id);
|
||||
}
|
||||
|
||||
assert_eq!(ids.len(), 16);
|
||||
assert!(
|
||||
ids.iter().all(|id| *id == ids[0]),
|
||||
"every racer must land on ONE event row, not create rivals"
|
||||
);
|
||||
|
||||
let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM event WHERE slug = 'wedding'")
|
||||
.fetch_one(&pool)
|
||||
.await
|
||||
.expect("count");
|
||||
assert_eq!(count, 1, "exactly one event row may exist for a slug");
|
||||
}
|
||||
|
||||
/// A late arrival must not clobber the row it collides with — the no-op `DO UPDATE` exists to
|
||||
/// return the loser a row, not to let it rewrite one mid-event.
|
||||
#[sqlx::test]
|
||||
async fn a_late_create_does_not_disturb_the_existing_row(pool: PgPool) {
|
||||
let first = Event::find_or_create(&pool, "wedding", "Hochzeit")
|
||||
.await
|
||||
.expect("first");
|
||||
|
||||
sqlx::query("UPDATE event SET name = $1, export_epoch = 7 WHERE id = $2")
|
||||
.bind("Anna und Ben")
|
||||
.bind(first.id)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("simulate a live event");
|
||||
|
||||
let late = Event::create(&pool, "wedding", "Hochzeit")
|
||||
.await
|
||||
.expect("a colliding insert must still return the row");
|
||||
|
||||
assert_eq!(late.id, first.id);
|
||||
assert_eq!(late.name, "Anna und Ben", "the name must survive");
|
||||
assert_eq!(late.export_epoch, 7, "and so must the export epoch");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user