flyingImer commented on code in PR #3915:
URL: https://github.com/apache/polaris/pull/3915#discussion_r2908479948
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/idempotency/RelationalJdbcIdempotencyStore.java:
##########
@@ -190,27 +195,67 @@ public HeartbeatResult updateHeartbeat(
return HeartbeatResult.LOST_OWNERSHIP;
}
+ @Override
+ public void cancelInProgressReservation(
+ String realmId, String idempotencyKey, String executorId) {
+ try {
+ QueryGenerator.PreparedQuery delete =
+ QueryGenerator.generateDeleteQuery(
+ ModelIdempotencyRecord.ALL_COLUMNS,
+ ModelIdempotencyRecord.TABLE_NAME,
+ Map.of(
+ ModelIdempotencyRecord.REALM_ID,
+ realmId,
+ ModelIdempotencyRecord.IDEMPOTENCY_KEY,
+ idempotencyKey,
+ ModelIdempotencyRecord.EXECUTOR_ID,
+ executorId),
+ Map.of(),
+ Map.of(),
+ Set.of(ModelIdempotencyRecord.HTTP_STATUS),
+ Set.of());
+ datasourceOperations.executeUpdate(delete);
+ } catch (SQLException e) {
+ throw new IdempotencyPersistenceException(
Review Comment:
I think there are two high-priority issues worth fixing before this moves
forward. First, the Postgres IT still uses `Map.of("Content-Type",
"application/json")` even though `finalizeRecord()` now expects `Map<String,
List<String>>`, which looks like a straight compile/type mismatch. Second, the
SPI docs describe `cancelInProgressReservation()` as best-effort and explicitly
say failures should not affect the response path, but the JDBC impl now throws
on cancel failure. Would it make sense to either make the JDBC path
log-and-swallow here, or tighten the SPI contract + callers so the boundary is
explicit?
--
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]
