amogh-jahagirdar commented on code in PR #15126:
URL: https://github.com/apache/iceberg/pull/15126#discussion_r2743592763
##########
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:
I agree with @singhpk234 here. It's a bit brittle to rely on the message in
the exception. A lot of the cases where that's done in the code base is at the
integration points with other libraries/systems like JDBC/hive where there
isn't a better exception provided. Here it's all the Iceberg core code base,
where there can be other use cases where this behavior is desirable and we have
an opportunity to do it cleanly.
In that case, I think what @rdblue suggested in the issue makes a lot of
sense; we could define a `RetryableValidationException`, throw it instead of
the validation exception in addSnapshot when performing the sequence number
comparison. Here in catalog handlers, then we'd then throw the
ValidationFailure wrapping a CommitFailedException to break out and kick the
CommitFailedException back to the client to retry.
--
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]
