Skip to content

Commit cfbcfed

Browse files
tmatertmater
committed
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
1 parent e59281a commit cfbcfed

File tree

5 files changed

+17
-16
lines changed

5 files changed

+17
-16
lines changed

‎runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
import org.apache.polaris.core.entity.PolarisEntityType;
107107
import org.apache.polaris.core.entity.PrincipalEntity;
108108
import org.apache.polaris.core.entity.TaskEntity;
109+
import org.apache.polaris.core.exceptions.CommitConflictException;
109110
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
110111
import org.apache.polaris.core.persistence.PolarisEntityManager;
111112
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
@@ -2127,7 +2128,7 @@ public void testConcurrencyConflictUpdateTableDuringFinalTransaction() {
21272128
Schema expected = update.apply();
21282129

21292130
Assertions.assertThatThrownBy(() -> update.commit())
2130-
.isInstanceOf(CommitFailedException.class)
2131+
.isInstanceOf(CommitConflictException.class)
21312132
.hasMessageContaining("conflict_table");
21322133
}
21332134

‎service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.apache.iceberg.catalog.TableIdentifier;
4242
import org.apache.iceberg.exceptions.AlreadyExistsException;
4343
import org.apache.iceberg.exceptions.BadRequestException;
44-
import org.apache.iceberg.exceptions.CommitFailedException;
4544
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
4645
import org.apache.iceberg.exceptions.NotFoundException;
4746
import org.apache.iceberg.exceptions.ValidationException;
@@ -92,6 +91,7 @@
9291
import org.apache.polaris.core.entity.PrincipalRoleEntity;
9392
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
9493
import org.apache.polaris.core.entity.table.federated.FederatedEntities;
94+
import org.apache.polaris.core.exceptions.CommitConflictException;
9595
import org.apache.polaris.core.persistence.PolarisEntityManager;
9696
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
9797
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
@@ -880,7 +880,7 @@ private void validateUpdateCatalogDiffOrThrow(
880880
.orElseThrow(() -> new NotFoundException("Catalog %s not found", name));
881881

882882
if (currentCatalogEntity.getEntityVersion() != updateRequest.getCurrentEntityVersion()) {
883-
throw new CommitFailedException(
883+
throw new CommitConflictException(
884884
"Failed to update Catalog; currentEntityVersion '%s', expected '%s'",
885885
currentCatalogEntity.getEntityVersion(), updateRequest.getCurrentEntityVersion());
886886
}
@@ -932,7 +932,7 @@ private void validateUpdateCatalogDiffOrThrow(
932932
getCurrentPolarisContext(), null, updatedEntity))))
933933
.orElseThrow(
934934
() ->
935-
new CommitFailedException(
935+
new CommitConflictException(
936936
"Concurrent modification on Catalog '%s'; retry later", name));
937937
return returnedEntity;
938938
}
@@ -1048,7 +1048,7 @@ public void deletePrincipal(String name) {
10481048
"Cannot update a federated principal: %s", currentPrincipalEntity.getName());
10491049
}
10501050
if (currentPrincipalEntity.getEntityVersion() != updateRequest.getCurrentEntityVersion()) {
1051-
throw new CommitFailedException(
1051+
throw new CommitConflictException(
10521052
"Failed to update Principal; currentEntityVersion '%s', expected '%s'",
10531053
currentPrincipalEntity.getEntityVersion(), updateRequest.getCurrentEntityVersion());
10541054
}
@@ -1069,7 +1069,7 @@ public void deletePrincipal(String name) {
10691069
getCurrentPolarisContext(), null, updatedEntity))))
10701070
.orElseThrow(
10711071
() ->
1072-
new CommitFailedException(
1072+
new CommitConflictException(
10731073
"Concurrent modification on Principal '%s'; retry later", name));
10741074
return returnedEntity;
10751075
}
@@ -1220,7 +1220,7 @@ public void deletePrincipalRole(String name) {
12201220
.orElseThrow(() -> new NotFoundException("PrincipalRole %s not found", name));
12211221

12221222
if (currentPrincipalRoleEntity.getEntityVersion() != updateRequest.getCurrentEntityVersion()) {
1223-
throw new CommitFailedException(
1223+
throw new CommitConflictException(
12241224
"Failed to update PrincipalRole; currentEntityVersion '%s', expected '%s'",
12251225
currentPrincipalRoleEntity.getEntityVersion(), updateRequest.getCurrentEntityVersion());
12261226
}
@@ -1242,7 +1242,7 @@ public void deletePrincipalRole(String name) {
12421242
getCurrentPolarisContext(), null, updatedEntity))))
12431243
.orElseThrow(
12441244
() ->
1245-
new CommitFailedException(
1245+
new CommitConflictException(
12461246
"Concurrent modification on PrincipalRole '%s'; retry later", name));
12471247
return returnedEntity;
12481248
}
@@ -1347,7 +1347,7 @@ public void deleteCatalogRole(String catalogName, String name) {
13471347
.orElseThrow(() -> new NotFoundException("CatalogRole %s not found", name));
13481348

13491349
if (currentCatalogRoleEntity.getEntityVersion() != updateRequest.getCurrentEntityVersion()) {
1350-
throw new CommitFailedException(
1350+
throw new CommitConflictException(
13511351
"Failed to update CatalogRole; currentEntityVersion '%s', expected '%s'",
13521352
currentCatalogRoleEntity.getEntityVersion(), updateRequest.getCurrentEntityVersion());
13531353
}
@@ -1371,7 +1371,7 @@ public void deleteCatalogRole(String catalogName, String name) {
13711371
updatedEntity))))
13721372
.orElseThrow(
13731373
() ->
1374-
new CommitFailedException(
1374+
new CommitConflictException(
13751375
"Concurrent modification on CatalogRole '%s'; retry later", name));
13761376
return returnedEntity;
13771377
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) {
23542354
throw new NotFoundException("Parent path does not exist for %s", identifier);
23552355

23562356
case BaseResult.ReturnStatus.TARGET_ENTITY_CONCURRENTLY_MODIFIED:
2357-
throw new CommitFailedException(
2357+
throw new CommitConflictException(
23582358
"Failed to commit Table or View %s because it was concurrently modified", identifier);
23592359

23602360
default:

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
import org.apache.iceberg.catalog.ViewCatalog;
5252
import org.apache.iceberg.exceptions.AlreadyExistsException;
5353
import org.apache.iceberg.exceptions.BadRequestException;
54-
import org.apache.iceberg.exceptions.CommitFailedException;
5554
import org.apache.iceberg.exceptions.ForbiddenException;
5655
import org.apache.iceberg.exceptions.NoSuchTableException;
5756
import org.apache.iceberg.hadoop.HadoopCatalog;
@@ -84,6 +83,7 @@
8483
import org.apache.polaris.core.entity.CatalogEntity;
8584
import org.apache.polaris.core.entity.PolarisEntitySubType;
8685
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
86+
import org.apache.polaris.core.exceptions.CommitConflictException;
8787
import org.apache.polaris.core.persistence.PolarisEntityManager;
8888
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
8989
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
@@ -997,7 +997,7 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest)
997997
callContext.getPolarisCallContext(), pendingUpdates);
998998
if (!result.isSuccess()) {
999999
// TODO: Retries and server-side cleanup on failure
1000-
throw new CommitFailedException(
1000+
throw new CommitConflictException(
10011001
"Transaction commit failed with status: %s, extraInfo: %s",
10021002
result.getReturnStatus(), result.getExtraInformation());
10031003
}

‎spec/generated/bundled-polaris-catalog-service.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ paths:
635635
TableToUpdateDoesNotExist:
636636
$ref: '#/components/examples/NoSuchTableError'
637637
'409':
638-
description: Conflict - CommitFailedException, one or more requirements failed. The client may retry.
638+
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
639639
content:
640640
application/json:
641641
schema:
@@ -930,7 +930,7 @@ paths:
930930
TableToUpdateDoesNotExist:
931931
$ref: '#/components/examples/NoSuchTableError'
932932
'409':
933-
description: Conflict - CommitFailedException, one or more requirements failed. The client may retry.
933+
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
934934
content:
935935
application/json:
936936
schema:
@@ -1140,7 +1140,7 @@ paths:
11401140
ViewToUpdateDoesNotExist:
11411141
$ref: '#/components/examples/NoSuchViewError'
11421142
'409':
1143-
description: Conflict - CommitFailedException. The client may retry.
1143+
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
11441144
content:
11451145
application/json:
11461146
schema:

0 commit comments

Comments
 (0)