eschutho opened a new pull request, #44662:
URL: https://github.com/apache/superset/pull/44662
### SUMMARY
Superset keeps query results in a "data cache" (usually Redis or Memcached)
so repeat loads are fast. A single very large result, tens of megabytes, can
fill that cache's memory and push out many smaller, useful entries. Superset
already had a setting to stop this, `DATA_CACHE_MAX_VALUE_SIZE`: a cap on how
big one cached value may be. It was off by default, and some code paths that
write to the data cache ignored it.
This PR:
1. **Turns the cap on by default at 10 MB.** A value larger than 10 MB is
not stored. The request still returns its data, but the next load re-runs the
query because nothing was cached. Each skipped write logs a WARNING that names
the cache key and the size, and increments the `skip_cache_value_too_large`
statsd counter. Operators can raise the cap or set it to `None` to turn it off.
2. **Applies the cap to every writer to the data cache.** The size check
moves into a small shared helper, `exceeds_max_cache_value_size()` in
`superset/utils/cache.py`. The normal chart path (`set_and_log_cache`) uses it,
and so do three places that wrote to the cache directly and skipped the check:
- the SQL executor's result cache (`superset/sql/execution/executor.py`)
- the column-values endpoint that fills filter dropdowns
(`superset/datasource/api.py`)
- the compatible metrics/dimensions endpoint
(`superset/datasource/api.py`)
In all three, a skipped write is just a cache miss: the next request runs
the query again. Nothing reads the value back expecting it to be there, so
skipping can't cause a loop or an error.
3. **Fixes an endless retry with async chart queries.** With
`GLOBAL_ASYNC_QUERIES` on, a background worker runs the chart query and stores
the result in the data cache. The browser then asks for the chart again and
expects to read that stored result. If the result was too big to store, the
browser's read misses, the server treats that as "not ready yet", and a new
background job starts, again and again. With this change the worker checks that
the result really landed in the cache. If it didn't, the job fails with a clear
message ("The query result could not be cached and cannot be returned
asynchronously... exceeds DATA_CACHE_MAX_VALUE_SIZE..."), so the user sees an
error instead of a chart that never finishes loading.
4. **Docs.** Updates the config comment, the caching docs page, and
`UPDATING.md`. Also rewrites a code comment in `superset/utils/cache.py` that
still called `None` the default.
A timeout of `0` ("never expire") behaves as before.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable (no UI change). With async queries on, an oversized chart
result now shows an error instead of loading forever.
### TESTING INSTRUCTIONS
Unit tests:
```
pytest tests/unit_tests/utils/cache_test.py \
tests/unit_tests/tasks/test_async_queries.py \
tests/unit_tests/sql/execution/test_executor.py \
tests/unit_tests/datasource/cache_size_cap_test.py
```
What they cover:
- The default cap is set. A normal-size value is cached with the real
default, and a value over the default is skipped with a WARNING and the counter.
- Boundary: a value exactly at the cap is cached, and one byte over is
skipped.
- With the cap set to `None`, values aren't even serialized to measure them.
- An explicit cache timeout is passed through unchanged, and a timeout of
`0` is still honored.
- Async: when the query succeeds but its result isn't in the cache, the task
raises and never reports a cache key the client would retry against.
- The SQL executor, column-values endpoint, and compatible endpoint each
skip an oversized value (still returning it to the caller) and cache a normal
one.
Manual check (optional):
1. Configure a real data cache (e.g. Redis) and set
`DATA_CACHE_MAX_VALUE_SIZE = 1024` in `superset_config.py`.
2. Load a chart that returns more than 1 KB of data. It still renders. The
logs show `Skipping cache set for key ...`, and reloading re-runs the query.
3. With `GLOBAL_ASYNC_QUERIES` on, load the same chart. It shows the "could
not be cached" error rather than loading forever.
### 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))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Behavior change for operators: the new 10 MB default means results over 10
MB are no longer cached. See `UPDATING.md`.
🤖 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]
