hcrosse commented on PR #2384: URL: https://github.com/apache/iceberg-go/pull/2384#issuecomment-6640462374
Hi, a review on https://github.com/apache/iceberg-go/pull/2354 asked for the two PRs to coordinate their landing order: https://github.com/apache/iceberg-go/pull/2354#pullrequestreview-5772798117. Both PRs change how retry replay handles staged ref moves and counters. I suggest landing https://github.com/apache/iceberg-go/pull/2354 as it is. I would then open a small PR to make `stagedReplay` the single path. That PR would also decide whether an update that no longer applies on retry returns `ErrTransactionUnusable` or leaves the transaction resumable. Your PR could then rebase onto it and drop its changes to `rebuildSnapshotUpdates`. In my test, routing every transaction through `stagedReplay` with your new API and tests but without your replay changes passed all eight promotion retry test cases and all of `./table`. One `catalog/sql` test from https://github.com/apache/iceberg-go/pull/2350 fails, which is the classification question above. Would that work for you, or would you prefer to land your PR first? -- 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]
