feat: retry stale v3 snapshot row-lineage validation#794
Conversation
Add a retryable validation error kind and use it for add-snapshot stale sequence-number and stale first-row-id checks, matching Java Iceberg's RetryableValidationException behavior. Include the new retryable validation kind in commit retry policy, while preserving normal validation failures for mixed/non-retryable builder errors. Add focused v3 row-lineage tests for multi-file assignment, branch commits, retry reassignment, stale snapshot validation, and delete-manifest null first_row_id handling.
| [[nodiscard]] Status CheckErrors() const { | ||
| if (!errors_.empty()) { | ||
| std::string error_msg = "Validation failed due to the following errors:\n"; | ||
| bool all_retryable = true; |
There was a problem hiding this comment.
I'm wondering why we require all the errors to be retryable before treating the result as RetryableValidationFailed. Java impl throws a RetryableValidationException immediately when it encounters a retryable validation error, will this affect the behavior of lib users?
There was a problem hiding this comment.
The all_retryable check is intentional because C++ builders accumulate errors, while Java is fail-fast. In Java, a RetryableValidationException stops evaluation immediately, so there is no mixed-error result to classify.
In C++, if we treated the result as retryable when only the first error is retryable, a chained builder call could retry even when another collected error is deterministic and cannot be fixed by refreshing metadata. Requiring all collected errors to be retryable keeps retry limited to cases where refreshing can plausibly fix the whole update.
|
Thanks all for the review! |
merging_snapshot_update_test.cc called op->SetTargetBranch("audit"), a
method that does not exist on MergingSnapshotUpdate, breaking the CMake
-Werror build on upstream/main HEAD (introduced by apache#794). The real API for
targeting a branch is ToBranch(). Carry-patch until upstream fixes it.
4 participants
Add a retryable validation error kind and use it for add-snapshot stale sequence-number and stale first-row-id checks, matching Java Iceberg's RetryableValidationException behavior.
Include the new retryable validation kind in commit retry policy, while preserving normal validation failures for mixed/non-retryable builder errors. Add focused v3 row-lineage tests for multi-file assignment, branch commits, retry reassignment, stale snapshot validation, and delete-manifest null first_row_id handling.