You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by di...@apache.org on 2026/07/24 17:59:31 UTC
(polaris) branch main updated: Set 'Retry-After' header when table rename fails with a concurrent modification error (#5144)
This is an automated email from the ASF dual-hosted git repository.
dimas-b pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new ab6fd47a5d Set 'Retry-After' header when table rename fails with a concurrent modification error (#5144)
ab6fd47a5d is described below
commit ab6fd47a5d6a9b3e4c1ce424b926424433ae6648
Author: Nándor Kollár <na...@users.noreply.github.com>
AuthorDate: Fri Jul 24 19:59:25 2026 +0200
Set 'Retry-After' header when table rename fails with a concurrent modification error (#5144)
* Set 'Retry-After' header when table rename fails with a concurrent modification
* Mention Retry-After header in the changelog
---
CHANGELOG.md | 2 +-
.../PolarisServiceUnavailableException.java | 40 ++++++++++++++++++++++
.../catalog/iceberg/LocalIcebergCatalog.java | 9 +++--
.../service/exception/PolarisExceptionMapper.java | 15 +++++---
.../iceberg/AbstractLocalIcebergCatalogTest.java | 4 +--
.../service/exception/ExceptionMapperTest.java | 11 ++++++
6 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1370fefa19..c1634024df 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -143,7 +143,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- The admin tool `purge` command now prints the underlying exception stack trace to stderr when a purge fails unexpectedly, matching the `bootstrap` command. Previously a failed purge printed only a generic message, giving operators no diagnostic information.
- The access token carried in `PolarisCredential` is now redacted from its `toString()`, so it is no longer written to authentication logs (including at WARN level on failed authentication).
- JWT verification now validates the issuer claim (`"polaris"`) in addition to the active claim. Tokens signed with the same key but carrying a different issuer are now rejected.
-- Renaming a table or view now maps concurrent-modification and resolution failures to meaningful HTTP status codes instead of HTTP 500. A concurrent modification of the source returns 503 (Service Unavailable, retryable); a source or target path that no longer resolves (concurrently dropped or replaced) returns 404 (Not Found). HTTP 409 is intentionally not used because the Iceberg REST rename endpoint reserves 409 for "the target already exists".
+- Renaming a table or view now maps concurrent-modification and resolution failures to meaningful HTTP status codes instead of HTTP 500. A concurrent modification of the source returns 503 (Service Unavailable, retryable) with a `Retry-After: 3` header indicating the client should wait 3 seconds before retrying; a source or target path that no longer resolves (concurrently dropped or replaced) returns 404 (Not Found). HTTP 409 is intentionally not used because the Iceberg REST rename end [...]
- Inheritable policy mapping inserts in the JDBC backend now use the active transaction connection, so they roll back correctly with the surrounding transaction.
- Generic table drop now accepts table-scoped `TABLE_DROP` privilege.
- Azure SAS tokens are now signed for the configured duration instead of a hardcoded 1 hour, so long jobs no longer fail with expired credentials.
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisServiceUnavailableException.java b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisServiceUnavailableException.java
new file mode 100644
index 0000000000..5479fd9f94
--- /dev/null
+++ b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisServiceUnavailableException.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+
+/**
+ * Signals a transient failure that the client may resolve by retrying. Mapped to HTTP 503 (Service
+ * Unavailable) with a {@code Retry-After} header whose value is {@link #getRetryAfterSeconds()}.
+ */
+public class PolarisServiceUnavailableException extends PolarisException {
+
+ private final int retryAfterSeconds;
+
+ @FormatMethod
+ public PolarisServiceUnavailableException(int retryAfterSeconds, String message, Object... args) {
+ super(String.format(message, args));
+ this.retryAfterSeconds = retryAfterSeconds;
+ }
+
+ public int getRetryAfterSeconds() {
+ return retryAfterSeconds;
+ }
+}
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
index 47c972356c..f7545bfc50 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
@@ -80,7 +80,6 @@ import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.exceptions.NotFoundException;
import org.apache.iceberg.exceptions.ServiceFailureException;
-import org.apache.iceberg.exceptions.ServiceUnavailableException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.io.CloseableGroup;
import org.apache.iceberg.io.FileIO;
@@ -117,6 +116,7 @@ import org.apache.polaris.core.entity.PolarisEntityUtils;
import org.apache.polaris.core.entity.PolarisTaskConstants;
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
import org.apache.polaris.core.exceptions.CommitConflictException;
+import org.apache.polaris.core.exceptions.PolarisServiceUnavailableException;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
@@ -2745,9 +2745,12 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
// Surface as 503 so clients can retry. We avoid 409 because the rename endpoint reserves
// 409 for "target already exists" (handled by the ENTITY_ALREADY_EXISTS case above).
case BaseResult.ReturnStatus.TARGET_ENTITY_CONCURRENTLY_MODIFIED:
- throw new ServiceUnavailableException(
+ // TODO: make the value of 'Retry-After' header configurable
+ throw new PolarisServiceUnavailableException(
+ 3,
"Cannot rename %s to %s because it was concurrently modified; please retry",
- from, to);
+ from,
+ to);
// some entities cannot be renamed
case BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RENAMED:
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java b/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
index 1e9926bfd7..97d77de7db 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
@@ -19,6 +19,7 @@
package org.apache.polaris.service.exception;
import com.google.common.annotations.VisibleForTesting;
+import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
@@ -27,6 +28,7 @@ import org.apache.iceberg.rest.responses.ErrorResponse;
import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.exceptions.CommitConflictException;
import org.apache.polaris.core.exceptions.PolarisException;
+import org.apache.polaris.core.exceptions.PolarisServiceUnavailableException;
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
import org.apache.polaris.core.policy.exceptions.PolicyAttachException;
@@ -48,6 +50,8 @@ public class PolarisExceptionMapper implements ExceptionMapper<PolarisException>
private Response.Status getStatus(PolarisException exception) {
return switch (exception) {
+ case PolarisServiceUnavailableException polarisServiceUnavailableException ->
+ Response.Status.SERVICE_UNAVAILABLE;
case AlreadyExistsException alreadyExistsException -> Response.Status.CONFLICT;
case CommitConflictException commitConflictException -> Response.Status.CONFLICT;
case InvalidPolicyException invalidPolicyException -> Response.Status.BAD_REQUEST;
@@ -77,10 +81,13 @@ public class PolarisExceptionMapper implements ExceptionMapper<PolarisException>
.withType(exception.getClass().getSimpleName())
.withMessage(exception.getMessage())
.build();
- return Response.status(status)
- .entity(errorResponse)
- .type(MediaType.APPLICATION_JSON_TYPE)
- .build();
+ Response.ResponseBuilder builder =
+ Response.status(status).entity(errorResponse).type(MediaType.APPLICATION_JSON_TYPE);
+ if (exception instanceof PolarisServiceUnavailableException e
+ && e.getRetryAfterSeconds() != 0) {
+ builder.header(HttpHeaders.RETRY_AFTER, e.getRetryAfterSeconds());
+ }
+ return builder.build();
}
@VisibleForTesting
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
index ef19eec4cb..14a5759396 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
@@ -95,7 +95,6 @@ import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NotFoundException;
import org.apache.iceberg.exceptions.ServiceFailureException;
-import org.apache.iceberg.exceptions.ServiceUnavailableException;
import org.apache.iceberg.inmemory.InMemoryFileIO;
import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.io.FileIO;
@@ -127,6 +126,7 @@ import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.TaskEntity;
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
import org.apache.polaris.core.exceptions.CommitConflictException;
+import org.apache.polaris.core.exceptions.PolarisServiceUnavailableException;
import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
@@ -2874,7 +2874,7 @@ public abstract class AbstractLocalIcebergCatalogTest extends CatalogTests<Local
// Transient conflict: entity present but concurrently modified -> 503, retryable.
Arguments.of(
BaseResult.ReturnStatus.TARGET_ENTITY_CONCURRENTLY_MODIFIED,
- ServiceUnavailableException.class),
+ PolarisServiceUnavailableException.class),
// Source path could not be resolved (e.g. concurrently dropped) -> 404, not retryable.
Arguments.of(
BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, NoSuchNamespaceException.class),
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java b/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
index f37d34db55..4520b7bf84 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
@@ -29,6 +29,7 @@ import com.fasterxml.jackson.core.JsonGenerationException;
import com.fasterxml.jackson.core.JsonLocation;
import com.fasterxml.jackson.core.JsonProcessingException;
import jakarta.ws.rs.ForbiddenException;
+import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import java.io.IOException;
@@ -37,6 +38,7 @@ import java.util.stream.Stream;
import org.apache.iceberg.exceptions.RuntimeIOException;
import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.exceptions.CommitConflictException;
+import org.apache.polaris.core.exceptions.PolarisServiceUnavailableException;
import org.jboss.logmanager.Level;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -100,6 +102,15 @@ public class ExceptionMapperTest {
assertThat(response.getStatus()).isEqualTo(409);
}
+ @Test
+ public void testServiceUnavailableWithRetryAfter() {
+ PolarisExceptionMapper mapper = new PolarisExceptionMapper();
+ Response response =
+ mapper.toResponse(new PolarisServiceUnavailableException(3, "transient conflict"));
+ assertThat(response.getStatus()).isEqualTo(503);
+ assertThat(response.getHeaderString(HttpHeaders.RETRY_AFTER)).isEqualTo("3");
+ }
+
static Stream<Arguments> testFullExceptionIsLogged() {
// ConstraintViolationException isn't included because it doesn't propagate any info to its
// inherited Exception