huaxingao commented on code in PR #4659:
URL: https://github.com/apache/polaris/pull/4659#discussion_r3392662554
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -460,15 +488,167 @@ public void authorizeCreateTableDirect(
}
}
+ /**
+ * Create a table, optionally honoring an {@code Idempotency-Key} from the
REST request.
+ *
+ * <p>When an idempotency key is supplied and the feature is enabled:
+ *
+ * <ol>
+ * <li>Authorization runs first, so idempotency cannot bypass it.
+ * <li>Pre-flight loads any prior record for {@code (realm, key)}. A match
(same caller, same
+ * resource binding) replays the response from authoritative catalog
state; a binding
+ * mismatch raises 422.
+ * <li>On a fresh key, the table is created and the record is inserted
afterwards. A concurrent
+ * caller that wins the race causes our insert to return DUPLICATE —
we then replay too, so
+ * the response is equivalent to what the winner returned.
+ * </ol>
+ *
+ * <p>No response body is stored. Replays go through {@code loadTable +
+ * buildLoadTableResponseWithDelegationCredentials}, which re-vends fresh
credentials for the
+ * current caller.
+ */
public LoadTableResponse createTableDirect(
Namespace namespace,
CreateTableRequest request,
EnumSet<AccessDelegationMode> delegationModes,
- Optional<String> refreshCredentialsEndpoint) {
+ Optional<String> refreshCredentialsEndpoint,
+ Optional<String> idempotencyKey) {
authorizeCreateTableDirect(namespace, request, !delegationModes.isEmpty());
Optional<AccessDelegationMode> resolvedMode =
resolveAccessDelegationModes(delegationModes);
+ if (idempotencyKey.isEmpty() || !idempotencySupport().isEnabled()) {
+ return doCreateTableDirect(namespace, request, resolvedMode,
refreshCredentialsEndpoint);
+ }
+ return createTableDirectIdempotently(
+ namespace,
+ request,
+ delegationModes,
+ resolvedMode,
+ refreshCredentialsEndpoint,
+ idempotencyKey.get());
+ }
+
+ private LoadTableResponse createTableDirectIdempotently(
+ Namespace namespace,
+ CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes,
+ Optional<AccessDelegationMode> resolvedMode,
+ Optional<String> refreshCredentialsEndpoint,
+ String idempotencyKey) {
+
+ String operationType = "create-table";
+ String principalHash =
+ idempotencySupport().principalHash(polarisPrincipal(),
realmContext().getRealmIdentifier());
+ String resourceHash =
+ idempotencySupport()
+ .resourceHash(
+ "create-table:"
+ + namespace.toString()
+ + ":"
+ + request.name()
+ + ":"
+ + delegationModesToken(delegationModes));
Review Comment:
Done — binding now uses `resolvedMode` instead of the full requested modes.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -460,15 +488,167 @@ public void authorizeCreateTableDirect(
}
}
+ /**
+ * Create a table, optionally honoring an {@code Idempotency-Key} from the
REST request.
+ *
+ * <p>When an idempotency key is supplied and the feature is enabled:
+ *
+ * <ol>
+ * <li>Authorization runs first, so idempotency cannot bypass it.
+ * <li>Pre-flight loads any prior record for {@code (realm, key)}. A match
(same caller, same
+ * resource binding) replays the response from authoritative catalog
state; a binding
+ * mismatch raises 422.
+ * <li>On a fresh key, the table is created and the record is inserted
afterwards. A concurrent
+ * caller that wins the race causes our insert to return DUPLICATE —
we then replay too, so
+ * the response is equivalent to what the winner returned.
+ * </ol>
+ *
+ * <p>No response body is stored. Replays go through {@code loadTable +
+ * buildLoadTableResponseWithDelegationCredentials}, which re-vends fresh
credentials for the
+ * current caller.
+ */
public LoadTableResponse createTableDirect(
Namespace namespace,
CreateTableRequest request,
EnumSet<AccessDelegationMode> delegationModes,
- Optional<String> refreshCredentialsEndpoint) {
+ Optional<String> refreshCredentialsEndpoint,
+ Optional<String> idempotencyKey) {
authorizeCreateTableDirect(namespace, request, !delegationModes.isEmpty());
Optional<AccessDelegationMode> resolvedMode =
resolveAccessDelegationModes(delegationModes);
+ if (idempotencyKey.isEmpty() || !idempotencySupport().isEnabled()) {
+ return doCreateTableDirect(namespace, request, resolvedMode,
refreshCredentialsEndpoint);
+ }
+ return createTableDirectIdempotently(
+ namespace,
+ request,
+ delegationModes,
+ resolvedMode,
+ refreshCredentialsEndpoint,
+ idempotencyKey.get());
+ }
+
+ private LoadTableResponse createTableDirectIdempotently(
+ Namespace namespace,
+ CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes,
+ Optional<AccessDelegationMode> resolvedMode,
+ Optional<String> refreshCredentialsEndpoint,
+ String idempotencyKey) {
+
+ String operationType = "create-table";
+ String principalHash =
+ idempotencySupport().principalHash(polarisPrincipal(),
realmContext().getRealmIdentifier());
+ String resourceHash =
+ idempotencySupport()
+ .resourceHash(
+ "create-table:"
+ + namespace.toString()
+ + ":"
+ + request.name()
+ + ":"
+ + delegationModesToken(delegationModes));
+
+ // Pre-flight: replay a prior success, or 422 on a binding mismatch (same
key, different
+ // request/principal).
+ try {
+ IdempotencyHandlerSupport.Outcome preflight =
+ idempotencySupport()
+ .preflight(idempotencyKey, operationType, resourceHash,
principalHash);
+ if (preflight instanceof IdempotencyHandlerSupport.Outcome.Duplicate
dup) {
+ return replayCreateTableDirect(
+ namespace, request, resolvedMode, refreshCredentialsEndpoint,
dup.existing());
+ }
+ } catch (IdempotencyHandlerSupport.ConflictException e) {
+ throw new UnprocessableEntityException("%s", e.getMessage());
+ }
+
+ // Run the operation. A concurrent request carrying the same key can win
the catalog-level race
+ // and make this attempt fail with AlreadyExistsException; if that winner
recorded a matching
+ // idempotency outcome, replay it instead of returning a 409.
+ LoadTableResponse response;
+ try {
+ response = doCreateTableDirect(namespace, request, resolvedMode,
refreshCredentialsEndpoint);
+ } catch (AlreadyExistsException e) {
+ try {
+ Optional<IdempotencyRecord> raceWinner =
+ resolveConcurrentDuplicate(idempotencyKey, operationType,
resourceHash, principalHash);
+ if (raceWinner.isPresent()) {
+ return replayCreateTableDirect(
Review Comment:
Extracted a shared private helper (`buildLoadTableResponseForExistingTable`)
used by both `loadTable`(...) and the replay path, with an optional
`expectedMetadataLocation` param for the divergence check. Kept it
private/authz-free so replay doesn't pick up an extra LOAD_TABLE check.
--
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]
