rdblue commented on issue #15001: URL: https://github.com/apache/iceberg/issues/15001#issuecomment-3781759936
I think the error described here makes sense. A sequence number conflict should trigger a retry. The REST catalog service should be sending back a `CommitFailedException` rather than the `ValidationException`. I think the question is how best to do that. We could add a validation that is triggered by adding a snapshot, but that would be more specific than the checks that are happening in `addSnapshot`. Right now, a snapshot can use a future sequence number to avoid a retry like this and the [last sequence number in metadata is increased to match it](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1261). Using an assertion would require a failure and retry even if metadata were still valid. In addition, we would have the same problem for `next-row-id` that is also validated for v3 tables and later, so we would also require an assertion for `last-row-id` and anything similar. On the other hand, `MetadataUpdate#applyTo` is also called in the retryable block. So we could also wrap validation exceptions from `addSnapshot` in `CommitFailedException`. Or we could introduce a `RetryableValidationException` to more narrowly target the validation exceptions that can be retried and wrapped by `CommitFailedException`. I think I'd prefer this exception wrapping solution. It's pretty narrow and doesn't require adding new requirements to the spec. -- 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]
