codeant-ai-for-open-source[bot] commented on code in PR #43759:
URL: https://github.com/apache/superset/pull/43759#discussion_r4155616554
##########
superset/datasets/api.py:
##########
@@ -153,6 +161,31 @@
)
+def _consume_preview_rate_limit(dataset_id: int) -> bool:
+ """
+ Fixed-window per-user, per-dataset throttle on the preview endpoint.
+
+ Debouncing on the client is a courtesy, not a guard: a held keydown, or a
+ handful of owners with the editor open, becomes sustained load on a
+ production cluster. Returns False once the window's budget is spent.
+ """
+ limit = app.config.get("PARTITION_TRANSFORM_PREVIEW_RATE_LIMIT", 30)
+ if not limit:
+ return True
+
+ user_id = get_user_id() or 0
+ key = f"partition_mapping_preview:{user_id}:{dataset_id}"
+ try:
+ used = cache_manager.cache.get(key) or 0
+ if used >= limit:
+ return False
+ cache_manager.cache.set(key, used + 1, timeout=60)
Review Comment:
Agreed—the concern is valid. `backend.inc()` is not guaranteed to preserve
the TTL on every cachelib backend; on read-modify-write implementations it can
restamp the key, turning the fixed window into a sliding or unexpectedly long
lockout.
The safest fix is to make this limiter Redis-only and use Redis-native
operations:
- `SET key 1 NX EX 60` for the first request
- `INCR key` for subsequent requests, without calling `EXPIRE` again
- explicitly document Redis as a requirement for enforcing this limit
- fail open, as today, when Redis is unavailable or another backend is
configured
Alternatively, use a dedicated fixed-window rate-limiter implementation that
supports an atomic increment while preserving the original TTL. The current
`add()`/`inc()` combination is not sufficient to guarantee the documented
behavior across supported backends, so this should be corrected before merging.
--
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]
