From df2d2af75b80b23859982db8031633567c224609 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Fri, 7 Aug 2026 17:24:08 -0400 Subject: [PATCH] remove empty instances --- apps/match/src/match.app.ts | 29 +++-- apps/match/src/test/integration/api.test.ts | 117 ++++++++++++++++---- apps/match/wrangler.jsonc | 7 +- packages/domain/src/room-instance-db.ts | 51 +++++++++ 4 files changed, 170 insertions(+), 34 deletions(-) diff --git a/apps/match/src/match.app.ts b/apps/match/src/match.app.ts index 069d326..d306419 100644 --- a/apps/match/src/match.app.ts +++ b/apps/match/src/match.app.ts @@ -6,6 +6,7 @@ import { areFriends, canManageRoom, createRoomInstance, + deleteEmptyRoomInstances, deleteExpiredPresence, deletePresence, GAME_VERSION, @@ -1475,24 +1476,33 @@ const app = new Hono() ) /** - * Cron: sweep presence that has aged past its TTL. Reads already ignore expired rows, - * so this isn't about correctness of `/player` — it's that a player who crashed or - * hard-quit never matchmakes out of their instance, so nothing recomputes that - * instance's fullness and it can stay flagged full (and unjoinable) with nobody in it. - * Recompute the instances the expiring rows point at, *then* delete: the sweep is the - * only thing that notices those departures. Fullness is recomputed after the delete so - * the head-count no longer sees them. + * Cron: sweep presence that has aged past its TTL, then the instances left empty. + * + * The presence purge isn't about correctness of `/player` — reads already ignore + * expired rows. It's that a player who crashed or hard-quit never matchmakes out of + * their instance, so nothing recomputes that instance's fullness and it can stay + * flagged full (and unjoinable) with nobody in it. Note the instances the expiring + * rows point at *before* deleting: the sweep is the only thing that notices those + * departures. + * + * Emptying an instance is what makes it garbage — nothing ever reuses it, and a + * joiner handed one would land alone in a Photon room everyone left — so the empty + * sweep runs next. It reads presence without consulting expiry, so it depends on + * running after the purge above: this order is what makes a lapsed row count as a + * departure. Fullness is recomputed last, so it works from the final head-count and + * skips (returns null for) the instances just deleted. */ async function sweepExpiredPresence(env: Env): Promise { const staleInstanceIds = await getExpiredPresenceInstanceIds(env.DB) const removed = await deleteExpiredPresence(env.DB) + const emptyInstanceIds = await deleteEmptyRoomInstances(env.DB) for (const instanceId of staleInstanceIds) { await refreshInstanceFullness(env.DB, instanceId) } // The tagged logger is request-scoped (its middleware never runs for a cron), so // log plainly here — Workers observability picks it up either way. console.log( - `presence sweep: removed ${removed} expired rows, refreshed ${staleInstanceIds.length} instances` + `presence sweep: removed ${removed} expired rows, deleted ${emptyInstanceIds.length} empty instances, refreshed ${staleInstanceIds.length} instances` ) } @@ -1513,7 +1523,8 @@ app.get( 'Room backend. Rooms and room instances are D1-backed (matchmaking finds or creates a', '`room_instance` per session); presence — the instance each player is currently in —', 'lives in the shared `presence` table and expires on a TTL. A cron sweep clears', - 'expired presence and frees up instances a crashed player never left.', + 'expired presence, frees up instances a crashed player never left, and deletes', + 'instances nobody is standing in any more.', ].join('\n'), }, servers: [{ url: 'https://match.recflare.net', description: 'Production' }], diff --git a/apps/match/src/test/integration/api.test.ts b/apps/match/src/test/integration/api.test.ts index ee875f7..b3fb610 100644 --- a/apps/match/src/test/integration/api.test.ts +++ b/apps/match/src/test/integration/api.test.ts @@ -11,6 +11,7 @@ import { beforeAll, describe, expect, test } from 'vitest' import { countPlayersInInstance, createRoomInstance, + EMPTY_INSTANCE_GRACE_SECONDS, GAME_VERSION, getRoomInstance, PRESENCE_SCHEMA_DDL, @@ -731,14 +732,15 @@ describe('auth-gated endpoints', () => { expect(await stale.text()).toBe('') }) - // Seed presence directly into D1 with a chosen `expiresAt` (epoch seconds) so the - // TTL-refresh branch can be exercised deterministically (independent of timing). - const seedPresence = (id: number, expiresAt: number) => + // Seed presence directly into D1 with a chosen instance and `expiresAt` (epoch + // seconds), so the TTL branches can be exercised deterministically (independent of + // timing) and a player can be planted in an instance without matchmaking there. + const seedPresenceInInstance = (id: number, roomInstanceId: number, expiresAt: number) => env.DB.prepare('INSERT OR REPLACE INTO presence (data) VALUES (?1)') .bind( JSON.stringify({ accountId: id, - roomInstance: { roomInstanceId: 1000042, roomId: 1 }, + roomInstance: { roomInstanceId, roomId: 1 }, statusVisibility: 0, deviceClass: 0, vrMovementMode: 1, @@ -749,6 +751,9 @@ describe('auth-gated endpoints', () => { ) .run() + const seedPresence = (id: number, expiresAt: number) => + seedPresenceInInstance(id, 1000042, expiresAt) + const storedExpiresAt = async (id: number): Promise => { const row = await env.DB.prepare('SELECT data FROM presence WHERE account_id = ?1') .bind(id) @@ -792,24 +797,9 @@ describe('auth-gated endpoints', () => { test('countPlayersInInstance counts live players in a room instance (excludes expired)', async () => { // Three players in instance 1000099 — two live, one expired. - const seedInInstance = (id: number, expiresAt: number) => - env.DB.prepare('INSERT OR REPLACE INTO presence (data) VALUES (?1)') - .bind( - JSON.stringify({ - accountId: id, - roomInstance: { roomInstanceId: 1000099, roomId: 2 }, - statusVisibility: 0, - deviceClass: 0, - vrMovementMode: 1, - platform: 0, - appVersion: GAME_VERSION, - expiresAt, - }) - ) - .run() - await seedInInstance(710, nowSeconds() + 800) - await seedInInstance(711, nowSeconds() + 800) - await seedInInstance(712, nowSeconds() - 10) // already expired → not counted + await seedPresenceInInstance(710, 1000099, nowSeconds() + 800) + await seedPresenceInInstance(711, 1000099, nowSeconds() + 800) + await seedPresenceInInstance(712, 1000099, nowSeconds() - 10) // expired → not counted expect(await countPlayersInInstance(env.DB, 1000099)).toBe(2) expect(await countPlayersInInstance(env.DB, 999999)).toBe(0) }) @@ -872,6 +862,89 @@ describe('auth-gated endpoints', () => { expect((await getRoomInstance(env.DB, solo))?.isFull).toBe(false) }) + // Age an instance past EMPTY_INSTANCE_GRACE_SECONDS by backdating its `createdAt` + // (the generated `created_at` column follows the blob), so the empty-instance sweep + // can be exercised without waiting out the grace window. + const backdateInstance = (id: number, secondsAgo = EMPTY_INSTANCE_GRACE_SECONDS + 60) => + env.DB.prepare( + "UPDATE room_instance SET data = json_set(data, '$.createdAt', ?2) WHERE id = ?1" + ) + .bind(id, new Date(Date.now() - secondsAgo * 1000).toISOString()) + .run() + + const expirePresence = (accountId: number) => + env.DB.prepare( + "UPDATE presence SET data = json_set(data, '$.expiresAt', ?2) WHERE account_id = ?1" + ) + .bind(accountId, nowSeconds() - 10) + .run() + + test('the cron sweep deletes instances nobody is left standing in', async () => { + // Two instances built directly rather than by matchmaking, so neither is one a + // previous test's player is still standing in (public matchmakes reuse instances). + // One holds a player who crashed out — an expired row the sweep purges first, + // leaving the instance empty — the other a live player. + const abandoned = await createRoomInstance(env.DB, { + ownerAccountId: 830, + roomId: 2, + photonRoomId: 'abandoned-instance', + maxCapacity: 12, + }) + await seedPresenceInInstance(830, abandoned.roomInstanceId, nowSeconds() - 10) + const occupied = await createRoomInstance(env.DB, { + ownerAccountId: 831, + roomId: 2, + photonRoomId: 'occupied-instance', + maxCapacity: 12, + }) + await seedPresenceInInstance(831, occupied.roomInstanceId, nowSeconds() + 800) + await backdateInstance(abandoned.roomInstanceId) + await backdateInstance(occupied.roomInstanceId) + + const ctx = createExecutionContext() + await scheduled(createScheduledController(), env, ctx) + await waitOnExecutionContext(ctx) + + expect(await getRoomInstance(env.DB, abandoned.roomInstanceId)).toBeNull() + expect(await getRoomInstance(env.DB, occupied.roomInstanceId)).not.toBeNull() + }) + + test('the cron sweep spares a freshly created instance nobody has joined yet', async () => { + // The instance and its creator's presence are written by the same request but not + // atomically — a sweep landing in between must not delete the instance the player + // is being handed. `createdAt` is left alone, so it's inside the grace window. + const fresh = await createRoomInstance(env.DB, { + ownerAccountId: 832, + roomId: 2, + photonRoomId: 'fresh-instance', + maxCapacity: 12, + }) + + const ctx = createExecutionContext() + await scheduled(createScheduledController(), env, ctx) + await waitOnExecutionContext(ctx) + + expect(await getRoomInstance(env.DB, fresh.roomInstanceId)).not.toBeNull() + }) + + test('the cron sweep spares an empty dorm instance', async () => { + // A dorm is backed by one persistent instance so its Photon room id survives + // re-entry — it sits empty whenever the owner is anywhere else. + const headers = await bearer('833') + const dorm = (await ( + await exports.default.fetch(`${ORIGIN}/matchmake/dorm`, { method: 'POST', headers }) + ).json()) as { roomInstance: { roomInstanceId: number } } + const dormInstanceId = dorm.roomInstance.roomInstanceId + await expirePresence(833) + await backdateInstance(dormInstanceId) + + const ctx = createExecutionContext() + await scheduled(createScheduledController(), env, ctx) + await waitOnExecutionContext(ctx) + + expect(await getRoomInstance(env.DB, dormInstanceId)).not.toBeNull() + }) + test('player/login and exclusivelogin preserve presence', async () => { const headers = await bearer('9') await exports.default.fetch(`${ORIGIN}/matchmake/dorm`, { method: 'POST', headers }) diff --git a/apps/match/wrangler.jsonc b/apps/match/wrangler.jsonc index 1e7f000..cde9b03 100644 --- a/apps/match/wrangler.jsonc +++ b/apps/match/wrangler.jsonc @@ -16,9 +16,10 @@ } ], // Presence sweep. Rows expire on their own TTL (15m) and reads already ignore - // expired ones, so this is housekeeping: it purges them and recomputes the - // fullness of the instances the departed players were in (a crashed player never - // matchmakes out, so nothing else notices they left). Every 5 minutes. + // expired ones, so this is housekeeping: it purges them, deletes the room + // instances left with nobody in them, and recomputes the fullness of the + // instances the departed players were in (a crashed player never matchmakes out, + // so nothing else notices they left). Every 5 minutes. "triggers": { "crons": ["*/5 * * * *"] }, diff --git a/packages/domain/src/room-instance-db.ts b/packages/domain/src/room-instance-db.ts index da087c4..6a27876 100644 --- a/packages/domain/src/room-instance-db.ts +++ b/packages/domain/src/room-instance-db.ts @@ -12,6 +12,7 @@ * the client DTO (`toDto`). */ +import { RoomInstanceType } from './enums' import { countPlayersInInstance, getPlayerIdsByRoomInstance } from './presence-db' /** Schema DDL (mirror of migrations/0004_room_instance.sql). */ @@ -282,6 +283,56 @@ export async function refreshInstanceFullness( return isFull } +/** + * How long (s) a room instance is left alone after it's created, even with nobody in + * it. Every path that creates an instance writes the creator's presence in the same + * request, so an empty instance is normally already abandoned — but the two writes + * aren't atomic, and a cron firing in between would delete the instance the player is + * being handed. One cron interval of slack closes that window. + */ +export const EMPTY_INSTANCE_GRACE_SECONDS = 300 + +/** + * Delete room instances with no presence rows pointing at them — the sessions left + * behind when every player quit or timed out. Nothing reuses them (matchmaking would + * happily hand a joiner an instance whose Photon room has long since emptied), so + * they're pure accumulation: one row per room visit, forever. + * + * Emptiness is a plain "are there any rows" test — expiry is not consulted, because + * {@link deleteExpiredPresence} is what retires a lapsed row and must have run first. + * Run out of that order and a crashed player's stale row keeps their instance alive + * until the following sweep. + * + * Instances younger than `graceSeconds` are skipped (see + * {@link EMPTY_INSTANCE_GRACE_SECONDS}), as are dorms: a dorm is backed by one + * persistent instance so its Photon room id survives re-entry, and it sits empty + * whenever the owner is elsewhere. + * + * Returns the ids deleted. + */ +export async function deleteEmptyRoomInstances( + db: D1Database, + graceSeconds = EMPTY_INSTANCE_GRACE_SECONDS, + now = Date.now() +): Promise { + // `createdAt` is an ISO-8601 UTC timestamp, which sorts lexicographically in the + // same order it sorts chronologically — so a string compare is a time compare. + const createdBefore = new Date(now - graceSeconds * 1000).toISOString() + const { results } = await db + .prepare( + `DELETE FROM room_instance + WHERE created_at < ?1 + AND room_instance_type != ?2 + AND NOT EXISTS ( + SELECT 1 FROM presence WHERE presence.room_instance_id = room_instance.id + ) + RETURNING json_extract(data, '$.roomInstanceId') AS id` + ) + .bind(createdBefore, RoomInstanceType.Dormroom) + .all<{ id: number }>() + return results.map((r) => r.id) +} + /** * The oldest joinable public instance of a room (not private, not full, joins * enabled, not already in progress), or null when there's none to join. Used by