This is an automated email from the ASF dual-hosted git repository.
dimas 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 5e43a07a2 Replace CommitFailedException with CommitConflictException
(#2198)
5e43a07a2 is described below
commit 5e43a07a216d4407b7031892ec274fe96661b10d
Author: Tamas Mate <50709850+tma...@users.noreply.github.com>
AuthorDate: Thu Jul 31 16:58:41 2025 +0200
Replace CommitFailedException with CommitConflictException (#2198)
* Replace CommitFailedException with CommitConflictException
In some cases, we were using CommitFailedException to represent commit
conflicts, which returns the correct 409 response but is tied to Iceberg.
However, some of these conflicts originate from Polaris, making
CommitConflictException a more appropriate and accurate choice.
This change updates those instances to improve clarity and exception
handling semantics.
Resolves #2168
---
.../quarkus/catalog/AbstractIcebergCatalogTest.java | 3 ++-
.../polaris/service/admin/PolarisAdminService.java | 18 +++++++++---------
.../service/catalog/iceberg/IcebergCatalog.java | 2 +-
.../service/catalog/iceberg/IcebergCatalogHandler.java | 2 +-
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java
index 6596513ea..05460d12e 100644
---
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java
+++
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java
@@ -105,6 +105,7 @@ import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.entity.TaskEntity;
+import org.apache.polaris.core.exceptions.CommitConflictException;
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisEntityManager;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
@@ -2112,7 +2113,7 @@ public abstract class AbstractIcebergCatalogTest extends
CatalogTests<IcebergCat
Schema expected = update.apply();
Assertions.assertThatThrownBy(() -> update.commit())
- .isInstanceOf(CommitFailedException.class)
+ .isInstanceOf(CommitConflictException.class)
.hasMessageContaining("conflict_table");
}
diff --git
a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
index 5003c78a2..59a05d614 100644
---
a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
+++
b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
@@ -41,7 +41,6 @@ import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.BadRequestException;
-import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NotFoundException;
import org.apache.iceberg.exceptions.ValidationException;
@@ -92,6 +91,7 @@ import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.entity.PrincipalRoleEntity;
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
import org.apache.polaris.core.entity.table.federated.FederatedEntities;
+import org.apache.polaris.core.exceptions.CommitConflictException;
import org.apache.polaris.core.persistence.PolarisEntityManager;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
@@ -880,7 +880,7 @@ public class PolarisAdminService {
.orElseThrow(() -> new NotFoundException("Catalog %s not found",
name));
if (currentCatalogEntity.getEntityVersion() !=
updateRequest.getCurrentEntityVersion()) {
- throw new CommitFailedException(
+ throw new CommitConflictException(
"Failed to update Catalog; currentEntityVersion '%s', expected '%s'",
currentCatalogEntity.getEntityVersion(),
updateRequest.getCurrentEntityVersion());
}
@@ -932,7 +932,7 @@ public class PolarisAdminService {
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
- new CommitFailedException(
+ new CommitConflictException(
"Concurrent modification on Catalog '%s'; retry
later", name));
return returnedEntity;
}
@@ -1048,7 +1048,7 @@ public class PolarisAdminService {
"Cannot update a federated principal: %s",
currentPrincipalEntity.getName());
}
if (currentPrincipalEntity.getEntityVersion() !=
updateRequest.getCurrentEntityVersion()) {
- throw new CommitFailedException(
+ throw new CommitConflictException(
"Failed to update Principal; currentEntityVersion '%s', expected
'%s'",
currentPrincipalEntity.getEntityVersion(),
updateRequest.getCurrentEntityVersion());
}
@@ -1069,7 +1069,7 @@ public class PolarisAdminService {
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
- new CommitFailedException(
+ new CommitConflictException(
"Concurrent modification on Principal '%s'; retry
later", name));
return returnedEntity;
}
@@ -1220,7 +1220,7 @@ public class PolarisAdminService {
.orElseThrow(() -> new NotFoundException("PrincipalRole %s not
found", name));
if (currentPrincipalRoleEntity.getEntityVersion() !=
updateRequest.getCurrentEntityVersion()) {
- throw new CommitFailedException(
+ throw new CommitConflictException(
"Failed to update PrincipalRole; currentEntityVersion '%s', expected
'%s'",
currentPrincipalRoleEntity.getEntityVersion(),
updateRequest.getCurrentEntityVersion());
}
@@ -1242,7 +1242,7 @@ public class PolarisAdminService {
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
- new CommitFailedException(
+ new CommitConflictException(
"Concurrent modification on PrincipalRole '%s'; retry
later", name));
return returnedEntity;
}
@@ -1347,7 +1347,7 @@ public class PolarisAdminService {
.orElseThrow(() -> new NotFoundException("CatalogRole %s not
found", name));
if (currentCatalogRoleEntity.getEntityVersion() !=
updateRequest.getCurrentEntityVersion()) {
- throw new CommitFailedException(
+ throw new CommitConflictException(
"Failed to update CatalogRole; currentEntityVersion '%s', expected
'%s'",
currentCatalogRoleEntity.getEntityVersion(),
updateRequest.getCurrentEntityVersion());
}
@@ -1371,7 +1371,7 @@ public class PolarisAdminService {
updatedEntity))))
.orElseThrow(
() ->
- new CommitFailedException(
+ new CommitConflictException(
"Concurrent modification on CatalogRole '%s'; retry
later", name));
return returnedEntity;
}
diff --git
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
index 4ccf5ad6c..310a3cd32 100644
---
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
+++
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
@@ -2357,7 +2357,7 @@ public class IcebergCatalog extends
BaseMetastoreViewCatalog
throw new NotFoundException("Parent path does not exist for %s",
identifier);
case BaseResult.ReturnStatus.TARGET_ENTITY_CONCURRENTLY_MODIFIED:
- throw new CommitFailedException(
+ throw new CommitConflictException(
"Failed to commit Table or View %s because it was concurrently
modified", identifier);
default:
diff --git
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
index d8ceea08d..ebcbd4bc7 100644
---
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
+++
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
@@ -996,7 +996,7 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
metaStoreManager.updateEntitiesPropertiesIfNotChanged(
callContext.getPolarisCallContext(), pendingUpdates);
if (!result.isSuccess()) {
- // TODO: Retries and server-side cleanup on failure
+ // TODO: Retries and server-side cleanup on failure, review possible
exceptions
throw new CommitFailedException(
"Transaction commit failed with status: %s, extraInfo: %s",
result.getReturnStatus(), result.getExtraInformation());
