flyrain commented on code in PR #5122:
URL: https://github.com/apache/polaris/pull/5122#discussion_r3653121897
##########
runtime/service/src/main/java/org/apache/polaris/service/idempotency/EntityIdempotency.java:
##########
@@ -52,20 +56,32 @@
* evolve the encoding later without ambiguity.
*
* <p>For {@code createTable} this window holds a single entry, but the shape
generalizes to a
- * bounded per-entity window for repeated mutations (e.g. {@code
updateTable}). Expired keys are
- * dropped inline whenever the entity is rewritten ({@link #recordKey});
expiry is also honored at
- * read time ({@link #hasLiveKey}) so a stale-but-not-yet-purged key is
treated as absent.
+ * per-entity window for repeated mutations (e.g. {@code updateTable}). The
window is bounded only
+ * by expiry (TTL): expired keys are dropped inline whenever the entity is
rewritten ({@link
+ * #recordKey}), and expiry is also honored at read time ({@link #hasLiveKey})
so a
+ * stale-but-not-yet-purged key is treated as absent. Live keys are never
evicted by count; a large
+ * safety ceiling ({@link #MAX_LIVE_KEYS}) bounds the window by failing the
write rather than
+ * dropping a still-live key.
*/
public final class EntityIdempotency {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(EntityIdempotency.class);
+
/** Internal-properties key under which the per-entity idempotency key
window is stored. */
public static final String IDEMPOTENCY_KEYS_PROPERTY =
"polaris-idempotency-keys";
- /** Upper bound on live idempotency keys stored on a single entity. */
- public static final int MAX_WINDOW_SIZE = 64;
-
/** Magic prefix for the version 1 SMILE window ({@code IS1} + base64url
bytes). */
- private static final String WINDOW_FORMAT_SMILE_V1 = "IS1";
+ private static final String FORMAT_SMILE_V1 = "IS1";
+
+ /**
+ * Safety ceiling on the number of live keys retained on a single entity.
Unlike a count-based
+ * eviction cap, this never drops a live key: when the window is full the
write fails instead, so
+ * a key that could still be retried is never silently discarded. The limit
is deliberately high —
+ * reaching it requires this many successful commits to one table within the
key TTL — so
+ * legitimate workloads never hit it, while still bounding stored
idempotency data against
+ * unbounded growth (a DoS guardrail).
+ */
+ private static final int MAX_LIVE_KEYS = 10_000;
Review Comment:
JB, good questions. 10K is up boundary to avoid DoS per discussion with
@dimas-b. The main knob to control the volume is the TTL.
@huaxingao also did a bunch of benchmark in this PR,
https://github.com/apache/polaris/pull/4912#issuecomment-4814094846. Please
check it out.
--
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]