mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-10 22:01:40 +00:00
fix: read a team back from the primary after writing it (PUT-2033)
create() inserted on the primary then read the row back through the replica, so a replica that had not caught up answered the same as a team that was not there and the call threw. Nothing rolled the insert back, and the owner membership was never written, leaving a team its owner could not see but which still counted against their cap. Read back through pread in create() and update(), seed the row cache with the result, and delete the row if the read-back still finds nothing. #cached no longer stores a null, which would otherwise hold a lagging replica's answer for the full TTL.
This commit is contained in:
1 parent
359d68e51f
commit
8a53d14540
2 files changed
+73
-8
No files matched your search
@@ -104,6 +104,50 @@ describe('TeamStore', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('creates a team even when replica reads cannot see it yet', async () => {
|
||||
const db = server.clients.db as unknown as {
|
||||
read: (q: string, p?: unknown[]) => Promise<unknown[]>;
|
||||
pread: (q: string, p?: unknown[]) => Promise<unknown[]>;
|
||||
};
|
||||
const [realRead, realPread] = [db.read.bind(db), db.pread.bind(db)];
|
||||
// sqlite's pread delegates to read, so the primary is pinned separately.
|
||||
db.pread = realRead;
|
||||
db.read = async (q: string, p?: unknown[]) =>
|
||||
/FROM `group`/u.test(q) ? [] : realRead(q, p);
|
||||
|
||||
try {
|
||||
const created = await store.create({
|
||||
ownerUserId: owner.id,
|
||||
name: 'Lagging Replica',
|
||||
});
|
||||
expect(created).toMatchObject({ name: 'Lagging Replica' });
|
||||
} finally {
|
||||
[db.read, db.pread] = [realRead, realPread];
|
||||
}
|
||||
});
|
||||
|
||||
it('leaves no team behind when the row cannot be read back at all', async () => {
|
||||
const db = server.clients.db as unknown as {
|
||||
read: (q: string, p?: unknown[]) => Promise<unknown[]>;
|
||||
pread: (q: string, p?: unknown[]) => Promise<unknown[]>;
|
||||
};
|
||||
const [realRead, realPread] = [db.read.bind(db), db.pread.bind(db)];
|
||||
const blind = async (q: string, p?: unknown[]) =>
|
||||
/FROM `group`/u.test(q) ? [] : realRead(q, p);
|
||||
[db.read, db.pread] = [blind, blind];
|
||||
|
||||
const loner = await makeUser();
|
||||
try {
|
||||
await expect(
|
||||
store.create({ ownerUserId: loner.id, name: 'Doomed' }),
|
||||
).rejects.toThrow(/disappeared/u);
|
||||
} finally {
|
||||
[db.read, db.pread] = [realRead, realPread];
|
||||
}
|
||||
|
||||
expect(await store.countOwned(loner.id)).toBe(0);
|
||||
});
|
||||
|
||||
it('creates a team without a handle', async () => {
|
||||
const created = await store.create({
|
||||
ownerUserId: owner.id,
|
||||
|
||||
@@ -148,10 +148,7 @@ const RESERVED_HANDLES = new Set([
|
||||
]);
|
||||
|
||||
export type HandleRejection =
|
||||
| 'too_short'
|
||||
| 'too_long'
|
||||
| 'malformed'
|
||||
| 'reserved';
|
||||
'too_short' | 'too_long' | 'malformed' | 'reserved';
|
||||
|
||||
/** Trimmed and capped, so the same name is accepted on every engine. */
|
||||
export const normalizeTeamName = (name: string): string => {
|
||||
@@ -215,6 +212,15 @@ export class TeamStore extends PuterStore {
|
||||
return (rows[0] as unknown as TeamRow) ?? null;
|
||||
}
|
||||
|
||||
/** For reading back a row this request just wrote; the replica may lag. */
|
||||
async #readByUidFromPrimary(uid: string): Promise<TeamRow | null> {
|
||||
const rows = await this.clients.db.pread(
|
||||
`SELECT * FROM \`group\` WHERE \`uid\` = ? AND ${this.#live()}`,
|
||||
[uid, TEAM_KIND],
|
||||
);
|
||||
return (rows[0] as unknown as TeamRow) ?? null;
|
||||
}
|
||||
|
||||
/** Includes soft-deleted rows, so an audit survives its team. */
|
||||
async getByUidIncludingDeleted(uid: string): Promise<TeamRow | null> {
|
||||
const rows = await this.clients.db.read(
|
||||
@@ -271,9 +277,16 @@ export class TeamStore extends PuterStore {
|
||||
[uid, input.ownerUserId, TEAM_KIND, name, handle, '{}', '{}'],
|
||||
);
|
||||
|
||||
const created = await this.getByUid(uid);
|
||||
if (!created)
|
||||
const created = await this.#readByUidFromPrimary(uid);
|
||||
if (!created) {
|
||||
// Unreachable to the caller, but still counts against their cap.
|
||||
await this.clients.db.write(
|
||||
'DELETE FROM `group` WHERE `uid` = ? AND `kind` = ?',
|
||||
[uid, TEAM_KIND],
|
||||
);
|
||||
throw new Error('team disappeared immediately after insert');
|
||||
}
|
||||
await this.#cachePut(`team:row:${uid}`, created);
|
||||
return created;
|
||||
}
|
||||
|
||||
@@ -314,7 +327,9 @@ export class TeamStore extends PuterStore {
|
||||
[...params, uid, TEAM_KIND],
|
||||
);
|
||||
await this.#bustRow(uid);
|
||||
return this.getByUid(uid);
|
||||
const updated = await this.#readByUidFromPrimary(uid);
|
||||
if (updated) await this.#cachePut(`team:row:${uid}`, updated);
|
||||
return updated;
|
||||
}
|
||||
/** Releases the handle, since nothing addresses by it; keeps `name`. */
|
||||
async softDelete(uid: string): Promise<boolean> {
|
||||
@@ -516,6 +531,13 @@ export class TeamStore extends PuterStore {
|
||||
return read();
|
||||
}
|
||||
const value = await read();
|
||||
// A miss may just be a lagging replica; caching it holds for the TTL.
|
||||
if (value !== null && value !== undefined)
|
||||
await this.#cachePut(key, value);
|
||||
return value;
|
||||
}
|
||||
|
||||
async #cachePut(key: string, value: unknown): Promise<void> {
|
||||
try {
|
||||
await this.clients.redis.set(
|
||||
key,
|
||||
@@ -526,7 +548,6 @@ export class TeamStore extends PuterStore {
|
||||
} catch {
|
||||
/* a cache that cannot be written is still correct */
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
async #bust(...keys: string[]): Promise<void> {
|
||||
|
||||
Reference in new issue
Block a user