dimas-b commented on code in PR #3915:
URL: https://github.com/apache/polaris/pull/3915#discussion_r2906889647


##########
polaris-core/src/main/java/org/apache/polaris/core/entity/IdempotencyRecord.java:
##########
@@ -112,13 +112,12 @@ public String responseSummary() {
   }
 
   /**
-   * Serialized representation of a small, whitelisted set of HTTP response 
headers.
+   * Allowlisted snapshot of HTTP response headers to replay for duplicates.
    *
-   * <p>Stored as a JSON string so that the HTTP layer can replay key headers 
(such as {@code
-   * Content-Type}) when serving a duplicate idempotent request.
+   * <p>How this map is persisted is store-specific.
    */
   @Override
-  public String responseHeaders() {
+  public Map<String, String> responseHeaders() {

Review Comment:
   Thanks for the context, @huaxingao !
   
   IMHO, this still sounds like we cannot know beforehand what the client or 
the API layer in the server may need to support for the response headers. Since 
HTTP allows multi-values headers, I tend to this the Persistence SPI should 
support that from day 1. Adding multi-valued entries later will be much harder, 
I think.



##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/IdempotencyStore.java:
##########
@@ -131,29 +132,53 @@ ReserveResult reserve(
   HeartbeatResult updateHeartbeat(
       String realmId, String idempotencyKey, String executorId, Instant now);
 
+  /**
+   * Cancels (deletes) an in-progress idempotency reservation owned by {@code 
executorId}.
+   *
+   * <p>This is used for cases where an idempotency key was reserved but the 
request should not be
+   * tracked for replay (for example, 401/403/408/429 responses per the 
proposal). Implementations
+   * should only cancel the record if it is still in-progress (i.e., not 
finalized) and owned by the
+   * given executor.
+   *
+   * @param realmId logical tenant or realm identifier
+   * @param idempotencyKey application-provided idempotency key
+   * @param executorId identifier of the executor that owns the reservation
+   * @return {@code true} if a record was cancelled, {@code false} otherwise

Review Comment:
   thx!



-- 
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