agnes-xinyi-lu commented on code in PR #15126:
URL: https://github.com/apache/iceberg/pull/15126#discussion_r2729213735
##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -633,7 +634,20 @@ static TableMetadata commit(TableOperations ops,
UpdateTableRequest request) {
// apply changes
TableMetadata.Builder metadataBuilder =
TableMetadata.buildFrom(base);
- request.updates().forEach(update ->
update.applyTo(metadataBuilder));
+ try {
+ request.updates().forEach(update ->
update.applyTo(metadataBuilder));
+ } catch (ValidationException e) {
+ // Sequence number conflicts from concurrent commits are
retryable by the client,
+ // but server-side retry won't help since the sequence
number is in the request.
Review Comment:
Thanks for the review @singhpk234 !
Checking exception message is not an uncommon pattern within [iceberg repo
](https://github.com/search?q=repo%3Aapache%2Ficeberg+%22getMessage%28%29.contains%22&type=code),
it helps target particular scenarios that were thrown in a more generic
exception type. Refactoring the exception itself will require TableMetadata
change which increases risks.
I'm trying to minimize the change to get this issue fixed as per my
understanding of the
[comment](https://github.com/apache/iceberg/issues/15001#issuecomment-3781759936)
on the issue. As my original idea was to add an UpdateRequirement to the spec
for this assertion.
Any thoughts?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
