PERL-918 Update retryable writes spec #165
Conversation
xdg
left a comment
xdg
left a comment
There was a problem hiding this comment.
MongoDB::NetworkError should also default to being retryable. Otherwise, looks good.
| my $err = length($@) ? $@ : "caught error, but it was lost in eval unwind"; | ||
| my $retry_link = $self->{_topology}->get_writable_link; | ||
|
|
||
| # Rare chance that the new link is not retryable |
There was a problem hiding this comment.
Comment is now mildly confusing given extra error check.
There was a problem hiding this comment.
Oh oops I did have a comment there about the extra check, must have got caught with a debugging cleanup...
| # errors in a retryable write. This has to be seperate to the other functions | ||
| # due to not all result objects having the base response inside, so cannot be | ||
| # used to parse operationTime or $clusterTime | ||
| sub _assert_session_errors { |
There was a problem hiding this comment.
I'm a little nervous about this, but if all existing tests pass (particularly for bulk operations) then I'm okay with it.
There was a problem hiding this comment.
Yea this was the least complicated place to add this assertion - I couldn't see any information about the comment in MongoDB::Op::_BulkWrite->execute of write concern errors are thrown only for the entire batch in the specs for bulk operations, and as retryable writes can happen on certain write concern errors it must be done inside the retry loop.
The other single ops do throw WriteConcernErrors further down the stack, but only on the return value of the send_*write_op which is outside the retryable writes block - could maybe move that assertion up the call stack slightly to be inside the execute block? If commandResult is also modified to check for write concern errors in the assert step, that would get rid of this extra assertion? Would mean that bulk writes would throw write concern errors earlier, and as I said I couldnt find if that is a spec decision or not...
|
As far as I can tell, |
xdg
left a comment
xdg
left a comment
There was a problem hiding this comment.
LGTM. Please work on integrating with PERL-875 and giving me a single branch to merge.
|
Closing this PR as the #163 includes it. |
3 participants
Updated retryable-writes spec tests as the appeared at mongodb/specifications@b7c53fc