Repository navigation
perf(channels): coalesce concurrent GetChannels/GetEncryptedChannels cache misses #109
Description
Upstream source
Evaluate and port the behaviour from Kpa-clawbot/CoreScope#2059: coalesce concurrent cache misses for GetChannels and GetEncryptedChannels so one cold/expired key causes one SQLite query rather than one query per request.
Why this is still relevant here
Current fork master still has no channel-query singleflight. GetChannels uses a 60-second cache, but concurrent callers can all miss before the first result is stored. GetEncryptedChannels follows a separate query path.
This fork has diverged from upstream:
- PR perf(db): pin GetChannels and CountFloodAdvertsForNode to their indexes (issue #100) #107 pins the channel query to the intended index with a safe fallback.
- PR fix(server): avoid mutating the cached channels slice on includeEncrypted #98 protects the cached channels slice when encrypted results are appended.
- The channel cache currently has a single cached region key rather than upstream's map-shaped cache.
Do not cherry-pick blindly. Preserve those behaviours and adapt request coalescing to the current cache contract.
Acceptance criteria
- Concurrent misses for the same normalized region execute at most one real
GetChannelsquery. - Concurrent misses for the same normalized region execute at most one real
GetEncryptedChannelsquery. - Different region keys are never coalesced together and cannot receive each other's result.
- Errors are shared only within the in-flight call and are not cached as successful data.
- A second cache check occurs inside the singleflight winner.
- Existing index-pinning/fallback behaviour from perf(db): pin GetChannels and CountFloodAdvertsForNode to their indexes (issue #100) #107 remains intact.
- Existing cache-slice protection from fix(server): avoid mutating the cached channels slice on includeEncrypted #98 remains intact.
- Deterministic tests count actual query executions; timing alone is not sufficient.
- Run the relevant Go tests with
-raceand measure cold concurrent behaviour before and after.
No deploy is part of this issue.
Activity
dborup commentedon Sep 28, 2026
Additional scope found in upstream Kpa-clawbot#1936
The selective cache work in Kpa-clawbot/CoreScope#1936 reinforces this issue but should not be ported wholesale.
Current fork findings:
GetChannelsstill has a single-slot cache rather than a cache keyed by normalized region.GetEncryptedChannelshas no equivalent cache.annotateMessageAreasandannotateBotReplyTouchedAreas.Recommended extension to this issue: use keyed caches plus per-key singleflight for normalized-region channel and encrypted-channel queries. Benchmark message-page caching as a separate decision, and require an immutable/copy-on-read contract before adding it.
dborup commentedon Sep 29, 2026
Draft PR #138 klar til review: CI grøn (Go Build & Test, Playwright E2E, Docker build).
GetChannelsogGetEncryptedChannelsbruger nu en begrænset cache pr. normaliseret region med singleflight og et ekstra cache-tjek inde i flighten. #107 og #98 er bevaret. Der er ingen message-page-cache og intet nyt indeks. 16 samtidige kolde kald giver 2 queries (master: 16).Generated by Claude Code
fix(channels): fail a channel list query whose rows stop on an error (#…
fix(channels): bound the byte size of channel list cache keys (#109)
test(channels): pin that a long region key is queried unhashed (#109)
dborup commentedon Sep 30, 2026
Fixed by #138 (merged as
e829d1ea). The list cache is per normalized region, concurrent misses share one query,rows.Err()is checked, and keys over 256 bytes are stored as a digest while the query still gets the full region. Review note, not blocking: TTL expiry itself has no dedicated test.