diff --git a/app/features/api-private/core/refresh-caches.server.ts b/app/features/api-private/core/refresh-caches.server.ts index 3860cb9a4..0affc9a25 100644 --- a/app/features/api-private/core/refresh-caches.server.ts +++ b/app/features/api-private/core/refresh-caches.server.ts @@ -8,6 +8,7 @@ import { refreshRunningTournaments, } from "~/features/tournament-bracket/core/Tournament.server"; import { refreshTentativeTiersCache } from "~/features/tournament-organization/core/tentativeTiers.server"; +import { clearUserCardCache } from "~/features/user-card/UserCardRepository.server"; import { cache } from "~/utils/cache.server"; /** @@ -19,6 +20,7 @@ export async function refreshCaches() { clearAllTournamentDataCache(); clearParticipationInfoMap(); clearSeasonSkillsCache(); + clearUserCardCache(); cache.clear(); await refreshBannedCache(); await refreshSendouQInstance(); diff --git a/app/features/chat/ChatProvider.tsx b/app/features/chat/ChatProvider.tsx index 480412d3f..b9c0f9202 100644 --- a/app/features/chat/ChatProvider.tsx +++ b/app/features/chat/ChatProvider.tsx @@ -20,7 +20,7 @@ import type { import { chatUsersSearchParams } from "./chat-search-params"; import type { ChatMessage, ChatUser } from "./chat-types"; import { messageTypeToSound, soundEnabled, soundVolume } from "./chat-utils"; -import { revalidateWithScope } from "./revalidation-scope"; +import { scheduleBroadcastRevalidation } from "./revalidation-scope"; import { ChatContext } from "./useChatContext"; const PING_INTERVAL_MS = 60_000; @@ -252,7 +252,12 @@ function ChatProviderInner({ const isOwnRevalidate = messageArr[0].revalidateOnly && messageArr[0].authorUserId === userId; if (!isOwnRevalidate) { - revalidateWithScope(revalidate, messageArr[0].revalidateScope); + // jittered so a broadcast fanning out to a whole room does not make + // every subscribed client refetch in the same instant + scheduleBroadcastRevalidation( + revalidate, + messageArr[0].revalidateScope, + ); } } diff --git a/app/features/chat/ChatSystemMessage.server.ts b/app/features/chat/ChatSystemMessage.server.ts index 0369edae4..6646c3c7a 100644 --- a/app/features/chat/ChatSystemMessage.server.ts +++ b/app/features/chat/ChatSystemMessage.server.ts @@ -7,6 +7,7 @@ import { IS_E2E_TEST_RUN } from "~/utils/e2e"; import invariant from "~/utils/invariant"; import { logger } from "~/utils/logger"; import type { ChatMessage } from "./chat-types"; +import { createRevalidateBroadcastThrottle } from "./revalidate-broadcast-throttle"; const SKALOP_TOKEN_HEADER_NAME = "Skalop-Token"; @@ -53,24 +54,57 @@ if (!IS_E2E_TEST_RUN) { systemMessagesDisabled = true; } +const REVALIDATE_BROADCAST_THROTTLE_WINDOW_MS = 2_000; + +const revalidateThrottle = createRevalidateBroadcastThrottle({ + windowMs: REVALIDATE_BROADCAST_THROTTLE_WINDOW_MS, + sendLeading: (msg) => postMessages([toFullMessage(msg)]), + // no author on purpose: the trailing broadcast covers many actors' changes, + // so no client may skip it as a duplicate of their own submission + sendTrailing: (msg) => + postMessages([ + { + id: nanoid(), + timestamp: Date.now(), + room: msg.room, + revalidateOnly: true, + revalidateScope: msg.revalidateScope, + }, + ]), +}); + export const send: ChatSystemMessageService["send"] = (partialMsg) => { if (systemMessagesDisabled) return; const msgArr = Array.isArray(partialMsg) ? partialMsg : [partialMsg]; - const fullMessages: ChatMessage[] = msgArr.map((partialMsg) => { - return { - id: nanoid(), - timestamp: Date.now(), - room: partialMsg.room, - context: partialMsg.context, - type: partialMsg.type, - revalidateOnly: partialMsg.revalidateOnly, - revalidateScope: partialMsg.revalidateScope, - authorUserId: partialMsg.authorUserId ?? actorIdOrNullSafe() ?? undefined, - }; - }); + const immediate: PartialChatMessage[] = []; + for (const msg of msgArr) { + if (revalidateThrottle.throttles(msg)) { + revalidateThrottle.handle(msg); + } else { + immediate.push(msg); + } + } + if (immediate.length === 0) return; + return postMessages(immediate.map(toFullMessage)); +}; + +function toFullMessage(partialMsg: PartialChatMessage): ChatMessage { + return { + id: nanoid(), + timestamp: Date.now(), + room: partialMsg.room, + context: partialMsg.context, + type: partialMsg.type, + revalidateOnly: partialMsg.revalidateOnly, + revalidateScope: partialMsg.revalidateScope, + authorUserId: partialMsg.authorUserId ?? actorIdOrNullSafe() ?? undefined, + }; +} + +function postMessages(fullMessages: ChatMessage[]) { return void fetch(ServerConfig.skalop.systemMessageUrl!, { method: "POST", body: JSON.stringify({ @@ -82,7 +116,7 @@ export const send: ChatSystemMessageService["send"] = (partialMsg) => { ["Content-Type", "application/json"], ], }).catch(logSkalpError("sendMessage")); -}; +} export function removeRoom(chatCode: string) { if (systemMessagesDisabled) return; diff --git a/app/features/chat/revalidate-broadcast-throttle.test.ts b/app/features/chat/revalidate-broadcast-throttle.test.ts new file mode 100644 index 000000000..632e0fe75 --- /dev/null +++ b/app/features/chat/revalidate-broadcast-throttle.test.ts @@ -0,0 +1,167 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { + createRevalidateBroadcastThrottle, + MAX_ENTRIES, +} from "./revalidate-broadcast-throttle"; + +const WINDOW_MS = 2_000; + +const setup = () => { + const sendLeading = vi.fn(); + const sendTrailing = vi.fn(); + const throttle = createRevalidateBroadcastThrottle({ + windowMs: WINDOW_MS, + sendLeading, + sendTrailing, + }); + return { throttle, sendLeading, sendTrailing }; +}; + +describe("createRevalidateBroadcastThrottle", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test("throttles revalidation broadcasts but not chat messages", () => { + const { throttle } = setup(); + + expect(throttle.throttles({ revalidateOnly: true })).toBe(true); + expect( + throttle.throttles({ revalidateOnly: true, type: "TOURNAMENT_UPDATED" }), + ).toBe(true); + expect( + throttle.throttles({ revalidateOnly: true, type: "MATCH_STARTED" }), + ).toBe(true); + expect(throttle.throttles({})).toBe(false); + }); + + test("first broadcast of a window is delivered immediately", () => { + const { throttle, sendLeading, sendTrailing } = setup(); + + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + + expect(sendLeading).toHaveBeenCalledTimes(1); + expect(sendTrailing).not.toHaveBeenCalled(); + }); + + test("broadcasts within the window coalesce into one trailing broadcast", () => { + const { throttle, sendLeading, sendTrailing } = setup(); + + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + vi.advanceTimersByTime(500); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + + expect(sendLeading).toHaveBeenCalledTimes(1); + expect(sendTrailing).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(WINDOW_MS - 500); + expect(sendTrailing).toHaveBeenCalledTimes(1); + expect(sendTrailing).toHaveBeenCalledWith({ + room: "sq-looking", + revalidateScope: undefined, + }); + }); + + test("a broadcast after the window opens a fresh window and sends immediately", () => { + const { throttle, sendLeading, sendTrailing } = setup(); + + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + vi.advanceTimersByTime(WINDOW_MS); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + + expect(sendLeading).toHaveBeenCalledTimes(2); + expect(sendTrailing).not.toHaveBeenCalled(); + }); + + test("rooms are throttled independently", () => { + const { throttle, sendLeading } = setup(); + + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + throttle.handle({ room: "tournament-1", revalidateOnly: true }); + + expect(sendLeading).toHaveBeenCalledTimes(2); + }); + + test("coalesced broadcasts of one scope keep it, differing scopes widen to unset", () => { + const { throttle, sendTrailing } = setup(); + + throttle.handle({ room: "tournament-1", revalidateOnly: true }); + throttle.handle({ + room: "tournament-1", + revalidateOnly: true, + revalidateScope: "MATCH_RESULTS", + }); + throttle.handle({ + room: "tournament-1", + revalidateOnly: true, + revalidateScope: "MATCH_RESULTS", + }); + vi.advanceTimersByTime(WINDOW_MS); + expect(sendTrailing).toHaveBeenLastCalledWith({ + room: "tournament-1", + revalidateScope: "MATCH_RESULTS", + }); + + throttle.handle({ room: "tournament-1", revalidateOnly: true }); + throttle.handle({ + room: "tournament-1", + revalidateOnly: true, + revalidateScope: "MATCH_RESULTS", + }); + throttle.handle({ room: "tournament-1", revalidateOnly: true }); + vi.advanceTimersByTime(WINDOW_MS); + expect(sendTrailing).toHaveBeenLastCalledWith({ + room: "tournament-1", + revalidateScope: undefined, + }); + }); + + test("forgetting idle rooms spares one whose trailing broadcast is still pending", () => { + const { throttle, sendLeading, sendTrailing } = setup(); + const startedAt = Date.now(); + + for (let i = 0; i <= MAX_ENTRIES; i++) { + throttle.handle({ room: `tournament-${i}`, revalidateOnly: true }); + } + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + sendLeading.mockClear(); + + // every room is idle by now, but the trailing broadcast is late rather than sent + // (its timer would have run at the window's end on a server that was not busy) + vi.setSystemTime(startedAt + WINDOW_MS + 1_000); + throttle.handle({ room: "tournament-late", revalidateOnly: true }); + expect(sendLeading).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(WINDOW_MS); + expect(sendTrailing).toHaveBeenCalledTimes(1); + expect(sendTrailing).toHaveBeenCalledWith({ + room: "sq-looking", + revalidateScope: undefined, + }); + }); + + test("the trailing broadcast starts a new window", () => { + const { throttle, sendLeading, sendTrailing } = setup(); + + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + vi.advanceTimersByTime(1_000); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + vi.advanceTimersByTime(1_000); + expect(sendTrailing).toHaveBeenCalledTimes(1); + + // still within the trailing broadcast's window: coalesces again + vi.advanceTimersByTime(500); + throttle.handle({ room: "sq-looking", revalidateOnly: true }); + expect(sendLeading).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(WINDOW_MS); + expect(sendTrailing).toHaveBeenCalledTimes(2); + }); +}); diff --git a/app/features/chat/revalidate-broadcast-throttle.ts b/app/features/chat/revalidate-broadcast-throttle.ts new file mode 100644 index 000000000..8d5de0715 --- /dev/null +++ b/app/features/chat/revalidate-broadcast-throttle.ts @@ -0,0 +1,100 @@ +import type { ChatMessage } from "./chat-types"; + +type ThrottleableMessage = Pick< + ChatMessage, + "room" | "type" | "revalidateOnly" | "revalidateScope" +>; + +interface ThrottleEntry { + lastSentAt: number; + trailing: { + scope: ChatMessage["revalidateScope"]; + timer: ReturnType; + } | null; +} + +/** Above this many rooms tracked, idle ones are forgotten (see `prune`). */ +export const MAX_ENTRIES = 5_000; + +/** + * Rate limits `revalidateOnly` broadcasts per room so that a burst of them + * (e.g. every player of a forming SendouQ match confirming within seconds, or every + * reported game of a live tournament) fans out to the room's subscribers at most + * twice per window instead of once per event — each fan-out makes every subscribed + * client refetch its loaders at once. + * + * The first broadcast of a window is delivered immediately; further broadcasts for + * the same room within the window coalesce into a single trailing broadcast at the + * window's end, so subscribers never miss the final state. A trailing broadcast + * covers every coalesced one: its scope is widened to the broadest seen and it + * carries no author (nobody may skip it as their own) and no type (an absorbed + * broadcast's sound is dropped — the room heard the window's leading one already). + */ +export function createRevalidateBroadcastThrottle({ + windowMs, + sendLeading, + sendTrailing, +}: { + windowMs: number; + /** Delivers a broadcast that opened a fresh window, unaltered. */ + sendLeading: (msg: ThrottleableMessage) => void; + /** Delivers the coalesced trailing broadcast of a window. */ + sendTrailing: (msg: { + room: string; + revalidateScope: ChatMessage["revalidateScope"]; + }) => void; +}) { + const entries = new Map(); + + const prune = (now: number) => { + if (entries.size <= MAX_ENTRIES) return; + for (const [room, entry] of entries) { + if (!entry.trailing && now - entry.lastSentAt >= windowMs) { + entries.delete(room); + } + } + }; + + return { + /** + * Whether the throttle applies to the message: every revalidation broadcast. + * Real chat messages always pass through untouched. + */ + throttles(msg: Pick): boolean { + return Boolean(msg.revalidateOnly); + }, + handle(msg: ThrottleableMessage): void { + const now = Date.now(); + prune(now); + + const entry = entries.get(msg.room); + if (!entry || (!entry.trailing && now - entry.lastSentAt >= windowMs)) { + entries.set(msg.room, { lastSentAt: now, trailing: null }); + sendLeading(msg); + return; + } + + if (entry.trailing) { + // an unset scope means anything may have changed, so differing scopes widen to unset + if (entry.trailing.scope !== msg.revalidateScope) { + entry.trailing.scope = undefined; + } + return; + } + + const timer = setTimeout( + () => { + const current = entries.get(msg.room); + if (!current?.trailing) return; + current.lastSentAt = Date.now(); + const scope = current.trailing.scope; + current.trailing = null; + sendTrailing({ room: msg.room, revalidateScope: scope }); + }, + windowMs - (now - entry.lastSentAt), + ); + timer.unref?.(); + entry.trailing = { scope: msg.revalidateScope, timer }; + }, + }; +} diff --git a/app/features/chat/revalidation-scope.test.ts b/app/features/chat/revalidation-scope.test.ts index 9184405ba..1b7b46f10 100644 --- a/app/features/chat/revalidation-scope.test.ts +++ b/app/features/chat/revalidation-scope.test.ts @@ -1,8 +1,9 @@ import type { ShouldRevalidateFunctionArgs } from "react-router"; -import { describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { isMatchResultsScopedRevalidation, revalidateWithScope, + scheduleBroadcastRevalidation, } from "./revalidation-scope"; const revalidationArgs = () => @@ -71,3 +72,59 @@ describe("revalidateWithScope", () => { await flushMicrotasks(); }); }); + +describe("scheduleBroadcastRevalidation", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.runAllTimers(); + vi.useRealTimers(); + }); + + test("revalidates once after a delay instead of immediately", () => { + const revalidate = vi.fn(() => Promise.resolve()); + + scheduleBroadcastRevalidation(revalidate, undefined); + expect(revalidate).not.toHaveBeenCalled(); + + vi.runAllTimers(); + expect(revalidate).toHaveBeenCalledTimes(1); + }); + + test("broadcasts arriving while one is scheduled are absorbed into it", () => { + const revalidate = vi.fn(() => Promise.resolve()); + + scheduleBroadcastRevalidation(revalidate, undefined); + scheduleBroadcastRevalidation(revalidate, undefined); + scheduleBroadcastRevalidation(revalidate, undefined); + + vi.runAllTimers(); + expect(revalidate).toHaveBeenCalledTimes(1); + }); + + test("same-scope broadcasts keep the scope active during the revalidation", () => { + const { promise, resolve } = deferred(); + + scheduleBroadcastRevalidation(() => promise, "MATCH_RESULTS"); + scheduleBroadcastRevalidation(() => promise, "MATCH_RESULTS"); + + vi.runAllTimers(); + expect(isMatchResultsScopedRevalidation(revalidationArgs())).toBe(true); + + resolve(); + }); + + test("an absorbed unscoped broadcast widens the scheduled scope", () => { + const { promise, resolve } = deferred(); + + scheduleBroadcastRevalidation(() => promise, "MATCH_RESULTS"); + scheduleBroadcastRevalidation(() => promise, undefined); + + vi.runAllTimers(); + expect(isMatchResultsScopedRevalidation(revalidationArgs())).toBe(false); + + resolve(); + }); +}); diff --git a/app/features/chat/revalidation-scope.ts b/app/features/chat/revalidation-scope.ts index 0568ccc39..32ee32c8a 100644 --- a/app/features/chat/revalidation-scope.ts +++ b/app/features/chat/revalidation-scope.ts @@ -2,8 +2,11 @@ import type { ShouldRevalidateFunctionArgs } from "react-router"; import { isRevalidation } from "~/utils/remix"; import type { RevalidateScope } from "./chat-types"; +const BROADCAST_REVALIDATE_MAX_JITTER_MS = 1_500; + let activeScope: RevalidateScope | null = null; let pendingRevalidations = 0; +let scheduledBroadcast: { scope: RevalidateScope | null } | null = null; /** * Runs a websocket broadcast triggered revalidation, remembering the broadcast's scope @@ -30,6 +33,33 @@ export function revalidateWithScope( }); } +/** + * Runs a websocket broadcast triggered revalidation after a random delay so the clients + * subscribed to a topic do not all refetch in the same instant the broadcast fans out + * (thundering herd — a broadcast to e.g. the SendouQ looking room or a live tournament's + * room reaches every client on that page at once). A broadcast arriving while one is + * already scheduled is absorbed into it, widening its scope as needed: the eventual + * single fetch returns data fresh enough to cover both. + */ +export function scheduleBroadcastRevalidation( + revalidate: () => Promise, + scope: RevalidateScope | undefined, +) { + if (scheduledBroadcast) { + if (scheduledBroadcast.scope !== (scope ?? null)) { + scheduledBroadcast.scope = null; + } + return; + } + + const pending: { scope: RevalidateScope | null } = { scope: scope ?? null }; + scheduledBroadcast = pending; + setTimeout(() => { + scheduledBroadcast = null; + revalidateWithScope(revalidate, pending.scope ?? undefined); + }, Math.random() * BROADCAST_REVALIDATE_MAX_JITTER_MS); +} + /** * Whether the pending revalidation is a websocket broadcast scoped to match results, * meaning only match data (reported scores, pick/ban events) changed. Loaders whose data diff --git a/app/features/sendouq/core/ready-check.server.test.ts b/app/features/sendouq/core/ready-check.server.test.ts index ce39b0b82..2186282aa 100644 --- a/app/features/sendouq/core/ready-check.server.test.ts +++ b/app/features/sendouq/core/ready-check.server.test.ts @@ -11,9 +11,15 @@ import { backdate } from "~/db/seed/core/backdate"; import * as SQGroupFactory from "~/db/seed/factories/SQGroupFactory"; import * as UserFactory from "~/db/seed/factories/UserFactory"; import { db } from "~/db/sql"; +import * as ChatSystemMessage from "~/features/chat/ChatSystemMessage.server"; import * as SQGroupRepository from "~/features/sendouq/SQGroupRepository.server"; import invariant from "~/utils/invariant"; -import { FULL_GROUP_SIZE, SENDOUQ } from "../q-constants"; +import { + FULL_GROUP_SIZE, + SENDOUQ, + SENDOUQ_LOOKING_ROOM, + sqGroupWebsocketRoom, +} from "../q-constants"; import * as ReadyCheck from "./ready-check.server"; import { refreshSendouQInstance, SendouQ } from "./SendouQ.server"; @@ -56,6 +62,13 @@ const findGroupStatus = async (groupId: number) => { const findMatch = () => db.selectFrom("GroupMatch").selectAll().executeTakeFirst(); +/** Rooms every system message sent so far was broadcast to, in order. */ +const broadcastedRooms = () => + vi + .mocked(ChatSystemMessage.send) + .mock.calls.flatMap(([msg]) => (Array.isArray(msg) ? msg : [msg])) + .map((msg) => msg.room); + /** Confirms every member of both groups as ready, which is what creates the match. */ const confirmEveryoneReady = async (groupId: number) => { for (;;) { @@ -76,6 +89,7 @@ describe("SendouQ ready check", () => { beforeEach(async () => { groups = await setupMatchedUpGroups(); + vi.mocked(ChatSystemMessage.send).mockClear(); }); test("takes both groups out of the looking pool", async () => { @@ -178,6 +192,23 @@ describe("SendouQ ready check", () => { expect(await findMatch()).toBeUndefined(); }); + test("a confirmation revalidates only the two groups, expiring also the looking pool", async () => { + const readyCheck = await findReadyCheck(groups.ownGroup.id); + invariant(readyCheck); + + await ReadyCheck.confirm({ readyCheck, userId: groups.ownMembers[1].id }); + + // the looking pool is unchanged while the ready check runs, so it is left alone + expect(broadcastedRooms()).toEqual([ + sqGroupWebsocketRoom(groups.ownGroup.id), + sqGroupWebsocketRoom(groups.theirGroup.id), + ]); + + await ReadyCheck.expire(readyCheck); + + expect(broadcastedRooms()).toContain(SENDOUQ_LOOKING_ROOM); + }); + test("expiring sends both groups back to looking and marks who missed it", async () => { const readyCheck = await findReadyCheck(groups.ownGroup.id); invariant(readyCheck); diff --git a/app/features/sendouq/core/ready-check.server.ts b/app/features/sendouq/core/ready-check.server.ts index 8a19c9274..7b700d52a 100644 --- a/app/features/sendouq/core/ready-check.server.ts +++ b/app/features/sendouq/core/ready-check.server.ts @@ -152,6 +152,11 @@ export async function expire(readyCheck: { await refreshSendouQInstance(); revalidateGroups(readyCheck); + // both groups return to the looking pool, so its shape changed for everyone + ChatSystemMessage.send({ + room: SENDOUQ_LOOKING_ROOM, + revalidateOnly: true, + }); } async function createMatch({ @@ -251,9 +256,5 @@ function revalidateGroups(readyCheck: { room: sqGroupWebsocketRoom(readyCheck.bravoGroupId), revalidateOnly: true, }, - { - room: SENDOUQ_LOOKING_ROOM, - revalidateOnly: true, - }, ]); } diff --git a/app/features/sendouq/loaders/q.looking.server.ts b/app/features/sendouq/loaders/q.looking.server.ts index 071bfa872..a22620454 100644 --- a/app/features/sendouq/loaders/q.looking.server.ts +++ b/app/features/sendouq/loaders/q.looking.server.ts @@ -41,7 +41,7 @@ export const loader = async ({ url }: LoaderFunctionArgs) => { ]); return { - ...(await UserCardRepository.findAllByUserIds({ + ...(await UserCardRepository.findAllByUserIdsCached({ userIds: cardUserIds, })), groups: groupsToShow, diff --git a/app/features/user-card/UserCardRepository.server.test.ts b/app/features/user-card/UserCardRepository.server.test.ts index 9b36d3294..e34fedb98 100644 --- a/app/features/user-card/UserCardRepository.server.test.ts +++ b/app/features/user-card/UserCardRepository.server.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from "vitest"; import * as ImageFactory from "~/db/seed/factories/ImageFactory"; import * as UserFactory from "~/db/seed/factories/UserFactory"; import * as XRankPlacementFactory from "~/db/seed/factories/XRankPlacementFactory"; +import * as PrivateUserNoteRepository from "~/features/sendouq/PrivateUserNoteRepository.server"; import { withNoUser, withUserId } from "~/utils/Test"; import * as UserCardRepository from "./UserCardRepository.server"; import type { UserCardData } from "./user-card-types"; @@ -347,3 +348,101 @@ describe("UserCardRepository.findAllByUserIds", () => { expect(banner).toHaveProperty("url"); }); }); + +describe("UserCardRepository.findAllByUserIdsCached", () => { + let target: { id: number }; + let viewer: { id: number }; + let otherViewer: { id: number }; + + beforeEach(async () => { + // user ids repeat between tests, so cards cached by an earlier test would be served here + UserCardRepository.clearUserCardCache(); + [target, viewer, otherViewer] = await UserFactory.createMany(3); + }); + + const cachedCard = (userId: number, actorUserId?: number) => { + const find = () => + UserCardRepository.findAllByUserIdsCached({ userIds: [userId] }); + + return typeof actorUserId === "number" + ? withUserId(actorUserId, find) + : withNoUser(find); + }; + + it("gives every viewer their own private note", async () => { + await withUserId(viewer.id, () => + PrivateUserNoteRepository.upsertOwnNote({ + targetId: target.id, + sentiment: "POSITIVE", + text: "great teammate", + }), + ); + + const forAuthor = await cachedCard(target.id, viewer.id); + expect(forAuthor.userCards.get(target.id)?.privateNote).toMatchObject({ + sentiment: "POSITIVE", + text: "great teammate", + }); + + // served from the entry the call above cached, which must not carry its note + const forOther = await cachedCard(target.id, otherViewer.id); + expect(forOther.userCards.get(target.id)?.privateNote).toBeNull(); + + const forAnonymous = await cachedCard(target.id); + expect(forAnonymous.userCards.get(target.id)?.privateNote).toBeNull(); + }); + + it("serves cards from the cache and queries only the users missing from it", async () => { + const first = await cachedCard(target.id); + expect(first.userCards.get(target.id)?.shortBio).toBeNull(); + + await withUserId(target.id, () => + UserCardRepository.updateOwnCard({ + shortBio: "edited", + bannerPresetImg: null, + bannerImgId: null, + unverifiedPeakXP: null, + hiddenCardStats: [], + }), + ); + + const { userCards } = await withNoUser(() => + UserCardRepository.findAllByUserIdsCached({ + userIds: [target.id, viewer.id], + }), + ); + + expect(userCards.get(target.id)?.shortBio).toBeNull(); + expect(userCards.get(viewer.id)?.id).toBe(viewer.id); + }); + + it("returns a fresh card once the cached one has expired", async () => { + await cachedCard(target.id); + + await withUserId(target.id, () => + UserCardRepository.updateOwnCard({ + shortBio: "edited", + bannerPresetImg: null, + bannerImgId: null, + unverifiedPeakXP: null, + hiddenCardStats: [], + }), + ); + UserCardRepository.clearUserCardCache(); + + const { userCards } = await cachedCard(target.id); + expect(userCards.get(target.id)?.shortBio).toBe("edited"); + }); + + it("coalesces concurrent misses for the same user into one query", async () => { + const [first, second] = await Promise.all([ + cachedCard(target.id, viewer.id), + cachedCard(target.id, otherViewer.id), + ]); + + // both viewers were served the same cached card, so only one query built it + expect(first.userCards.get(target.id)?.stats).toBe( + second.userCards.get(target.id)?.stats, + ); + }); +}); diff --git a/app/features/user-card/UserCardRepository.server.ts b/app/features/user-card/UserCardRepository.server.ts index a9dd72bf7..19f1af0c8 100644 --- a/app/features/user-card/UserCardRepository.server.ts +++ b/app/features/user-card/UserCardRepository.server.ts @@ -2,6 +2,7 @@ import { sub } from "date-fns"; import type { Expression, ExpressionBuilder } from "kysely"; import { sql } from "kysely"; import { jsonBuildObject, jsonObjectFrom } from "kysely/helpers/sqlite"; +import { ServerConfig } from "~/config.server"; import { db } from "~/db/sql"; import type { Tables } from "~/db/tables"; import type { CustomTheme, PeakXP } from "~/db/tables-json"; @@ -13,6 +14,7 @@ import { TIERS } from "~/features/mmr/mmr-constants"; import type { TieredSkill } from "~/features/mmr/tiered.server"; import { userSkills } from "~/features/mmr/tiered.server"; import type { XRankPlacementRegion } from "~/features/top-search/top-search-types"; +import { LRUCache } from "~/modules/cache"; import type { StageId } from "~/modules/in-game-lists/types"; import { dateToDatabaseTimestamp } from "~/utils/dates"; import { @@ -55,8 +57,120 @@ export async function findAllByUserIds({ }): Promise<{ userCards: Map }> { if (userIds.length === 0) return { userCards: new Map() }; - const viewerId = actorIdOrNull(); + return { + userCards: await queryUserCards({ + userIds, + viewerId: actorIdOrNull(), + include, + includeHiddenStats, + }), + }; +} +const CARD_CACHE_TTL_MS = 30 * 1000; +const CARD_CACHE_MAX_ENTRIES = 2_000; +const cardCache = new LRUCache< + number, + { storedAt: number; card: Promise } +>({ max: CARD_CACHE_MAX_ENTRIES }); + +/** + * Like {@link findAllByUserIds} (with the default options) but serves the + * viewer-independent card data from a short-lived in-memory cache, querying only users + * whose entry is missing or stale. The per-viewer `privateNote` is overlaid fresh on + * every call so a cached card is never viewer-specific. For high-frequency views (the + * SendouQ looking page) where broadcast-driven revalidation makes many clients rebuild + * the same cards at once; cards may be up to 30 seconds stale. + * + * The cache holds the in-flight query rather than its result, so concurrent misses for + * the same user (exactly what a revalidation burst causes) await one shared query + * instead of each firing their own. + */ +export async function findAllByUserIdsCached({ + userIds, +}: { + userIds: Array; +}): Promise<{ userCards: Map }> { + if (ServerConfig.disableCache) return findAllByUserIds({ userIds }); + if (userIds.length === 0) return { userCards: new Map() }; + + const now = Date.now(); + const pendingCards = new Map>(); + const missingIds: Array = []; + for (const userId of userIds) { + const entry = cardCache.get(userId); + if (entry && now - entry.storedAt < CARD_CACHE_TTL_MS) { + pendingCards.set(userId, entry.card); + } else { + missingIds.push(userId); + } + } + + if (missingIds.length > 0) { + const query = queryUserCards({ userIds: missingIds, viewerId: null }); + for (const userId of missingIds) { + pendingCards.set(userId, cacheQueriedCard({ userId, query, now })); + } + } + + const cards = new Map(); + for (const [userId, pendingCard] of pendingCards) { + const card = await pendingCard; + if (card) cards.set(userId, card); + } + + const privateNotes = await findPrivateNotesByTargetIds([...cards.keys()]); + + const userCards = new Map(); + for (const [userId, card] of cards) { + userCards.set(userId, { + ...card, + privateNote: privateNotes.get(userId) ?? null, + }); + } + + return { userCards }; +} + +/** Forgets every cached card, so the next read builds them from the database again. */ +export function clearUserCardCache() { + cardCache.clear(); +} + +function cacheQueriedCard({ + userId, + query, + now, +}: { + userId: number; + query: Promise>; + now: number; +}) { + const card = query.then((userCards) => userCards.get(userId)); + const entry = { storedAt: now, card }; + + // a failed query must not stay behind as this user's cached entry + card.catch(() => { + if (cardCache.get(userId) === entry) { + cardCache.delete(userId); + } + }); + cardCache.set(userId, entry); + + return card; +} + +async function queryUserCards({ + userIds, + viewerId, + include, + includeHiddenStats = false, +}: { + userIds: Array; + viewerId: number | null; + include?: { friendCode?: boolean }; + includeHiddenStats?: boolean; +}): Promise> { // a user's card surfaces the better of their last two finished seasons (see bestSeasonResult) const [rows, seasonResults] = await Promise.all([ db @@ -85,7 +199,33 @@ export async function findAllByUserIds({ ); } - return { userCards }; + return userCards; +} + +/** The acting user's private notes about the given users, keyed by their user id. */ +async function findPrivateNotesByTargetIds(userIds: Array) { + const notes = new Map>(); + + const viewerId = actorIdOrNull(); + if (viewerId === null || userIds.length === 0) return notes; + + const rows = await db + .selectFrom("PrivateUserNote") + .select([ + "PrivateUserNote.targetId", + "PrivateUserNote.text", + "PrivateUserNote.sentiment", + "PrivateUserNote.updatedAt", + ]) + .where("PrivateUserNote.authorId", "=", viewerId) + .where("PrivateUserNote.targetId", "in", userIds) + .execute(); + + for (const { targetId, ...privateNote } of rows) { + notes.set(targetId, privateNote); + } + + return notes; } /** diff --git a/scripts/benchmark-db/cases.ts b/scripts/benchmark-db/cases.ts index 389173125..6b36df3f9 100644 --- a/scripts/benchmark-db/cases.ts +++ b/scripts/benchmark-db/cases.ts @@ -1153,6 +1153,9 @@ export function buildCases(fx: Fixtures): { includeHiddenStats: true, }), ); + add("UserCardRepository.findAllByUserIdsCached", fx.manyUserIds, (userIds) => + UserCardRepository.findAllByUserIdsCached({ userIds }), + ); add("UserCardRepository.findCardEditExtrasByUserId", fx.heavyUser, (user) => UserCardRepository.findCardEditExtrasByUserId(user.id), );