huaxingao commented on code in PR #3205:
URL: https://github.com/apache/polaris/pull/3205#discussion_r2665799403
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/IdempotencyStore.java:
##########
@@ -44,6 +44,35 @@ enum ReserveResultType {
DUPLICATE
}
+ /**
+ * Result of attempting to update the heartbeat for an in-progress
idempotency record.
+ *
+ * <p>This allows callers to distinguish between different "no update"
scenarios instead of
+ * overloading them into a boolean.
+ */
+ enum HeartbeatResult {
+ /** The heartbeat was successfully updated for the in-progress record
owned by this executor. */
+ UPDATED,
+
+ /**
+ * The idempotency record exists but has already been finalized with an
HTTP status; there is no
+ * longer an in-progress reservation to heartbeat.
+ */
+ FINALIZED,
+
+ /**
+ * No idempotency record exists for the specified realm and key. This can
happen if the record
+ * was never created or has already been purged.
+ */
+ NOT_FOUND,
+
+ /**
+ * An in-progress idempotency record exists for the key, but it is owned
by a different
+ * executor. The caller should stop heartbeating as it no longer owns the
reservation.
+ */
+ LOST_OWNERSHIP
Review Comment:
Good question. I agree that a mismatched executorId shouldn’t be part of the
normal path.
In the intended model, a single executor reserves the key and keeps using
the same executorId for its retries, so other executors should see `DUPLICATE`
at reserve time and never call `updateHeartbeat` for that key. In that sense,
`LOST_OWNERSHIP` is mostly a defensive / diagnostic state.
--
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]
