codeant-ai-for-open-source[bot] commented on code in PR #44662:
URL: https://github.com/apache/superset/pull/44662#discussion_r4171058792
##########
superset/utils/cache.py:
##########
@@ -54,6 +54,67 @@ def generate_cache_key(values_dict: dict[str, Any],
key_prefix: str = "") -> str
return cache_key
+def exceeds_max_cache_value_size(cache_key: str, value: Any) -> bool:
+ """Check a value against ``DATA_CACHE_MAX_VALUE_SIZE`` before a data-cache
write.
+
+ This keeps one oversized value from flooding the cache backend (e.g.
+ Redis/Memcached) and evicting many smaller entries. When the serialized
+ (pickled) size of ``value`` is larger than the limit, a WARNING naming the
key
+ and size is logged and the ``skip_cache_value_too_large`` statsd counter is
+ incremented; the caller must then skip the write. Cache writers use
+ :func:`skip_oversized_cache_value`, which also removes any older value
stored
+ under the key.
+
+ :returns: ``True`` when the value is too large and must not be cached.
Always
+ ``False`` when ``DATA_CACHE_MAX_VALUE_SIZE`` is ``None``, in which
case the
+ value is not serialized and no overhead is incurred.
+ """
+ max_value_size = app.config.get("DATA_CACHE_MAX_VALUE_SIZE")
+ if max_value_size is None:
+ return False
+ value_size = len(pickle.dumps(value, protocol=pickle.HIGHEST_PROTOCOL))
+ if value_size <= max_value_size:
+ return False
+ logger.warning(
+ "Skipping cache set for key %s: serialized value size %d bytes "
+ "exceeds DATA_CACHE_MAX_VALUE_SIZE (%d bytes)",
+ cache_key,
+ value_size,
+ max_value_size,
+ )
+ app.config["STATS_LOGGER"].incr("skip_cache_value_too_large")
+ return True
+
+
+def skip_oversized_cache_value(
+ cache_instance: Cache, cache_key: str, value: Any
+) -> bool:
+ """Decide whether a data-cache write must be skipped for size, and if so
remove
+ any older value stored under the same key.
+
+ Every writer to the data cache calls this before writing. Without the
delete,
+ an older, smaller value under ``cache_key`` would survive the skipped
write and
+ be served on the next read, even though a fresher result was just computed.
+ With the delete, the next read misses and recomputes. Deleting is
best-effort:
+ a failure is logged and never raised, and it is a no-op for ``NullCache``.
+
+ :returns: ``True`` when ``value`` exceeds ``DATA_CACHE_MAX_VALUE_SIZE``
and the
+ caller must not write it (see :func:`exceeds_max_cache_value_size`).
+ """
+ if not exceeds_max_cache_value_size(cache_key, value):
+ return False
+ if not isinstance(cache_instance.cache, NullCache):
+ try:
+ cache_instance.delete(cache_key)
Review Comment:
**Suggestion:** An oversized request can delete a newer, smaller value
stored concurrently between size checking and deletion, causing unnecessary
cache misses and recomputation.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cf1402a42b6a4a1186d6625989a9e724&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cf1402a42b6a4a1186d6625989a9e724&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/cache.py
**Line:** 104:108
**Comment:**
*Race Condition: An oversized request can delete a newer, smaller value
stored concurrently between size checking and deletion, causing unnecessary
cache misses and recomputation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44662&comment_hash=d3cf92824b7cbd9fdc1703ee9b319169396a33ab968fb9f76813dca89b687243&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44662&comment_hash=d3cf92824b7cbd9fdc1703ee9b319169396a33ab968fb9f76813dca89b687243&reaction=dislike'>๐</a>
--
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]
