PERL-792 Retryable Writes #159
Conversation
Rather than pass in a MongoDB::_Link object to helpers like document pre-encoders and the bypass document validation role, this passes in only a boolean value. This allows us to get the link object out of the bulk write execution loop, which is needed for retryable writes.
| # event, just that the failed one comes first. | ||
| my $first_insert_index; | ||
|
|
||
| for my $f_idx ( 0 .. $#events ) { |
There was a problem hiding this comment.
$#events -1? Or line 199 could go off the end. Should never happen if command monitoring is working right and always pairing, but I'd rather not rely on a reader knowing that.
|
|
||
| my $second_insert_index; | ||
|
|
||
| for my $s_idx ( $first_insert_index + 2 .. $#events ) { |
There was a problem hiding this comment.
Ditto, plus it's worth checking there are enough events past the first.
lib/MongoDB/Op/_BulkWrite.pm
Outdated
| } @{ $self->queue }; | ||
| } | ||
|
|
||
| ## Encapsulate the write loop for this |
There was a problem hiding this comment.
Please figure out what of this comment needs to be kept (if any) for documentation purposes and what can be deleted.
lib/MongoDB/Op/_BulkWrite.pm
Outdated
| unshift @left_to_send, $self->_split_chunk( $chunk, $_->size ); | ||
| } | ||
| } | ||
| # Put retryable writes catches... no not here... damn... ermm... oh. Could increment transaction id at the beginning before the loop, then after the redo increment again. Then it wont get caught? hmm.... food |
There was a problem hiding this comment.
Ah oops I thought i got rid of that comment... Was from the odd debugging stuff when I tied the driver in knots with $link
lib/MongoDB/Role/_RetryableBulk.pm
Outdated
|
|
||
| use strict; | ||
| use warnings; | ||
| package MongoDB::Role::_RetryableBulk; |
There was a problem hiding this comment.
Having a role just to provide an attribute seems excessive. I think it's OK to just put it in the two classes that need it.
| @@ -0,0 +1,320 @@ | |||
| # | |||
There was a problem hiding this comment.
I like how this file turned out! Very easy to follow.
|
This is just about done. Just a few more things to do. Also, rather than leave the tests as t/PERL-..., please call them t/retryable-... with some sensible description. |
|
there we go! Hope that all works :D |
|
LGTM. I'll try to rebase and get it merged. |
|
Rebased, fixed up, and merged. Thanks! |
3 participants
No description provided.