Copilot commented on code in PR #5122:
URL: https://github.com/apache/polaris/pull/5122#discussion_r3624906865
##########
runtime/service/src/main/java/org/apache/polaris/service/idempotency/EntityIdempotency.java:
##########
@@ -110,10 +120,11 @@ public static Map<String, String> recordKey(
}
}
- // Bounded window: drop the earliest-expiring entries (front of the sorted
list) to make room
- // *before* inserting, so the key just recorded is always retained even
when every entry shares
- // the same expiry (e.g. a burst of writes at one instant) and expiry
order can't rank recency.
- while (window.size() >= MAX_WINDOW_SIZE) {
+ // Bounded window, sized from this key's TTL. Drop the earliest-expiring
entries (front of the
+ // sorted list) *before* inserting so the key just recorded is always
retained. This matters
+ // when entries share an expiry (e.g. a burst of writes) and expiry order
can't rank recency.
+ int maxWindowSize = maxWindowSize(Duration.between(now, expiry));
+ while (window.size() >= maxWindowSize) {
Review Comment:
maxWindowSize is currently computed from `Duration.between(now, expiry)`,
which is the *remaining* time-to-expiry at the moment `recordKey` is called. In
the main call site (`IdempotencyRequestContext` computes `pendingExpiry`
up-front, but `LocalIcebergCatalog` passes a later `Instant.now()`), the
effective cap will shrink as the request runs (e.g., a 5m configured TTL
becomes a 1m remaining TTL -> cap ~12), causing premature eviction and
contradicting the intent of scaling with the configured TTL.
--
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]
