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]

Reply via email to