fix: commit pending proposals recovery strategy - WPB-23159#4254
fix: commit pending proposals recovery strategy - WPB-23159#4254netbe wants to merge 10 commits intorelease/cycle-4.15from
Conversation
Test Results 16 files 1 186 suites 11m 17s ⏱️ For more details on these failures, see this check. Results for commit 93c2b24. ♻️ This comment has been updated with latest results. Summary: workflow run #21706682407 |
netbe
requested review from
johnxnguyen and
samwyndham
and removed request for
johnxnguyen
| case .retryAfterBackoff: | ||
|
|
||
| throw MLSRetryError.retryLimitReached | ||
| try await BackoffRetrier(policy: .init(maxRetries: 2)).retry { [logger] in |
There was a problem hiding this comment.
issue: the backoff retrier internally sets up observation of the network reachability and will reset under certain conditions, we may need to decouple that from the retrier and place it in the sync agent where I think it's relevant.
There was a problem hiding this comment.
for now i simply disable it for this case
samwyndham
left a comment
samwyndham
left a comment
There was a problem hiding this comment.
@netbe Remember to re-target this to 4.15 release branch
netbe
force-pushed
the
fix/commit-pending-proposals-WPB-23159
branch
from
82adc38 to
0369bbd
Compare
samwyndham
left a comment
samwyndham
left a comment
There was a problem hiding this comment.
Looks good. I've added just a couple of comments before approval
| throw MLSRetryError.retryLimitReached | ||
| try await BackoffRetrier(policy: .init(maxRetries: 2), monitoringNetwork: false).retry { [logger] in | ||
| logger.warn( | ||
| "failed to send commit due to \(reason). retrying operation with backoff - attempt: \(retryCount)...", |
There was a problem hiding this comment.
sanity check: Is retryCount here correct considering the use of the BackoffRetrier?
| struct RepairFaultyMLSRemovalKeysWorkItem: WorkItem { | ||
|
|
||
| let id = UUID() | ||
| var id: String { |
There was a problem hiding this comment.
question: What is the purpose of making this a string?
There was a problem hiding this comment.
when we log item in workAgent only the id is visible, which makes it hard to know what kind of item is dropped / failed / started. I wanted something to help with it so basically including more info in the logs
There was a problem hiding this comment.
I see. I would personally be tempted to make WorkItem conform to LogConvertible or something like that. But I don't have a good argument why. Feel free to disregard this comment.
3 participants
Issue
Testing
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: