vigneshio opened a new pull request, #4646:
URL: https://github.com/apache/polaris/pull/4646
When a concurrent modification occurs during `renameTable`/`renameView`, the
operation now returns HTTP 409 (Conflict) instead of HTTP 500 (Internal
Server
Error), so Iceberg REST clients retry as required by the spec.
## Why
`renameTableLike` in `IcebergCatalog` previously threw a bare
`RuntimeException`
for both `TARGET_ENTITY_CONCURRENTLY_MODIFIED` and
`ENTITY_CANNOT_BE_RESOLVED`.
`IcebergExceptionMapper` maps unknown `RuntimeException`s to 500, so clients
received an opaque server error instead of a retryable conflict. The code
even
admitted this was temporary: *"this is temporary. Should throw a special
error
that will be caught and retried"*. `updateTableLike` already handles the same
concurrency status correctly with `CommitConflictException` (409); this
change
brings `renameTableLike` in line with that convention.
Both statuses are documented in `BaseResult.ReturnStatus` as retryable
concurrency conditions ("the client should retry"), and the rename
implementation (`TransactionalMetaStoreManagerImpl.renameEntityInCurrentTxn`)
returns both on real races, so each branch is reachable in practice.
## Changes
- `IcebergCatalog.renameTableLike`: replaced the bare `RuntimeException` with
`CommitConflictException` for concurrent rename failures
(`TARGET_ENTITY_CONCURRENTLY_MODIFIED`, `ENTITY_CANNOT_BE_RESOLVED`).
- Added parameterized test `testConcurrencyConflictRenameTable` covering both
statuses, verifying they surface as `CommitConflictException` (409).
- `CHANGELOG.md`: added a Fixes entry.
## Known follow-up (out of scope)
The same `renameTableLike` switch still lacks a case for
`CATALOG_PATH_CANNOT_BE_RESOLVED` (cross-namespace renames where the
destination
path can't be resolved). That falls through to `default →
IllegalStateException
→ 500`, whereas `updateTableLike` handles it as `NotFoundException` (404).
This
is a pre-existing gap not introduced by this change; tracking separately.
## Testing
- `./gradlew :polaris-runtime-service:compileTestJava` — passes
- `./gradlew :polaris-runtime-service:spotlessCheck
:polaris-runtime-service:checkstyleMain
:polaris-runtime-service:checkstyleTest` — passes
- `./gradlew :polaris-runtime-service:test --tests
"org.apache.polaris.service.catalog.iceberg.IcebergCatalogRelationalTest.testConcurrencyConflictRenameTable"`
— passes
- `./gradlew :polaris-runtime-service:test --tests
"org.apache.polaris.service.catalog.iceberg.IcebergCatalogRelationalTest.testConcurrencyConflictUpdateTableDuringFinalTransaction"`
— passes (regression check)
## Checklist
- [ ] 🛡️ Don't disclose security issues! (contact [email protected])
- [x] 🔗 Clearly explained why the changes are needed
- [x] 🧪 Added/updated tests with good coverage, or manually tested (and
explained how)
- [ ] 💡 Added comments for complex logic
- [x] 🧾 Updated `CHANGELOG.md` (if needed)
- [ ] 📚 Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
