Skip to content

Replace CommitFailedException with CommitConflictException #2198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dimas-b merged 3 commits into from
Jul 31, 2025
Merged

Replace CommitFailedException with CommitConflictException #2198

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cfbcfed
Replace CommitFailedException with CommitConflictException
tmater Jul 28, 2025
09546ee
Review fix
tmater Jul 29, 2025
81efaca
Revert bundled-polaris-catalog-service.yaml
tmater Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractIcebergCatalogTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
import org.apache.polaris.core.entity.PolarisEntityType;
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;
Expand Down Expand Up @@ -2127,7 +2128,7 @@ public void testConcurrencyConflictUpdateTableDuringFinalTransaction() {
Schema expected = update.apply();

Assertions.assertThatThrownBy(() -> update.commit())
.isInstanceOf(CommitFailedException.class)
.isInstanceOf(CommitConflictException.class)
.hasMessageContaining("conflict_table");
}

Expand Down
18 changes: 9 additions & 9 deletions service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
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;
Expand Down Expand Up @@ -92,6 +91,7 @@
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;
Expand Down Expand Up @@ -880,7 +880,7 @@ private void validateUpdateCatalogDiffOrThrow(
.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());
}
Expand Down Expand Up @@ -932,7 +932,7 @@ private void validateUpdateCatalogDiffOrThrow(
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
new CommitFailedException(
new CommitConflictException(
"Concurrent modification on Catalog '%s'; retry later", name));
return returnedEntity;
}
Expand Down Expand Up @@ -1048,7 +1048,7 @@ public void deletePrincipal(String name) {
"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());
}
Expand All @@ -1069,7 +1069,7 @@ public void deletePrincipal(String name) {
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
new CommitFailedException(
new CommitConflictException(
"Concurrent modification on Principal '%s'; retry later", name));
return returnedEntity;
}
Expand Down Expand Up @@ -1220,7 +1220,7 @@ public void deletePrincipalRole(String name) {
.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());
}
Expand All @@ -1242,7 +1242,7 @@ public void deletePrincipalRole(String name) {
getCurrentPolarisContext(), null, updatedEntity))))
.orElseThrow(
() ->
new CommitFailedException(
new CommitConflictException(
"Concurrent modification on PrincipalRole '%s'; retry later", name));
return returnedEntity;
}
Expand Down Expand Up @@ -1347,7 +1347,7 @@ public void deleteCatalogRole(String catalogName, String name) {
.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());
}
Expand All @@ -1371,7 +1371,7 @@ public void deleteCatalogRole(String catalogName, String name) {
updatedEntity))))
.orElseThrow(
() ->
new CommitFailedException(
new CommitConflictException(
"Concurrent modification on CatalogRole '%s'; retry later", name));
return returnedEntity;
}
Expand Down
2 changes: 1 addition & 1 deletion service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2354,7 +2354,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) {
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:
Expand Down
4 changes: 2 additions & 2 deletions ...ommon/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.apache.iceberg.catalog.ViewCatalog;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.BadRequestException;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.hadoop.HadoopCatalog;
Expand Down Expand Up @@ -84,6 +83,7 @@
import org.apache.polaris.core.entity.CatalogEntity;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
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;
Expand Down Expand Up @@ -997,7 +997,7 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest)
callContext.getPolarisCallContext(), pendingUpdates);
if (!result.isSuccess()) {
// TODO: Retries and server-side cleanup on failure
throw new CommitFailedException(
throw new CommitConflictException(
"Transaction commit failed with status: %s, extraInfo: %s",
result.getReturnStatus(), result.getExtraInformation());
Comment thread
tmater marked this conversation as resolved.
}
Expand Down
6 changes: 3 additions & 3 deletions spec/generated/bundled-polaris-catalog-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ paths:
TableToUpdateDoesNotExist:
$ref: '#/components/examples/NoSuchTableError'
'409':
description: Conflict - CommitFailedException, one or more requirements failed. The client may retry.
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
Comment thread
tmater marked this conversation as resolved.
Outdated
content:
application/json:
schema:
Expand Down Expand Up @@ -930,7 +930,7 @@ paths:
TableToUpdateDoesNotExist:
$ref: '#/components/examples/NoSuchTableError'
'409':
description: Conflict - CommitFailedException, one or more requirements failed. The client may retry.
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
content:
application/json:
schema:
Expand Down Expand Up @@ -1140,7 +1140,7 @@ paths:
ViewToUpdateDoesNotExist:
$ref: '#/components/examples/NoSuchViewError'
'409':
description: Conflict - CommitFailedException. The client may retry.
description: Conflict - CommitConflictException, one or more requirements failed. The client may retry.
content:
application/json:
schema:
Expand Down