feat(syncing): maintained RBSR fingerprint index for fast blob propagation#748
feat(syncing): maintained RBSR fingerprint index for fast blob propagation#748juligasa wants to merge 25 commits into
Conversation
4483f55 to
15ab1a2
Compare
13defb0 to
9bc1beb
Compare
9bc1beb to
7a8062d
Compare
@juligasa this is actually an oversight I've been meaning to improve for a long time, and bumping the protocol version. Maybe we should take advantage of this change and do it? It should also help us with measurements a little bit, because we'll avoid new peers talking to old peers as it could show quite different performance characteristics. |
| -- since different protocol versions advertise different codec-canonical sets. | ||
| -- rbsr_item holds the resolved blob set for the scope, maintained incrementally | ||
| -- by the syncing oracle so reconciliation no longer rebuilds the set per round. | ||
| CREATE TABLE rbsr_scope ( |
There was a problem hiding this comment.
Does it mean that every document will produce multiple scopes by default? E.g. the regular one, the recursive one, the depth-one one, and maybe some else, e.g. combining different blob types?
And for each of these scopes there's going to be tons of rows in rbsr_items table?
Maybe we should quit having filters per scope, and for things that we need filters for we simply define different scopes. This should simplify the whole thing by flattening it into one dimensions, instead of having multiple.
c3e6a92 to
33495a2
Compare
6c16b85 to
10d9ec3
Compare
|
Looks like the indexing hook misses blobs indexed by unstash cascades: Line 1947 in 10d9ec3 indexBlob for previously stashed blobs during reindexStashedBlobs, but those blob IDs are never passed to idx.runIndexedHook. The only hook calls are in Put / PutMany for the top-level fresh IDs it seems: seed/backend/blob/index_blockstore.go Line 44 in 10d9ec3 seed/backend/blob/index_blockstore.go Line 104 in 10d9ec3 rbsr_item does not. Reconcile can then serve a stale-short maintained index.
|
Isn't it super wasteful to re-evaluate all the scopes every 5 minutes? I mean, it really doesn't make sense doing it if there's no new blobs for example. It seems like we are not really confident in that we are able to accurately detect which blobs affect which scopes, so we have to do this safety net thing every 5 minutes to make sure we don't mess it up. I don't mind if we run it for a while, but having it long-term doesn't seem right. And it doesn't seem right to run it when we can know for sure there's nothing that could have changed the world (e.g. no new blobs since the last sweep). |
3e50cd7 to
5bf7fad
Compare
43a0f38 to
6f1347d
Compare
|
I just realized something that made me think that we definitely should bump the protocol version, and change the signature of the ReconcileBlobs RPC in |
|
Here's a review finding from my agent (GPT 5.6 Sol). It looks valid, especially if we do something about my previously shared concerns about the need for the background shadow sweep I shared above.
|
…ation Persist each reconciliation scope's blob set (rbsr_scope/rbsr_item) and serve RBSR range fingerprints from an O(log N) monoid tree built from those rows, instead of rebuilding the whole sorted set via collectBlobs on every reconcile round. An incremental oracle keeps the set current as blobs are indexed, with a shadow-verify sweep as the safety net. Hot discovery cooldown dropped to 1s. - rbsr.NewTreeStore: augmented treap with RangeFingerprinter (O(log N) range fingerprints, byte-identical to the linear fold; allows post-seal insert). - oracle (affectedScopes): seed-and-forward membership decision — which materialized scopes a freshly indexed blob joins — reusing collectBlobs' forward closure rather than hand-written reverse queries. - rbsr_scope/rbsr_item schema + migration; reindex drops them (lazy re-mat). - loadStore serves from the persistent index, with a reindex-gated fallback to the legacy collectBlobs rebuild. - index hook (Index.SetIndexedHook) patches materialized scopes inside the indexing transaction. - shadow-verify sweep re-materializes scopes whose set drifts from a fresh collectBlobs (safety net for the oracle's deferred edges). - /0.9.3 codec-canonicalization primitives (canonicalCodecFor; scope keyed by protocol version); live protocol negotiation is deferred (see PR notes). - hot cooldown hardcoded to 1s.
qMaterializeReplace and qFreshScopeBlobs read from the rbsr_blobs TEMP table (created at runtime by collectBlobs), so they must not be registered as dqb.Str queries — TestDBQueries prepares every dqb query against a schema-only DB, which has no temp tables, and failed with 'no such table: rbsr_blobs'. Switch them to plain const strings, matching the existing temp-table query convention in discovery.go. Also drop the unused qTouchScope.
The daemon e2e hung (10m timeout) and several sync tests failed because serving reconciliation from the maintained index broke convergence/correctness under real load. Gate it behind SEED_RBSR_INDEX_SERVE (default off): loadStore serves from the legacy collectBlobs rebuild, leaving the maintained tables dormant (no scope materializes, so the index hook and shadow-verify sweep are no-ops). Production sync behavior is unchanged until the serve path is validated in a real environment. loadStore also now falls back to the legacy path on ANY error from the index path, so a problem in the maintained index can never break sync even when the flag is enabled. This should also clear the SQLITE_READONLY failures in the syncing unit tests, which were collateral: the hung daemon package starved file-DB WAL init for the syncing test binary running concurrently under 'go test ./...'.
This package's test suite opens many connection pools in parallel (oracleFixture is called by several tests; the canonicalization test builds three DBs). Under CI concurrency the file-backed WAL pools exhaust -shm/file descriptors, which surfaces as SQLITE_READONLY (can't init the shared-memory file) on the first write. Switch the new tests to MakeTestMemoryDB, which uses no WAL/-shm files. Verified the in-memory DB works with collectBlobs temp tables, materialize, and the oracle locally.
MakeTestDB/MakeTestMemoryDB open a NumCPU-sized connection pool; under this package's parallel suite that intermittently returns SQLITE_READONLY on the first write in CI (seen with both file-backed and shared-cache in-memory pools, while existing single-DB tests in the same package pass). A single-connection in-memory pool — the exact config TestDBQueries uses, which passes in CI — is stable. Add a newMemDB helper and use it for the oracle/index/canonical tests.
main split reader/writer pool connections: Pool.WithSave now acquires a read-only connection and Pool.WithTx the writer. The maintained-index write paths (loadStore materialize, shadow-verify sweep) and their test fixtures used WithSave for INSERT/UPDATE/DELETE, landing on a read-only connection -> SQLITE_READONLY (the CI failure). Switch all main-DB writes to WithTx; reads and TEMP-only bodies (loadRBSRStore, collectBlobs) stay on WithSave. Tests use the file-backed MakeTestDB to avoid in-memory shared-cache schema locking. Reproduced the failure locally after rebasing onto main, then verified the fix passes with -race.
…tally MaintainRBSRIndex only did a forward link-walk from resource-anchored seeds: it returned complete=false for Capability blobs and deferred the agent-capability delegation closure, late-arriving forward targets, and inbound Contact-by-subject entirely to the periodic shadow-verify sweep. Under real sync load that sweep can't keep pace, so the maintained set served well short of collectBlobs -- dropping the agent capabilities a site's members are granted (a recursive site scope served ~60% of its blobs, missing hundreds of capabilities). Maintain those edges incrementally in the index hook, mirroring collectBlobs: - runAgentCapClosure: the recursive `del IN (authors of in-scope blobs)` AGENT capability fixpoint, run against the persisted rbsr_item set for each scope whose membership changed in the batch (or whose member a fresh capability delegates to). - late-arrival reverse edge (scopesLinkingTo + forwardClosureDownloaded): a forward target -- a Change or media blob -- that arrived after the member whose closure should have pulled it in. - inbound Contact-by-subject: a fresh Contact added to its subject account's recursive scopes. oracle_incremental_test.go feeds blobs out of order (a Ref before its Change, two transitively-delegating capabilities, a contact) and asserts the maintained set equals collectBlobs with no re-materialization. Shadow-verify stays as the backstop for anything still deferred, but no longer carries the steady-state load: live, drift dropped from ~39 scopes per sweep to ~2.
… default Remove the SEED_RBSR_INDEX_SERVE gate (previously default off). loadStore now serves from the maintained index unconditionally -- still skipping it only while a reindex is in flight, and still falling back to the authoritative legacy collectBlobs rebuild on any error, so a problem in the index can never break sync. The convergence gap that justified the gate is fixed: the oracle now maintains the agent-capability delegation closure, late-arriving forward targets, and inbound Contact-by-subject incrementally, so the maintained set stays equal to collectBlobs under live load instead of serving short and leaning on the 5-minute shadow-verify sweep.
Addresses review feedback on the maintained RBSR fingerprint index:
- Flatten rbsr_scope identity to (iri, kind): collapse the recursive +
depth_one booleans and the blob_types filter column into a single kind
enum (exact/depth_one/recursive/dir_structure). The only real-world
type filter (the {Ref,Change} root-directory pass) becomes its own
kind; exotic filters fall back to the legacy non-indexed rebuild.
- Drop protocol_version from the scope identity. Canonicalization only
affects advertised codecs at serve time, not membership, so it stays a
serve-time argument to buildStoreFromScopes; a protocol bump rebuilds
the index. Schema + in-place migration rewritten to match.
- scopeKindFor/dkeyForKind isolate the schema<->logic mapping so the
oracle predicates (scopeCovers/scopeAllowsType) are untouched.
- rbsr: make sizeOf a method on *treeNode.
- scheduler: raise hot cooldown from 1s to 10s.
- runShadowVerify: use a Timer (fire at 0, reset after each cycle) instead of a Ticker, so the interval is "work then sleep" and slow sweeps don't stack. First sweep now runs at startup. - Unexport ShadowVerifySweep -> shadowVerifySweep (single in-package caller). - Remove the codec-canonicalization machinery (canonical.go + test): it was inert (the 0.9.3 negotiation that would activate it is deferred), and the codec-mismatch counter is empty even under heavy re-fetch load, so the dag-pb/raw divergence it guarded against isn't occurring. buildStoreFromScopes and loadStoreFromIndex drop the now-unused protocolVersion threading and serve the stored codec directly. Codec handling, if ever needed, belongs with the 0.9.3 protocol work.
buildStoreFromScopes and structuralFactsForBatch bound their scope-id array as a []byte, which the driver binds as a BLOB. SQLite >=3.45 interprets a BLOB argument to json_each as JSONB, so the text JSON was rejected as "malformed JSON" — failing every maintained-index serve and silently falling back to the legacy rebuild on every reconcile. int64SliceJSON now returns a string (TEXT bind), and buildStoreFromScopes reuses it instead of duplicating the builder. Note: the test-linked SQLite is lenient about BLOB->JSON, so `go test` can't reproduce this; only the strict production build hit it.
The index serve falls back to the legacy rebuild for filters the flattened index doesn't maintain (errScopeNotRepresentable) and whenever the serve context is torn down mid-query (context.Canceled, or a SQLITE_INTERRUPT/BUSY from the interrupt handler). Those are normal and were flooding the logs at warn. Key the downgrade on ctx.Err() so genuine index-path failures still warn.
The preflight Has filter runs once up front, but the wantlist is fetched in tiers over several seconds while other concurrent discovery tasks persist the same blocks. Each tier fetched against that stale snapshot and re-pulled blocks already on disk, dropped at putBlock as `exists` — 47-67% of received bytes on a cold sync. Right before each tier's GetBlocks, re-check Has (skip blocks on disk now) and claim CIDs in a daemon-wide in-flight set (skip blocks another task is fetching now). A skipped block is already ours, owned by another task, or re-driven next cycle. Measured on a cold sync: exists ~0% for the first 6 minutes, one bounded burst, then flat/declining — versus the old continuous 47-67% climb.
…body as a CID The file-upload helpers read the daemon response body as the CID without checking the status. On a failed upload the daemon returns a non-2xx with the error text in the body, so that text became the CID and got baked into documents as `ipfs://Failed to add file...`. Guard response.ok in the five upload paths that lacked it (the other five already did).
…cascade When a late dependency/capability arrives and unstashes older Refs/Changes, reindexStashedBlobs re-indexes them via indexBlob, but their ids were never passed to runIndexedHook — only the top-level ids from Put/PutMany were. So the legacy tables got updated while the maintained RBSR index (rbsr_item) did not, and reconcile could serve a stale-short set. Thread a hookIDs accumulator through indexBlob (recording every blob it indexes, including nested cascade re-indexes) and hand the full set to runIndexedHook. The full-reindex path passes nil (it rebuilds the derived tables wholesale). Adds a regression test asserting the hook fires for a ref unstashed by a capability cascade. Reported by @burdiyan.
A hard-killed daemon leaves its WAL un-truncated. A large inherited WAL makes every read slow (each read resolves pages through the WAL), and once the app is serving the constant readers stop the background PASSIVE checkpointer from ever reclaiming it — PASSIVE only advances up to the oldest active reader — so it stays huge and stalls indexing and reconcile serving for the whole session. Run one blocking wal_checkpoint(TRUNCATE) during Open, right after the DB is opened and before any subsystem can read. With no readers yet it drains the inherited WAL cleanly: a brief, one-time boot delay proportional to the WAL size instead of a persistent live stall. A clean shutdown already truncates on Close, so this only does work after an unclean exit. Verified: a hard-kill-inherited 343 MB WAL drains to zero ~1.5s into boot, before readers exist (StartupWALDrained logs the pre-drain size).
The RBSR maintenance hook runs as the last step of every Put/PutMany indexing transaction. Any hook error used to roll back the whole batch — up to 100 freshly synced blobs — and a deterministic error would block those blobs from ever syncing. The hook only maintains the derived rbsr_item index, which the shadow-verify sweep now repairs on drift, so failing the batch is never worth it: log the error and commit. Context cancellation still propagates (the transaction is doomed then anyway). Partial hook writes are safe to commit (INSERT OR IGNORE, per-scope repairs).
… path Live-DB forensics found the maintained index diverging from collectBlobs under real load (one scope 43% under-advertised; comments among the missing blobs), scopes flapping stale every sweep, and the serve path serializing on the single writer connection. Root causes and fixes: - The agent-capability closure ignored scope type filters, pouring capabilities into dirStructure (Ref,Change) scopes that legacy collectBlobs keeps them out of — permanent drift-flag flapping. Both the scopesWithAuthor trigger and the closure loop now gate on scopeAllowsType. - qMediaClosure inserted size<0 placeholder rows that materialization and shadow-verify exclude — the second flap source. The closure and qInsertItem now enforce the downloaded-only membership contract; the late-arrival edge re-adds blobs once downloaded. - Scopes marked stale dropped out of both the oracle and the sweep, rotting until their next serve. The sweep now heals in place instead of mark-stale: verification runs on a READ connection (clean scopes never touch the writer), drift re-materializes immediately in its own write tx, materialized=0 rows heal directly, and a per-scope error no longer aborts the pass. - The full-pass-every-5-minutes sweep is now a trickle: 25 scopes every 30s in id order with a wrap-around cursor, skipping scopes not accessed within 72h (serve-time materialization covers those), with optional idle eviction behind a disabled constant. It also pauses while a reindex is rebuilding the derived tables. - Comments addressed by their own hm://author/tsid IRI never received incremental updates (the oracle keys on the target-doc IRI). The structural-facts pass now mirrors the fillTables TSID seed for scopes addressed by the resource's own IRI, any kind. - Serving took the writer connection for every request (resolveScope always INSERTs). The load is now three-phase (read-lookup, write only for cold scopes, read-build with a mid-serve reindex re-check), shared by the server and the client, with best-effort hour-granular last_access refresh off the latency path. - The client store build (63 subscriptions x 2 collectBlobs/min on a desktop — the dominant CPU) now reuses the maintained index for representable scopes, with the same legacy fallback as the server. Version is stripped for scope identity, mirroring fillTables. The size>=0 membership contract guarantees the client never over-claims a blob it hasn't downloaded, so wants are never suppressed. The parity suite now drives comments through all of it: a three-scope (recursive / dirStructure / exact-TSID) differential test with staged arrivals, undownloaded link targets, comment edits, and a capability that must stay out of the filtered scope; sweep tests for heal-in-place, rot healing, error continuation, and cursor paging; a serve test that completes with the writer connection held hostage; and a client-vs- legacy store equality test. Validated against a copy of a real 1,250-scope database: one healing rotation converges every scope to collectBlobs exactly, the second rotation is fully clean.
Two ops diagnostics for the maintained RBSR index, both usable against a copy of any daemon database: - shadow_verify_offline_test.go (env-gated via SEED_SLIM_DB): runs full trickle rotations over every real scope, requiring zero failures and full convergence by the second rotation. A slim copy (all metadata tables, blobs.data NULLed) keeps it to tens of MB. - tools/rbsr-scope-diff.sh: read-only SQL replica of collectBlobs for one scope, diffed against its rbsr_item rows — safe against a live daemon database. Lists under/over-advertised blobs by type.
…deadlines CPU profiles on both a desktop (63 subscriptions) and a site server showed the scheduler's dispatch loop as the top consumer — ~40% of daemon CPU on the desktop — nearly all of it in full task-map scans from cleanupExpiredHotTasksLocked and boundedWake, at thousands of wakes per second. Two zero-wake sources caused the spin: - boundedWake clamped expired in-progress heartbeats to a zero wake; an in-progress task past its heartbeat stays in that state until its worker unwinds (canceled but blocked in a fetch), so the timer refired immediately, rescanned the map, and produced another zero wake for the entire unwind window. - dispatchReadyTasks could return a zero (or negative) wake for a cold task already due while all workers were saturated — spinning until a worker freed up, even though worker completions re-enter the dispatcher anyway. Deadline-derived wakes are now floored at 100ms (imperceptible for reaping or retrying; resetTimer's deliberate zero-wake dispatch of a ready hot task is untouched), and the cleanup scan is rate-limited to once per second. Heartbeat extension for actively-progressing expired tasks still happens within that second, so the floor bounds the residual churn to ~10 wakes/s worst case.
4e3a86f to
6366f13
Compare
…f minutes A comment on a subscribed site's document took 3+ minutes to arrive while the document was on screen with a live connection to the site peer. Four scheduler/wave defects stacked up: - scheduleNext checked subscription before the hot heartbeat, so a viewed subscribed space stayed on the 60s interval; hot now wins with the 10s cooldown, and a hot touch pulls a far-out subscription run within the cooldown instead of waiting out the pending interval slot. - Cold subscription rounds could occupy every worker; one slot is now reserved for hot tasks (skipped on single-worker pools). - The straggler quorum counts failures, so a wave over 60 known peers with 34 dead ones completed at the pace of their dial timeouts (~60s per round). Hot waves now cut early on a success signal: a few peers synced OK, nothing reconciled still owed, downloads idle. - Preemption could kill a just-dispatched round during peer selection (no progress signals exist yet at that phase), which starved every round under the post-restart task burst. Runs younger than progressGrace are now protected. E2E: a subscriber with the pull interval parked at 1h receives a new comment in ~10s while hot-touching the space (fails without the fix).
…ts fresh - Discovery poll 14s -> 8s to track the daemon's ~10s hot re-sync cooldown while focused. - Cap the unfocused poll at 30s: the 10x background multiplier produced 140s, past the daemon's 40s hot-task heartbeat TTL, so alt-tabbing to a browser silently degraded the open document to background- subscription latency (minutes). - Invalidate COMMENT_REPLY_COUNT when comment blobs sync in: the discussions list shows Reply (N) per thread, and it stayed stale until remount, so arriving comments changed nothing visible.
The group sort only considered the rendered linear chain, so a fresh reply behind a branch point (counted in moreCommentsCount but not part of the chain) left its thread pinned in place - new activity was invisible in the list order. Latest activity now includes every branched reply behind the chain's last comment.
qGetReplyCountByID counted distinct source blobs, and every edit of a reply is a new blob carrying the same reply-parent/thread-root links, so an edited reply inflated the thread root's Reply (N) by one per edit (observed: 12 shown for a thread with 11 replies). Dedupe by the source comment's identity (author + tsid).
What
Replaces the per-round RBSR store rebuild with a maintained, persistent fingerprint index so an interested, already-connected peer gets a new blob in ~1s without melting CPU.
Today every reconcile round (and every peer) rebuilds the entire sorted blob set from SQLite via
collectBlobs. This PR persists each scope's resolved set, serves range fingerprints from an O(log N) monoid tree, keeps the set current incrementally as blobs are indexed, and drops the hot discovery cooldown to 1s.How
rbsr.NewTreeStore): an augmented treap with deterministic hash-derived balancing.RangeFingerprintis O(log N) and byte-identical to the existing linear fold (the fold never sets the accumulator length, so the tree doesn't either). Supports post-seal insert. Wired via an optionalRangeFingerprinterseam inSession.Fingerprint(falls back to the fold otherwise).rbsr_scope/rbsr_item+ migration): a scope = a canonical discovery key + protocol version. We persist the resolved blob set (not literal tree nodes — a treap with rotations doesn't map to stable rows) and rebuild the in-memory tree from those rows. Materialization runscollectBlobsonce; reindex drops the tables (lazy re-materialize).loadStoreserves from the persistent index, with a reindex-gated fallback to the legacy rebuild.affectedScopes): the incremental membership decision — which materialized scopes a freshly indexed blob joins. Seed-and-forward: only the seeding decision is new logic; the closure reusescollectBlobs' own forward walk, so it can't silently diverge. Covers the dominant document-edit path (a new Ref + the Changes it heads via the media closure).Index.SetIndexedHook): patches materialized scopes inside the indexing transaction, so the maintenance commits atomically with the blob./0.9.3canonicalization primitives:canonicalCodecFor(dag-pb → raw), scope keyed by protocol version, build-from-rows applies it. Unit tests prove a dag-pb blob advertises identically to a raw-storing peer.Deferred (NOT in this PR): live
/0.9.3protocol negotiationThe canonicalization primitives are in and tested, but the live wire negotiation is deliberately left out and must be a separate, e2e-tested change. I implemented it, found a concrete breakage, and reverted it:
connect.gogates connections withCheckHyperMediaProtocolVersion(..., n.protocol.Version), requiring the peer'sFindHypermediaProtocolresult to exactly equal0.9.2. PeerstoreGetProtocolsreturns a map (nondeterministic order), so advertising both0.9.2+0.9.3would intermittently surface0.9.3and reject the peer.FindHypermediaProtocol/CheckHyperMediaProtocolVersionfor highest-mutual negotiation, a second listener/server tagging the canonical context, and client-side dial selection + a canonicalized client store. That touches connection establishment and must be validated with the daemon e2e harness.Until then the index serves the legacy-codec set (identical fingerprints to the old path), so this PR is backward-compatible.