feat: retry failed transaction commit#576
Conversation
82ada96 to
ff6c292
Compare
2053566 to
05c1625
Compare
wgtmac
left a comment
There was a problem hiding this comment.
I've carefully reviewed the retry mechanism and found a few parity issues and a structural data-loss concern regarding how pending updates are held during retries. Please see the inline comments.
|
I just recall a design flaw in the interaction between PendingUpdate and Transaction and created a fix: #591. Without this fix, users have to cache all created pending update instances, otherwise they cannot retry them since they are weak_ptr in the transaction instance. |
|
@linguoxuan Could you please rebase to resolve the conflict? |
296d9ed to
79c0218
Compare
wgtmac
left a comment
There was a problem hiding this comment.
Thanks for the update and fixing the CI! I've just reviewed it for a quick pass.
9f9f12b to
7108f53
Compare
|
Is this ready to review again? |
Yes. PTAL. |
|
Sorry I've accidentally pushed my local main (which is up to sync) against your main branch and resulted in this PR being closed due to empty diff. Could you either force push your local main or merge linguoxuan#1 to proceed? I've polished the design a little bit and fixed some issues so it is ready to merge. Thanks! |
This commit implements the retry for transaction commits. It introduces a generic RetryRunner utility with exponential backoff and error-kind filtering, and integrates it into Transaction::Commit() to automatically refresh table metadata and retry on commit conflicts.