Respond with 409 in case of concurrent Namespace update failures instead of 500

polaris-core/src/main/java/org/apache/polaris/core/exceptions/CommitConflictException.java

@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+public class CommitConflictException extends PolarisException {
+  @FormatMethod
+  public CommitConflictException(String message, Object... args) {
+    super(String.format(message, args));
+  }
+
+  @FormatMethod
+  public CommitConflictException(Throwable cause, String message, Object... args) {
+    super(String.format(message, args), cause);
+  }
+
+  public CommitConflictException(String message, Throwable cause) {
+    super(message, cause);
+  }
+}

service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

@@ -104,6 +104,7 @@
 import org.apache.polaris.core.entity.PolarisEntityType;
 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.persistence.PolarisMetaStoreManager;
 import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
 import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
@@ -715,7 +716,7 @@ public boolean setProperties(Namespace namespace, Map<String, String> properties
             .map(PolarisEntity::new)
             .orElse(null);
     if (returnedEntity == null) {
-      throw new RuntimeException("Concurrent modification of namespace: " + namespace);
+      throw new CommitConflictException("Concurrent modification of namespace: %s", namespace);
     }
     return true;
   }
@@ -747,7 +748,7 @@ public boolean removeProperties(Namespace namespace, Set<String> properties)
             .map(PolarisEntity::new)
             .orElse(null);
     if (returnedEntity == null) {
-      throw new RuntimeException("Concurrent modification of namespace: " + namespace);
+      throw new CommitConflictException("Concurrent modification of namespace: %s", namespace);
     }
     return true;
   }

service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java

@@ -24,6 +24,7 @@
 import jakarta.ws.rs.ext.Provider;
 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.persistence.PolicyMappingAlreadyExistsException;
 import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
@@ -45,23 +46,19 @@ public class PolarisExceptionMapper implements ExceptionMapper<PolarisException>
   private static final Logger LOGGER = LoggerFactory.getLogger(PolarisExceptionMapper.class);
 
   private Response.Status getStatus(PolarisException exception) {
-    if (exception instanceof AlreadyExistsException) {
-      return Response.Status.CONFLICT;
-    } else if (exception instanceof InvalidPolicyException) {
-      return Response.Status.BAD_REQUEST;
-    } else if (exception instanceof PolicyAttachException) {
-      return Response.Status.BAD_REQUEST;
-    } else if (exception instanceof NoSuchPolicyException) {
-      return Response.Status.NOT_FOUND;
-    } else if (exception instanceof PolicyVersionMismatchException) {
-      return Response.Status.CONFLICT;
-    } else if (exception instanceof PolicyMappingAlreadyExistsException) {
-      return Response.Status.CONFLICT;
-    } else if (exception instanceof PolicyInUseException) {
-      return Response.Status.BAD_REQUEST;
-    } else {
-      return Response.Status.INTERNAL_SERVER_ERROR;
-    }
+    return switch (exception) {
+      case AlreadyExistsException alreadyExistsException -> Response.Status.CONFLICT;
+      case CommitConflictException commitConflictException -> Response.Status.CONFLICT;
+      case InvalidPolicyException invalidPolicyException -> Response.Status.BAD_REQUEST;
+      case PolicyAttachException policyAttachException -> Response.Status.BAD_REQUEST;
+      case NoSuchPolicyException noSuchPolicyException -> Response.Status.NOT_FOUND;
+      case PolicyVersionMismatchException policyVersionMismatchException ->
+          Response.Status.CONFLICT;
+      case PolicyMappingAlreadyExistsException policyMappingAlreadyExistsException ->
+          Response.Status.CONFLICT;
+      case PolicyInUseException policyInUseException -> Response.Status.BAD_REQUEST;
+      default -> Response.Status.INTERNAL_SERVER_ERROR;
+    };
   }
 
   @Override

service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java

@@ -25,11 +25,14 @@
 import ch.qos.logback.core.read.ListAppender;
 import com.fasterxml.jackson.core.JsonLocation;
 import com.fasterxml.jackson.core.JsonProcessingException;
+import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.ext.ExceptionMapper;
 import java.util.Optional;
 import java.util.stream.Stream;
 import org.apache.polaris.core.exceptions.AlreadyExistsException;
+import org.apache.polaris.core.exceptions.CommitConflictException;
 import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -74,6 +77,13 @@ public void testFullExceptionIsLogged(
         .isTrue();
   }
 
+  @Test
+  public void testNamespaceException() {
+    PolarisExceptionMapper mapper = new PolarisExceptionMapper();
+    Response response = mapper.toResponse(new CommitConflictException("test"));
+    Assertions.assertThat(response.getStatus()).isEqualTo(409);
+  }
+
   static Stream<Arguments> testFullExceptionIsLogged() {
     // ConstraintViolationException isn't included because it doesn't propagate any info to its
     // inherited Exception