eschutho opened a new pull request, #44402:
URL: https://github.com/apache/superset/pull/44402
### SUMMARY
**The problem.** When a chart has annotation layers, Superset was saving a
separate copy of that chart's cached data for *every user* who viewed it — even
when all of them would see exactly the same result. A single chart could turn
into hundreds of identical copies in the cache, and since each copy can be tens
of megabytes, this wasted a large amount of cache memory for no benefit.
**Why it happened.** Annotations (the extra markers/notes drawn on a chart)
are loaded using the viewer's own permissions, and they get stored in the
*same* cache entry as the chart's data. To make sure a user could never be
served annotation data they weren't allowed to see, the code mixed the viewer's
user ID into the cache key. That was safe, but too blunt: because the user ID
is part of the key, every user got their own copy — including users who would
see identical data.
**The fix.** Instead of keying the cache on *who the user is*, we now key it
on *what the user is allowed to see* (their access scope). Users with the same
access share one cache entry; users with different access — or no access —
never share one. This keeps the exact same safety guarantee (nobody is served
annotation data they shouldn't see) while letting identical results be reused
instead of duplicated. Concretely:
- **Built-in ("native") annotation layers** show global annotation records
that are gated only by the "can read annotations" permission, so the key now
includes just that permission flag.
- **Chart-based annotation layers** (annotations pulled from another chart)
run a query against that chart's data source. The key now includes (a) whether
the user can access that data source — previously this access check only ran
when the data was missing from the cache, not when it was served from the cache
— and (b) that other chart's own cache key, which already accounts for the data
source version, row-level security (RLS) rules, and any per-user RLS logic.
So users with genuinely different access still get their own cache entries;
users with the same access now share one.
**Second, related change: size cap.** Superset can already skip caching
values that are too large (`DATA_CACHE_MAX_VALUE_SIZE`), but a few code paths
wrote to the DATA cache directly and skipped that check: the SQL executor's
result cache, and two datasource endpoints (filter-dropdown values, and
metrics/dimensions). This PR sends those through the same size check so
oversized values can't slip in there either. It does nothing when the cap is
turned off (the default), so it adds no overhead by default.
**Expiry (TTL) note.** Every cache write touched here already sets an expiry
time. The special `timeout=0` ("never expire") value is an intentional,
documented feature and is left unchanged.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — this is a caching / cache-key change with no UI surface.
### TESTING INSTRUCTIONS
Unit tests (added/updated, verified to fail before the fix and pass after):
```
pytest tests/unit_tests/common/test_query_context_processor.py -k
annotation_cache_key
pytest tests/unit_tests/utils/cache_test.py
```
- The annotation cache-key tests check that identical annotation-layer
results are shared across users with the same access, and still stay separate
when the annotation-read permission, datasource access, or RLS genuinely
differs.
- The cache-util tests check that the shared size guard skips oversized
values (and counts `skip_cache_value_too_large`), and does nothing when the cap
is disabled.
Manual check: open a chart with annotation layers as two users who share the
same role and RLS, and confirm only one cached data entry is created (not one
per user). Then view it as a user whose access or RLS differs, and confirm they
get their own separate entry.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
- [x] Bug fix (non-breaking change which fixes an issue)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
