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);