Skip to content

Commit 30071ce

Browse files
grgbkrgrgbkr
authored
feat: just leave the old connection entries (rocicorp#520)
Changes connection entries prefix to `conn/` from `connection/`. Also renames the connections by room index entries prefix to `conns_by_room/` from `connections_by_room/` for consistency. This prefix was never released to customers. Note it is safe to lose connection entries on a new code deploy because the code deploy causes all connections to close. The motivation for this is to solve an OOM problem being hit by our customer Monday. They have so many connection entries that the revalidation cron process fails with OOM. This [change](rocicorp@0a171b1) updated the revalidation process to scale to more connection entries than can fit in memory. However, it required a new index. To get this index in sync with the connection entries we needed a migration step that either added index entries for all existing connecting entries, or deleted all connection entries (empty entries, empty index is in sync). We first [implemented](rocicorp@0a171b1) a migration that built the index for existing entries. We then [switched](rocicorp@85a30e0) to a migration that deleted all the connection entries to avoid an expensive initial revalidation. However, manual testing showed that even this migration that deleted all existing connections could take several minutes to complete on a store with enough entries to cause OOM (at least 500k entries), during which time the system is effectively down. We don't know how many entires Monday has built up, and haven't found anyway to query how much storage a DO is using. To reduce risk of taking Monday down for an extended period of time, we have decided to just leave the old connection entries and move to a new index. This approach does not require a migration step, so the v0->v1 migration is removed. However the handling of down migrates is left in place as it is still useful.
1 parent 9bc0e03 commit 30071ce

File tree

3 files changed

+63
-258
lines changed

3 files changed

+63
-258
lines changed

‎packages/reflect-server/src/server/auth-do.test.ts‎

Lines changed: 53 additions & 209 deletions
Original file line numberDiff line numberDiff line change
@@ -103,20 +103,20 @@ async function storeTestConnectionState() {
103103
}
104104

105105
const expectedConnectionKeysForTestConnectionState = [
106-
'connection/testUserID1/testRoomID1/testClientID1/',
107-
'connection/testUserID1/testRoomID1/testClientID2/',
108-
'connection/testUserID1/testRoomID2/testClientID3/',
109-
'connection/testUserID2/testRoomID1/testClientID4/',
110-
'connection/testUserID2/testRoomID3/testClientID5/',
111-
'connection/testUserID3/testRoomID3/testClientID6/',
106+
'conn/testUserID1/testRoomID1/testClientID1/',
107+
'conn/testUserID1/testRoomID1/testClientID2/',
108+
'conn/testUserID1/testRoomID2/testClientID3/',
109+
'conn/testUserID2/testRoomID1/testClientID4/',
110+
'conn/testUserID2/testRoomID3/testClientID5/',
111+
'conn/testUserID3/testRoomID3/testClientID6/',
112112
];
113113
const expectedConnectionsByRoomKeysForTestConnectionState = [
114-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID1/',
115-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID2/',
116-
'connections_by_room/testRoomID1/connection/testUserID2/testRoomID1/testClientID4/',
117-
'connections_by_room/testRoomID2/connection/testUserID1/testRoomID2/testClientID3/',
118-
'connections_by_room/testRoomID3/connection/testUserID2/testRoomID3/testClientID5/',
119-
'connections_by_room/testRoomID3/connection/testUserID3/testRoomID3/testClientID6/',
114+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID1/',
115+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID2/',
116+
'conns_by_room/testRoomID1/conn/testUserID2/testRoomID1/testClientID4/',
117+
'conns_by_room/testRoomID2/conn/testUserID1/testRoomID2/testClientID3/',
118+
'conns_by_room/testRoomID3/conn/testUserID2/testRoomID3/testClientID5/',
119+
'conns_by_room/testRoomID3/conn/testUserID3/testRoomID3/testClientID6/',
120120
];
121121

122122
function createCreateRoomTestFixture() {
@@ -1235,18 +1235,18 @@ test('connect percent escapes components of the connection key', async () => {
12351235
expect(response.headers.get('Sec-WebSocket-Protocol')).toEqual(
12361236
encodedTestAuth,
12371237
);
1238-
expect(await storage.list({prefix: 'connection/'})).toEqual(
1238+
expect(await storage.list({prefix: 'conn/'})).toEqual(
12391239
new Map([
12401240
[
1241-
'connection/%2FtestUserID%2F%3F/testRoomID/%2FtestClientID%2F/',
1241+
'conn/%2FtestUserID%2F%3F/testRoomID/%2FtestClientID%2F/',
12421242
{connectTimestamp: testTime},
12431243
],
12441244
]),
12451245
);
1246-
expect(await storage.list({prefix: 'connections_by_room/'})).toEqual(
1246+
expect(await storage.list({prefix: 'conns_by_room/'})).toEqual(
12471247
new Map([
12481248
[
1249-
'connections_by_room/testRoomID/connection/%2FtestUserID%2F%3F/testRoomID/%2FtestClientID%2F/',
1249+
'conns_by_room/testRoomID/conn/%2FtestUserID%2F%3F/testRoomID/%2FtestClientID%2F/',
12501250
{},
12511251
],
12521252
]),
@@ -1764,29 +1764,27 @@ async function connectAndTestThatRoomGotCreated(
17641764

17651765
if (jurisdiction !== 'invalid') {
17661766
expect(response.webSocket).toBe(mocket);
1767-
expect((await storage.list({prefix: 'connection/'})).size).toEqual(1);
1767+
expect((await storage.list({prefix: 'conn/'})).size).toEqual(1);
17681768
const connectionRecord = (await storage.get(
1769-
`connection/${testUserID}/testRoomID1/testClientID1/`,
1769+
`conn/${testUserID}/testRoomID1/testClientID1/`,
17701770
)) as Record<string, unknown> | undefined;
17711771
assert(connectionRecord);
17721772
expect(connectionRecord.connectTimestamp).toEqual(testTime);
1773-
expect(await storage.list({prefix: 'connections_by_room/'})).toEqual(
1773+
expect(await storage.list({prefix: 'conns_by_room/'})).toEqual(
17741774
new Map([
17751775
[
1776-
`connections_by_room/testRoomID1/connection/${testUserID}/testRoomID1/testClientID1/`,
1776+
`conns_by_room/testRoomID1/conn/${testUserID}/testRoomID1/testClientID1/`,
17771777
{},
17781778
],
17791779
]),
17801780
);
17811781
} else {
1782-
expect((await storage.list({prefix: 'connection/'})).size).toEqual(0);
1782+
expect((await storage.list({prefix: 'conn/'})).size).toEqual(0);
17831783
const connectionRecord = await storage.get(
1784-
`connection/${testUserID}/testRoomID1/testClientID1/`,
1784+
`conn/${testUserID}/testRoomID1/testClientID1/`,
17851785
);
17861786
expect(connectionRecord).toBeUndefined();
1787-
expect((await storage.list({prefix: 'connections_by_room/'})).size).toEqual(
1788-
0,
1789-
);
1787+
expect((await storage.list({prefix: 'conns_by_room/'})).size).toEqual(0);
17901788
}
17911789
}
17921790

@@ -2076,17 +2074,15 @@ test('revalidateConnections', async () => {
20762074
expect(roomDORequestCountsByRoomID.get('testRoomID2')).toEqual(1);
20772075
expect(roomDORequestCountsByRoomID.get('testRoomID3')).toEqual(1);
20782076

2079-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([
2080-
'connection/testUserID1/testRoomID1/testClientID1/',
2081-
'connection/testUserID1/testRoomID2/testClientID3/',
2082-
'connection/testUserID2/testRoomID1/testClientID4/',
2077+
expect([...(await storage.list({prefix: 'conn/'})).keys()]).toEqual([
2078+
'conn/testUserID1/testRoomID1/testClientID1/',
2079+
'conn/testUserID1/testRoomID2/testClientID3/',
2080+
'conn/testUserID2/testRoomID1/testClientID4/',
20832081
]);
2084-
expect([
2085-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2086-
]).toEqual([
2087-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID1/',
2088-
'connections_by_room/testRoomID1/connection/testUserID2/testRoomID1/testClientID4/',
2089-
'connections_by_room/testRoomID2/connection/testUserID1/testRoomID2/testClientID3/',
2082+
expect([...(await storage.list({prefix: 'conns_by_room/'})).keys()]).toEqual([
2083+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID1/',
2084+
'conns_by_room/testRoomID1/conn/testUserID2/testRoomID1/testClientID4/',
2085+
'conns_by_room/testRoomID2/conn/testUserID1/testRoomID2/testClientID3/',
20902086
]);
20912087
});
20922088

@@ -2104,19 +2100,17 @@ test('revalidateConnections continues if one storage delete throws an error', as
21042100
expect(roomDORequestCountsByRoomID.get('testRoomID2')).toEqual(1);
21052101
expect(roomDORequestCountsByRoomID.get('testRoomID3')).toEqual(1);
21062102

2107-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([
2108-
'connection/testUserID1/testRoomID1/testClientID1/',
2109-
'connection/testUserID1/testRoomID1/testClientID2/',
2110-
'connection/testUserID1/testRoomID2/testClientID3/',
2111-
'connection/testUserID2/testRoomID1/testClientID4/',
2103+
expect([...(await storage.list({prefix: 'conn/'})).keys()]).toEqual([
2104+
'conn/testUserID1/testRoomID1/testClientID1/',
2105+
'conn/testUserID1/testRoomID1/testClientID2/',
2106+
'conn/testUserID1/testRoomID2/testClientID3/',
2107+
'conn/testUserID2/testRoomID1/testClientID4/',
21122108
]);
2113-
expect([
2114-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2115-
]).toEqual([
2116-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID1/',
2117-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID2/',
2118-
'connections_by_room/testRoomID1/connection/testUserID2/testRoomID1/testClientID4/',
2119-
'connections_by_room/testRoomID2/connection/testUserID1/testRoomID2/testClientID3/',
2109+
expect([...(await storage.list({prefix: 'conns_by_room/'})).keys()]).toEqual([
2110+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID1/',
2111+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID2/',
2112+
'conns_by_room/testRoomID1/conn/testUserID2/testRoomID1/testClientID4/',
2113+
'conns_by_room/testRoomID2/conn/testUserID1/testRoomID2/testClientID3/',
21202114
]);
21212115
});
21222116

@@ -2132,170 +2126,20 @@ test('revalidateConnections continues if one roomDO returns an error', async ()
21322126
expect(roomDORequestCountsByRoomID.get('testRoomID2')).toEqual(1);
21332127
expect(roomDORequestCountsByRoomID.get('testRoomID3')).toEqual(1);
21342128

2135-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([
2136-
'connection/testUserID1/testRoomID1/testClientID1/',
2137-
'connection/testUserID1/testRoomID1/testClientID2/',
2138-
'connection/testUserID1/testRoomID2/testClientID3/',
2139-
'connection/testUserID2/testRoomID1/testClientID4/',
2129+
expect([...(await storage.list({prefix: 'conn/'})).keys()]).toEqual([
2130+
'conn/testUserID1/testRoomID1/testClientID1/',
2131+
'conn/testUserID1/testRoomID1/testClientID2/',
2132+
'conn/testUserID1/testRoomID2/testClientID3/',
2133+
'conn/testUserID2/testRoomID1/testClientID4/',
21402134
]);
2141-
expect([
2142-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2143-
]).toEqual([
2144-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID1/',
2145-
'connections_by_room/testRoomID1/connection/testUserID1/testRoomID1/testClientID2/',
2146-
'connections_by_room/testRoomID1/connection/testUserID2/testRoomID1/testClientID4/',
2147-
'connections_by_room/testRoomID2/connection/testUserID1/testRoomID2/testClientID3/',
2135+
expect([...(await storage.list({prefix: 'conns_by_room/'})).keys()]).toEqual([
2136+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID1/',
2137+
'conns_by_room/testRoomID1/conn/testUserID1/testRoomID1/testClientID2/',
2138+
'conns_by_room/testRoomID1/conn/testUserID2/testRoomID1/testClientID4/',
2139+
'conns_by_room/testRoomID2/conn/testUserID1/testRoomID2/testClientID3/',
21482140
]);
21492141
});
21502142

2151-
test('test migration from schema 0 to schema 1, basic', async () => {
2152-
const {testRoomDO, state} = await createCreateRoomTestFixture();
2153-
await storage.deleteAll();
2154-
2155-
await storage.put('connection/testUserID1/testRoomID1/testClientID1/', {
2156-
connectTimestamp: 1000,
2157-
});
2158-
await storage.put('connection/testUserID1/testRoomID1/testClientID2/', {
2159-
connectTimestamp: 1000,
2160-
});
2161-
await storage.put('connection/testUserID2/testRoomID1/testClientID3/', {
2162-
connectTimestamp: 1000,
2163-
});
2164-
await storage.put('connection/testUserID1/testRoomID2/testClientID4/', {
2165-
connectTimestamp: 1000,
2166-
});
2167-
await storage.put('connection/testUserID2/testRoomID3/testClientID5/', {
2168-
connectTimestamp: 1000,
2169-
});
2170-
await storage.put(
2171-
'connection/%2FtestUserID%2F%3F/%2FtestRoomID%2F%3F/%2FtestClientID%2F/',
2172-
{
2173-
connectTimestamp: 1000,
2174-
},
2175-
);
2176-
2177-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual(undefined);
2178-
expect([
2179-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2180-
]).toEqual([]);
2181-
2182-
const ensureStorageSchemaMigratedCalls: Promise<void>[] = [];
2183-
new BaseAuthDO(
2184-
{
2185-
roomDO: testRoomDO,
2186-
state,
2187-
authHandler: () => Promise.reject('should not be called'),
2188-
authApiKey: TEST_AUTH_API_KEY,
2189-
logSink: new TestLogSink(),
2190-
logLevel: 'debug',
2191-
},
2192-
p => {
2193-
ensureStorageSchemaMigratedCalls.push(p);
2194-
return p;
2195-
},
2196-
);
2197-
expect(ensureStorageSchemaMigratedCalls.length).toEqual(1);
2198-
await ensureStorageSchemaMigratedCalls[0];
2199-
2200-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual({
2201-
version: STORAGE_SCHEMA_VERSION,
2202-
maxVersion: STORAGE_SCHEMA_VERSION,
2203-
minSafeRollbackVersion: STORAGE_SCHEMA_MIN_SAFE_ROLLBACK_VERSION,
2204-
});
2205-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([]);
2206-
expect([
2207-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2208-
]).toEqual([]);
2209-
});
2210-
2211-
test('test migration from schema 0 to schema 1, existing connections by room index entries', async () => {
2212-
await storage.deleteAll();
2213-
await storeTestConnectionState();
2214-
2215-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual(undefined);
2216-
expect(
2217-
[...(await storage.list({prefix: 'connection/'})).keys()].length,
2218-
).toBeGreaterThan(0);
2219-
expect(
2220-
[...(await storage.list({prefix: 'connections_by_room/'})).keys()].length,
2221-
).toBeGreaterThan(0);
2222-
2223-
const ensureStorageSchemaMigratedCalls: Promise<void>[] = [];
2224-
new BaseAuthDO(
2225-
{
2226-
roomDO: createRoomDOThatThrowsIfFetchIsCalled(),
2227-
state,
2228-
authHandler: () => Promise.reject('should not be called'),
2229-
authApiKey: TEST_AUTH_API_KEY,
2230-
logSink: new TestLogSink(),
2231-
logLevel: 'debug',
2232-
},
2233-
p => {
2234-
ensureStorageSchemaMigratedCalls.push(p);
2235-
return p;
2236-
},
2237-
);
2238-
expect(ensureStorageSchemaMigratedCalls.length).toEqual(1);
2239-
await ensureStorageSchemaMigratedCalls[0];
2240-
2241-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual({
2242-
version: STORAGE_SCHEMA_VERSION,
2243-
maxVersion: STORAGE_SCHEMA_VERSION,
2244-
minSafeRollbackVersion: STORAGE_SCHEMA_MIN_SAFE_ROLLBACK_VERSION,
2245-
});
2246-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([]);
2247-
expect([
2248-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2249-
]).toEqual([]);
2250-
});
2251-
2252-
// 3333 is chosen because it is >3 x the limit used to page through the
2253-
// connections and is not a multiple of the limit
2254-
test('test migration from schema 0 to schema 1, 3333 connections', async () => {
2255-
await storage.deleteAll();
2256-
2257-
for (let i = 0; i < 3333; i++) {
2258-
const connectionKeyString = `connection/testUserID${i % 10}/testRoomID${
2259-
i % 10
2260-
}/testClientID${i}/`;
2261-
await storage.put(connectionKeyString, {
2262-
connectTimestamp: 1000,
2263-
});
2264-
}
2265-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual(undefined);
2266-
expect([
2267-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2268-
]).toEqual([]);
2269-
2270-
const ensureStorageSchemaMigratedCalls: Promise<void>[] = [];
2271-
new BaseAuthDO(
2272-
{
2273-
roomDO: createRoomDOThatThrowsIfFetchIsCalled(),
2274-
state,
2275-
authHandler: () => Promise.reject('should not be called'),
2276-
authApiKey: TEST_AUTH_API_KEY,
2277-
logSink: new TestLogSink(),
2278-
logLevel: 'debug',
2279-
},
2280-
p => {
2281-
ensureStorageSchemaMigratedCalls.push(p);
2282-
return p;
2283-
},
2284-
);
2285-
expect(ensureStorageSchemaMigratedCalls.length).toEqual(1);
2286-
await ensureStorageSchemaMigratedCalls[0];
2287-
2288-
expect(await storage.get(STORAGE_SCHEMA_META_KEY)).toEqual({
2289-
version: STORAGE_SCHEMA_VERSION,
2290-
maxVersion: STORAGE_SCHEMA_VERSION,
2291-
minSafeRollbackVersion: STORAGE_SCHEMA_MIN_SAFE_ROLLBACK_VERSION,
2292-
});
2293-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual([]);
2294-
expect([
2295-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2296-
]).toEqual([]);
2297-
});
2298-
22992143
describe('test down migrate', () => {
23002144
const testWithMinSafeRollbackVersion = async (
23012145
version: number,
@@ -2356,11 +2200,11 @@ describe('test down migrate', () => {
23562200
minSafeRollbackVersion,
23572201
});
23582202
}
2359-
expect([...(await storage.list({prefix: 'connection/'})).keys()]).toEqual(
2203+
expect([...(await storage.list({prefix: 'conn/'})).keys()]).toEqual(
23602204
expectedConnectionKeysForTestConnectionState,
23612205
);
23622206
expect([
2363-
...(await storage.list({prefix: 'connections_by_room/'})).keys(),
2207+
...(await storage.list({prefix: 'conns_by_room/'})).keys(),
23642208
]).toEqual(expectedConnectionsByRoomKeysForTestConnectionState);
23652209
};
23662210

0 commit comments

Comments
 (0)