perf(webapp): history stops re-downloading every journal on every request (BEA-85) - #133
Open
ssowonny wants to merge 1 commit into
Open
Conversation
…verything (BEA-85) History folded every journal of a project on every request — 8-10s on a real project, and paging made it worse rather than better, since each "load more" re-downloaded and re-parsed the lot. There is no DB in this path, so no index was ever going to help. Journals only GROW: a device appends only to its own (the one-writer invariant) and the hub's appendOp rewrites its key with strictly more bytes. So the (Size, Modified) that List already reports proves a parse is still current — no staleness window, no expiry, no new Backend method. A warm request now pays one List and nothing else. The cold path can never be helped by a cache, so the Get loop is concurrent too (errgroup, limit 8) instead of one serial round trip per device. Files() reads the same funnel, so folder listings get it for free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
TL;DR
(size, mtime)thatListalready returns proves a cached parse is still good. A warm request now does oneListand zero downloads.What was actually slow
flowchart LR R["every request"] --> L["List journal/"] L --> G["Get EVERY journal<br/>one at a time"] G --> P["parse EVERY op<br/>ever written"] P --> S["sort twice,<br/>slice one page"] classDef hot fill:#ef444422,stroke:#ef4444,stroke-width:2px class G,P hotloadSourcedOpsis the funnel every reader goes through — history, folder listings, restore, and the hub's ownappendOp— and it cached nothing. A cursor is a position in an ordering, not a snapshot, so paging re-loaded everything and skipped forward in memory.What it does now
flowchart LR R["every request"] --> L["List journal/<br/>one round trip, always"] L --> C{"size + mtime<br/>unchanged?"} C -- hit --> U["reuse the parsed ops"] C -- miss --> F["Get + parse<br/>concurrently, limit 8"] U --> A["fresh outer slice<br/>callers sort it"] F --> A classDef good fill:#22c55e22,stroke:#22c55e,stroke-width:2px class U,F goodThe enabling fact is already load-bearing elsewhere in the repo: journals only grow. A device appends only to its own (the one-writer invariant) and the hub's
appendOpis a read-modify-write that rewrites the key with strictly more bytes. Nothing shrinks or rewrites one in place, so a matching(Size, Modified)proves the parse we hold is current — no staleness window, no time-based expiry, and no newBackendmethod. All three real backends report both fields.Files()needed no change: it goes through the same funnel, so folder listings get the cache for free. Please don't add a second cache next tovolume.snapshot.Numbers
Measured locally with a throwaway test (not committed): 16 000 ops across 8 journals, behind a backend with a simulated 40 ms round trip.
The warm request is now the
Listplus assembly. The 1s target in the issue is inferred, not given, and the reporter's real project is unmeasured — if the cold path there is still slow, that's the signal to do the tail-read step rather than loosen the number.What can't break
TestHistoryOutputUnchangedByCachecaptures/historyacross five query shapes warm, drops the cache, and compares byte for byte — ordering, cursors,add/edit/deleteclassification and the device-registry join all unchanged. Ops are assembled in the sameListorder the old loop used, which matters because both downstream sorts areSliceStable.[]journal.Opis never handed out directly, or the first sort would scramble the cache.journal.Opis all value fields, so appending one copies it.appendOpreads through the same cache while holdingupmuand computesmaxLamport/mySeqfrom what it reads. Safe because invalidation is by(size, mtime)and it re-Lists inside the lock, so it sees its own previous write. Deliberately no "skip the cache for writes" shortcut — that would reintroduce the serial fetch on the upload path.What you're accepting
s.volsnever evicts). Bounded at 64 MiB of raw journal bytes per project with all-or-nothing eviction — over the cap it drops everything and that pass costs exactly what every pass cost before. The real ceiling is that times the number of projects touched, named in aponytail:comment with the upgrade path.Modifiedgranularity onfile://is coarse on some filesystems. Size is the primary token and always changes on append; the mtime pairing is belt-and-braces.Deviation from the plan
One, small: the plan pruned vanished journals in the store pass. That pass only runs when something was fetched, so a journal that disappeared would never be seen and its entry stayed resident forever. Pruning moved into the partition pass, which runs on every request.
TestJournalCacheDropsVanishedJournalspins it.Architecture changes
architecture/webapp-server.md:RemoteSourcegained the cache fields (jcache,jbytes) andcacheJournals; new value typecachedJournalcomposed into it. Nothing removed.flowchart TB RemoteSource["<div style='text-align:left'><b>RemoteSource</b><br/>+Backend remote.Backend<br/>+Device Identity<br/>+PresignTTL time.Duration<br/>+Remove(ctx, path, who, note)<br/>+OpenBlob(ctx, sha)<br/>-verify(ctx, sha) re-hash until sealed<br/>-blobStat(ctx, blob) remote.Object<br/>-sealed sync.Map sha→proved immutable<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -jcache map key→cachedJournal</span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -jbytes int64 raw bytes cached</span><br/>-loadSourcedOps(ctx) []sourcedOp<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -cacheJournals(keep, misses, parsed, sizes)</span><br/>-appendOp(ctx, op)</div>"] sourcedOp["<div style='text-align:left'><b>sourcedOp</b><br/>+Op journal.Op<br/>+From journal key's device</div>"] cachedJournal["<div style='text-align:left'><b>cachedJournal</b><br/>+size int64<br/>+mod time.Time<br/>+bytes int64<br/>+ops []journal.Op</div>"] Why["Journals only GROW, so List's<br/>size + mtime is a free version token.<br/>No expiry, no new Backend method.<br/>Bounded by a raw-byte cap per project."] RemoteSource -. "attribution comes from the journal key" .-> sourcedOp RemoteSource -- "<span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅ parsed ops, keyed on size+mtime</span>" --> cachedJournal cachedJournal -.- Why classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class cachedJournal added class Why noteBox linkStyle 1 stroke:#22c55e,stroke-width:2pxWhat was run
go build ./...,go vet ./...— cleango test ./...— all packages passgo test -race -timeout 40m ./internal/webapp/— clean (23 min; it needs the longer timeout because unrelated slow tests blow past Go's 10m default)npm run e2e— 153 passed, 1 skipped (the skip is pre-existing)internal/webapp/journalcache_test.go— warm request fetches zero journals, one push invalidates only that journal, paging fetches nothing further, output byte-identical with and without the cache, vanished journals are dropped and the byte total stays honestNo frontend changes, so
static/is untouched.Closes BEA-85.
Build session
(only works on this machine)