perf: index, cache, and deflake /api/channels queries (rebase of #1887) - #1936
Merged
Conversation
/api/channels and /api/channels/{hash}/messages could take seconds
under load because of five compounding issues:
1. GetChannels' correlated "latest message" subquery and
GetChannelMessages' count/page queries sorted per channel_hash with
no covering index on first_seen. Added
idx_transmissions_channel_hash_payload(channel_hash, payload_type,
first_seen) so these resolve as index-order scans.
2. GetChannelMessages had no cache at all - every page view/poll re-ran
the full paginated query. Added a 10s TTL cache keyed by
hash+limit+offset+region.
3. GetChannels' cache was a single slot keyed by one region string, so
any mix of region values thrashed it into permanent cache misses.
Replaced with a map keyed by region.
4. GetEncryptedChannels had no cache at all. Gave it the same
map-keyed 60s TTL cache as GetChannels.
5. The region filter's UPPER(TRIM(obs.iata)) predicate couldn't use a
plain index. Added an expression index,
idx_observers_iata_norm(UPPER(TRIM(iata))), on observers.
The channel_hash column is added by the ingestor's legacy
'channel_hash_v1' migration, not by dbschema.go, so
ensureChannelIndexes probes for it before indexing rather than
assuming it exists.
Updated TestGetChannelMessagesPerfLargeChannel to clear the new cache
between its warm-up and timed calls so the regression guard still
measures the real SQL path instead of a cache hit.
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
2 participants
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.
Continues #1887 by @Jonher937. The commit is theirs, authorship unchanged; I only rebased it onto master.
It went CONFLICTING because #1934 (prepared statements, originally @Joel-Claw's #1878) landed in the same
DBstruct. Both PRs add fields there and this one also replaces the single-slot channels cache.Resolution: kept this PR's keyed caches (
channelsCache,encChannelsCache,msgCacheplus their entry types and TTL constants) and kept master's thirteen prepared-statement fields alongside them. The old single-slotchannelsCacheKey/channelsCacheRes/channelsCacheExptrio is gone, which is the point of this PR. Nothing else touched.Verified:
cmd/serverbuilds and the full suite passes, not just the channel tests.My review stands: approve, with two questions that do not block and are worth a look at some point.
msgCacheis keyed byhash|limit|offset|region, andoffsetgrows without bound as someone pages through a channel. Each entry also holds a full page of message maps, so a full 256-entry cache atlimit=50holds around 12,800 maps. The other two caches are keyed by region only and genuinely low-cardinality as your comment says; this one is the odd one out.getMsgCachereturns the cached slice directly, so every hit hands the caller the same message maps. If any handler mutates one before serialising, it corrupts the cache for the next ten seconds. Same class as the finding on perf: use cached ParsedDecoded() instead of repeated json.Unmarshal #1871, which was fixed there by copying at the two broadcast sites.