Repository navigation
fix(server): set hasResolvedPath before the startup load reads it (#184 part 2) - #186
Merged
Merged
Conversation
…gate (#184) Two cases, both starting with hasResolvedPath false at OpenDB because the ingestor adds the column afterwards: - the post-gate re-probe works: passes on master (waitForDBSchemaWith already calls detectSchema after AssertReady); - the post-gate probe fails: red on master. detectSchema gives up silently, the flag stays false, and RunStartupLoad indexes the observation without byPathHop. Adds DB.schemaProbeHook, a nil-in-production test seam that makes a detectSchema pass fail the way a failed PRAGMA table_info does. Relates to #184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
) main.go forced the flag only after <-store.FirstChunkReady(), so it could never help the first (newest) chunk. Until then the flag came from detectSchema, which returns silently when its PRAGMA fails. If both the OpenDB probe and the post-gate probe miss the column, the hot window is loaded without resolved_path: byNode only, no byPathHop credit for its relays until the next restart. waitForDBSchema now sets the flag as soon as dbschema.AssertReady has passed, since AssertReady requires observations.resolved_path. The late forceTrue in main.go is removed. Relates to #184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dborup
marked this pull request as ready for review
October 3, 2026 09:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #184 (part 2: the possible start-up race on
hasResolvedPath).Can the race happen?
Not in the form the issue describes. Issue #184 assumed: the
OpenDBprobe sees noresolved_pathcolumn,waitForDBSchemapasses, and the start-up load runs beforeforceTrue(). That case is already covered:dbschema.AssertReadyrequiresobservations.resolved_path(internal/dbschema/dbschema.go:316).main.goexits if the gate fails.waitForDBSchemaWithcallsdb.detectSchema()again (schema_wait.go). That re-probe sees the column the ingestor added and sets the flag beforeRunStartupLoadstarts.A narrower form can happen.
detectSchemagives up silently when itsPRAGMA table_info(observations)fails (db.go,if err != nil { return }). Suppose theOpenDBprobe missed the column and the post-gate re-probe then fails. The flag stays false whenRunStartupLoadstarts.The
forceTrue()inmain.gocannot help, because it ran only after<-store.FirstChunkReady(), so after the first (newest) chunk was already loaded. The 2 s healer does not bound the window either. With the flag false,LoadChunkedtakes the NULL branch: the chunk's relays go intobyNodeonly, with nobyPathHopcredit until the next restart.How likely is it? It needs a failed PRAGMA right after
AssertReadysucceeded on the same read-only handle. On a WAL database that is rare: a reader getsSQLITE_BUSYonly in edge cases such as WAL recovery. It is not reproduced on staging.Fix
waitForDBSchemasetshasResolvedPathFlagas soon asAssertReadyhas passed, sinceAssertReadyrequires the column. The lateforceTrue()inmain.gois removed. The flag is now true beforeRunStartupLoadstarts, whatever the re-probe does.DB.schemaProbeHookis a new test seam,nilin production and in the same style aschannelsRowsHook. It lets a test make adetectSchemapass fail.Test
TestStartupLoadIndexesResolvedPathAfterSchemaGate_184uses the e2e fixture. The column is dropped, so the probe at open sees the flag as false. The "ingestor" then runsdbschema.Applyand writes aresolved_pathfor one observation. The test then runswaitForDBSchema→ graph load →RunStartupLoad, inmain.go's order. It asserts the flag, and thatbyPathHop[<resolved pubkey>]holds the transmission.bedbe1f1)column added after the first probe(the issue's scenario)post-gate probe failsbyPathHop0 txThe DB is built without
OpenDB's healer goroutine, so the window is deterministic.Mutant, run in a copy of the tree: put
forceTrue()back afterRunStartupLoadinmain.goand remove it fromwaitForDBSchema. Result:post-gate probe failsis red on both assertions.Runs
cd cmd/server && go test ./...: ok. Master has 2,050 top-level tests and 624 subtests. This branch has 2,051 and 626, with 0 failures. 2 top-level tests are skipped on both.go test -race -count=10 -run 'TestStartupLoadIndexesResolvedPathAfterSchemaGate_184|TestWaitForDBSchema|TestWaitForServerSchema|TestWaitForSchema': ok.go vetandgofmt: clean.cmd/ingestor, tostore.go's ingest paths (fix(store): index live observations from the persisted resolved_path, as Load does (#158) #182 changes those) or to.github/(the 9 fork guards are unchanged).Performance
There is one atomic store at start-up. No hot path is touched.
Not in this PR (found while reading)
LoadChunked(chunked_load.go) andLoad(store.go) readhasResolvedPath()twice for each chunk. They read it once when building the SELECT and once for every row's scan targets. If the flag flips between the two reads, which the healer can do, the column count and the scan targets differ. The rows of that chunk are then dropped withscan error. This PR removes the flip forresolved_path. The same pattern exists forraw_hex,scope_nameandroute_mask. A fix would be one snapshot of the flags per chunk.AssertReadyguarantees could be set the same way:scope_name,last_seen,route_mask,default_scope,configured_scopeanddefault_scope_confirmed_at.Not verified
🤖 Generated with Claude Code