perf(conversion): cache conversion chart results for 60s#270
Open
ayushjhanwar-png wants to merge 1 commit into
Open
perf(conversion): cache conversion chart results for 60s#270ayushjhanwar-png wants to merge 1 commit into
ayushjhanwar-png wants to merge 1 commit into
Conversation
The conversion TRPC procedure recomputes the full conversion query for every dashboard load, even when the same chart is opened repeatedly in quick succession. Repeat opens within a minute end up running the same 40s+ CH query multiple times. Reuses the existing cacheMiddleware(60) defined in chart.ts (already applied to overview procedures, just not to conversion). One-line change: add .use(cacher) between .input() and .query(). Cache key is hash of the full input (project, date range, breakdowns, cohort, etc.) so different chart configurations get separate entries. TTL of 60 seconds matches the other cached chart procedures. This addresses repeat-load doubling. The parallel current + previous doubling within a single chart load (when input.previous is true) is a separate problem requiring frontend changes.
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.
Summary
Apply the existing `cacheMiddleware(60)` to the conversion TRPC procedure. Conversion queries are the slowest chart type, and repeat dashboard loads currently recompute the full SQL every time.
The middleware already exists in chart.ts:61 and is applied to all overview procedures. The conversion procedure was missing it.
Change
One-line addition between `.input()` and `.query()`:
```ts
```
How it works
What this addresses
Repeat-load doubling: when multiple users open the same chart within 60s, or when a single user reloads the dashboard, only the first load hits ClickHouse. Subsequent loads return from Redis.
What this does NOT address
The parallel current + previous-period query within a single chart load (when `input.previous` is true). That's a separate problem requiring frontend changes to lazy-load the comparison period.
Risks