From 3c600bbd1812a8a548fcd7b16d80b509043c4506 Mon Sep 17 00:00:00 2001 From: WEIREN FENG Date: Wed, 14 Oct 2026 13:26:31 -0700 Subject: [PATCH] Meetups: a join aborted by the crowd waits and tries again Everyone joining one meetup reads and writes the same document. On the test project, 199 people joined a meetup for twenty at once, and two of the joins were aborted for contention after the client library's own retries. Those two were told "internal" instead of that the meetup was full. The join now runs through retryOnContention, the wait-and-try-again that runEventOnce already used, moved out so both share it. Running a join again is safe: someone already on the roster is answered as joined. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../__tests__/meetup-join-contention.test.ts | 113 ++++++++++++++++++ functions/src/meetups.ts | 10 +- functions/src/shared.ts | 14 ++- 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 functions/src/__tests__/meetup-join-contention.test.ts diff --git a/functions/src/__tests__/meetup-join-contention.test.ts b/functions/src/__tests__/meetup-join-contention.test.ts new file mode 100644 index 0000000..a31353a --- /dev/null +++ b/functions/src/__tests__/meetup-join-contention.test.ts @@ -0,0 +1,113 @@ +import "./setup"; +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Transaction } from "firebase-admin/firestore"; +import { admin, db } from "../platform"; +import { joinMeetupCallable } from "../meetups"; +import { createPetCallable } from "../pets"; +import { CONTENTION_RETRY } from "../shared"; +import { callAs, clearRateLimits, errorCodeOf, purge } from "./helpers"; + +// Everyone joining one meetup reads and writes the same document. Two hundred +// at once on the test project had two joins aborted for contention, past the +// client library's own retries, and those two were told "internal". A +// contended join now waits and tries again. Running it again is safe: someone +// already on the roster is answered as joined. + +const ORGANIZER = "jc-organizer"; +const JOINER = "jc-joiner"; +const MEETUP_ID = "jc-meetup"; +const MEETUP = `meetups/${MEETUP_ID}`; + +const aborted = () => + Object.assign(new Error("10 ABORTED: Aborted due to cross-transaction contention."), { code: 10 }); + +/** Thrown inside the transaction to roll it back; the library does not retry it. */ +class GiveUp extends Error {} + +/** + * The first `failures` transactions that read the meetup end as though the + * client library had given up on them for contention. Others, such as the + * rate limit's, run as they are. + */ +function joinsContended(failures: number) { + const real = db.runTransaction.bind(db); + let joins = 0; + vi.spyOn(db, "runTransaction").mockImplementation((async ( + update: (t: Transaction) => Promise, + options?: Parameters[1] + ) => { + try { + return await real(async (t) => { + let readsTheMeetup = false; + const get = t.get.bind(t) as (ref: unknown) => unknown; + (t as unknown as { get: (ref: { path?: string }) => unknown }).get = (ref) => { + if (ref?.path === MEETUP) readsTheMeetup = true; + return get(ref); + }; + const result = await update(t); + if (readsTheMeetup && joins < failures) { + joins += 1; + throw new GiveUp(); + } + return result; + }, options); + } catch (error) { + if (error instanceof GiveUp) throw aborted(); + throw error; + } + }) as typeof db.runTransaction); + return { aborted: () => joins }; +} + +let petId = ""; +const saved = { ...CONTENTION_RETRY }; + +beforeEach(async () => { + Object.assign(CONTENTION_RETRY, { baseMs: 1, capMs: 2 }); + await purge(MEETUP, `users/${ORGANIZER}`, `users/${JOINER}`); + await clearRateLimits(); + await db.doc(`users/${ORGANIZER}`).set({ displayName: ORGANIZER }); + await db.doc(`users/${JOINER}`).set({ displayName: JOINER }); + await db.doc(MEETUP).set({ + organizerId: ORGANIZER, + title: "A crowded walk", + status: "upcoming", + participantCount: 1, + requirements: { maxPets: 20 }, + date: admin.firestore.Timestamp.fromMillis(Date.now() + 86_400_000), + }); + const pet = await callAs<{ id?: string; petId?: string }>(createPetCallable, JOINER, { name: "Crowd", species: "dog", gender: "male" }); + petId = (pet.id ?? pet.petId) as string; +}); +afterEach(() => { + vi.restoreAllMocks(); + Object.assign(CONTENTION_RETRY, saved); +}); +afterAll(async () => { + await purge(MEETUP, `users/${ORGANIZER}`, `users/${JOINER}`, `pets/${petId}`); + await clearRateLimits(); +}); + +describe("joining a meetup a crowd is joining", () => { + it("waits and tries again when the join is aborted for contention, and joins once", async () => { + const contention = joinsContended(3); + + const result = await callAs<{ success: boolean }>(joinMeetupCallable, JOINER, { meetupId: MEETUP_ID, petId }); + + expect(result.success).toBe(true); + expect(contention.aborted()).toBe(3); + expect((await db.doc(`${MEETUP}/participants/${JOINER}`).get()).exists).toBe(true); + expect((await db.doc(MEETUP).get()).get("participantCount")).toBe(2); + }); + + it("still fails when every try is contended", async () => { + const contention = joinsContended(Number.POSITIVE_INFINITY); + + const code = await errorCodeOf(() => callAs(joinMeetupCallable, JOINER, { meetupId: MEETUP_ID, petId })); + + expect(code).toContain("ABORTED"); + expect(contention.aborted()).toBe(CONTENTION_RETRY.retries + 1); + expect((await db.doc(`${MEETUP}/participants/${JOINER}`).get()).exists).toBe(false); + expect((await db.doc(MEETUP).get()).get("participantCount")).toBe(1); + }); +}); diff --git a/functions/src/meetups.ts b/functions/src/meetups.ts index 39980f9..cc8ac2a 100644 --- a/functions/src/meetups.ts +++ b/functions/src/meetups.ts @@ -14,6 +14,7 @@ import { requestData, requiredDocId, requiredTrimmedString, + retryOnContention, runEventOnce, stripUndefined, TRUSTED_MEDIA_URL_HOSTS, @@ -743,7 +744,12 @@ export const joinMeetupCallable = onCall(async (request) => { await assertNoBlockBetween(callerUid, admissionOrganizerId, "Joining this meetup"); } - return await db.runTransaction(async (t) => { + // Everyone joining one meetup reads and writes the same document, and two + // hundred at once had two joins aborted for contention on the test project, + // past the client library's own retries: told "internal" rather than that + // the meetup was full. Running the join again is safe — someone already on + // the roster is answered as joined. + return await retryOnContention(() => db.runTransaction(async (t) => { const meetupSnap = await t.get(meetupRef); if (!meetupSnap.exists) throw new HttpsError("not-found", "Meetup not found."); const meetup = meetupSnap.data() as Record; @@ -884,7 +890,7 @@ export const joinMeetupCallable = onCall(async (request) => { }); return { success: true }; - }); + })); }); // Flip expired meetups to "completed" even when no user visits the detail diff --git a/functions/src/shared.ts b/functions/src/shared.ts index b926fa4..3359891 100644 --- a/functions/src/shared.ts +++ b/functions/src/shared.ts @@ -383,9 +383,21 @@ export async function runEventOnce( // their counts aborted for contention on the test project, and these // triggers are not redelivered, so those likes were never counted. The // ledger makes another run safe, so a contended one waits and tries again. + return retryOnContention(once); +} + +/** + * Runs `attempt` again, after a growing wait, while it fails for contention. + * + * The client library retries a contended transaction too, but back to back + * and five times at most, and a crowd on one document outlasts that. Only for + * work that is safe to run again: runEventOnce has its ledger, and a meetup + * join finds the person already on the roster. + */ +export async function retryOnContention(attempt: () => Promise): Promise { for (let retry = 0; ; retry += 1) { try { - return await once(); + return await attempt(); } catch (error) { if (retry >= CONTENTION_RETRY.retries || !isContention(error)) throw error; const wait = Math.min(CONTENTION_RETRY.capMs, CONTENTION_RETRY.baseMs * 2 ** retry);