mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
fix(sync): pre-landing review fixes — monotonic clock seed, scoped CHECK guard
Adversarial (codex) review of the implementation diff caught three: - P1 (correctness): the fresh-schema setval was not monotonic. initSchema replays the schema blob, and the unconditional setval(MAX(generation)) could move page_generation_clock_seq.last_value BACKWARD on an already-upgraded brain, letting a stored query_cache bookmark serve stale rows. Seed via GREATEST over the sequence's OWN last_value (+ old table value + MAX(generation)) in all 3 fresh schemas and migration v118, so a replay is idempotent — mirrors the old table's ON CONFLICT DO NOTHING. Pinned by a new monotonic regression test. - P2: v119's CHECK-exists guard keyed on conname only (not globally unique). Scope it to conrelid = 'op_checkpoints'::regclass. - P3: in-progress note ran into the prior sentence in fail/warn doctor messages; separate it with '. '. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
3002e37bf6
commit
008577a778
@@ -3531,7 +3531,7 @@ export async function checkSyncFreshness(
|
||||
const details = { unchanged_count, synced_recently_count, stale_count };
|
||||
// BUG 4: append in-progress context when any source is actively syncing.
|
||||
// Empty otherwise, so steady-state messages are byte-for-byte unchanged.
|
||||
const inProgressNote = inProgress.length ? ` ${inProgress.join('; ')}.` : '';
|
||||
const inProgressNote = inProgress.length ? `. ${inProgress.join('; ')}` : '';
|
||||
|
||||
if (hasFailures) {
|
||||
return {
|
||||
|
||||
+4
-1
@@ -5306,6 +5306,7 @@ export const MIGRATIONS: Migration[] = [
|
||||
|
||||
SELECT setval('page_generation_clock_seq', GREATEST(
|
||||
1,
|
||||
COALESCE((SELECT last_value FROM page_generation_clock_seq), 0),
|
||||
COALESCE((SELECT value FROM page_generation_clock WHERE id = 1), 0),
|
||||
COALESCE((SELECT MAX(generation) FROM pages), 0)
|
||||
));
|
||||
@@ -5355,7 +5356,9 @@ export const MIGRATIONS: Migration[] = [
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'op_checkpoints_completed_keys_array'
|
||||
SELECT 1 FROM pg_constraint
|
||||
WHERE conname = 'op_checkpoints_completed_keys_array'
|
||||
AND conrelid = 'op_checkpoints'::regclass
|
||||
) THEN
|
||||
ALTER TABLE op_checkpoints
|
||||
ADD CONSTRAINT op_checkpoints_completed_keys_array
|
||||
|
||||
@@ -169,8 +169,15 @@ INSERT INTO page_generation_clock (id, value)
|
||||
-- the first write strictly exceeds the seed; floor 1 (sequence MINVALUE).
|
||||
-- Layer-1 reads last_value. Table + trigger names retained.
|
||||
CREATE SEQUENCE IF NOT EXISTS page_generation_clock_seq;
|
||||
SELECT setval('page_generation_clock_seq',
|
||||
GREATEST(1, COALESCE((SELECT MAX(generation) FROM pages), 0)));
|
||||
-- Monotonic seed: GREATEST over the sequence's OWN last_value too, so replaying
|
||||
-- this blob on an already-upgraded brain (initSchema is re-runnable) can never
|
||||
-- move last_value BACKWARD below a stored query_cache bookmark.
|
||||
SELECT setval('page_generation_clock_seq', GREATEST(
|
||||
1,
|
||||
COALESCE((SELECT last_value FROM page_generation_clock_seq), 0),
|
||||
COALESCE((SELECT value FROM page_generation_clock WHERE id = 1), 0),
|
||||
COALESCE((SELECT MAX(generation) FROM pages), 0)
|
||||
));
|
||||
|
||||
CREATE OR REPLACE FUNCTION bump_page_generation_clock_fn() RETURNS trigger AS $func$
|
||||
BEGIN
|
||||
|
||||
@@ -233,8 +233,16 @@ INSERT INTO page_generation_clock (id, value)
|
||||
-- first write strictly exceeds the seed; floor 1 (sequence MINVALUE). Layer-1
|
||||
-- reads last_value. The table + trigger names are retained.
|
||||
CREATE SEQUENCE IF NOT EXISTS page_generation_clock_seq;
|
||||
SELECT setval('page_generation_clock_seq',
|
||||
GREATEST(1, COALESCE((SELECT MAX(generation) FROM pages), 0)));
|
||||
-- Monotonic seed: GREATEST over the sequence's OWN last_value too, so replaying
|
||||
-- this blob on an already-upgraded brain (initSchema is re-runnable) can never
|
||||
-- move last_value BACKWARD below a stored query_cache bookmark (which would let
|
||||
-- Layer 1 serve stale rows). Mirrors the old table's ON CONFLICT DO NOTHING.
|
||||
SELECT setval('page_generation_clock_seq', GREATEST(
|
||||
1,
|
||||
COALESCE((SELECT last_value FROM page_generation_clock_seq), 0),
|
||||
COALESCE((SELECT value FROM page_generation_clock WHERE id = 1), 0),
|
||||
COALESCE((SELECT MAX(generation) FROM pages), 0)
|
||||
));
|
||||
|
||||
CREATE OR REPLACE FUNCTION bump_page_generation_clock_fn() RETURNS trigger AS \$func\$
|
||||
BEGIN
|
||||
|
||||
+10
-2
@@ -229,8 +229,16 @@ INSERT INTO page_generation_clock (id, value)
|
||||
-- first write strictly exceeds the seed; floor 1 (sequence MINVALUE). Layer-1
|
||||
-- reads last_value. The table + trigger names are retained.
|
||||
CREATE SEQUENCE IF NOT EXISTS page_generation_clock_seq;
|
||||
SELECT setval('page_generation_clock_seq',
|
||||
GREATEST(1, COALESCE((SELECT MAX(generation) FROM pages), 0)));
|
||||
-- Monotonic seed: GREATEST over the sequence's OWN last_value too, so replaying
|
||||
-- this blob on an already-upgraded brain (initSchema is re-runnable) can never
|
||||
-- move last_value BACKWARD below a stored query_cache bookmark (which would let
|
||||
-- Layer 1 serve stale rows). Mirrors the old table's ON CONFLICT DO NOTHING.
|
||||
SELECT setval('page_generation_clock_seq', GREATEST(
|
||||
1,
|
||||
COALESCE((SELECT last_value FROM page_generation_clock_seq), 0),
|
||||
COALESCE((SELECT value FROM page_generation_clock WHERE id = 1), 0),
|
||||
COALESCE((SELECT MAX(generation) FROM pages), 0)
|
||||
));
|
||||
|
||||
CREATE OR REPLACE FUNCTION bump_page_generation_clock_fn() RETURNS trigger AS $func$
|
||||
BEGIN
|
||||
|
||||
@@ -295,4 +295,21 @@ describe('v0.42.x sequence-backed clock (BUG 1: contention removal)', () => {
|
||||
expect(Number(n3[0].v)).toBe(101);
|
||||
await engine.executeRaw(`DROP SEQUENCE test_probe_seq`);
|
||||
});
|
||||
|
||||
test('re-seeding is monotonic: the GREATEST guard never moves last_value backward', async () => {
|
||||
// Regression for the codex P1: initSchema replays the schema blob, whose
|
||||
// setval must NOT reset the clock below its current value — a backward move
|
||||
// would let a stored query_cache bookmark serve stale rows. Push the
|
||||
// sequence high, then run the EXACT monotonic seed the blob + v118 use.
|
||||
await engine.executeRaw(`SELECT setval('page_generation_clock_seq', 999999)`);
|
||||
await engine.executeRaw(
|
||||
`SELECT setval('page_generation_clock_seq', GREATEST(
|
||||
1,
|
||||
COALESCE((SELECT last_value FROM page_generation_clock_seq), 0),
|
||||
COALESCE((SELECT value FROM page_generation_clock WHERE id = 1), 0),
|
||||
COALESCE((SELECT MAX(generation) FROM pages), 0)
|
||||
))`,
|
||||
);
|
||||
expect(await clockValue()).toBeGreaterThanOrEqual(999999);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user