RUST-1274 Fix commitTransaction on check out retries #651
Conversation
patrickfreed
requested review from
kmahar,
isabelatkinson and
abr-egn
and removed request for
kmahar
There was a problem hiding this comment.
Re-requesting review, since I changed some logic to fix RUST-1317, which was required to implement the test for this.
| topology.notify_topology_changed(); | ||
| } | ||
| self.check_server(&topology, &server).await; | ||
| topology.notify_topology_changed(); |
There was a problem hiding this comment.
Once I added a commit to fix RUST-1317, some tests started failing due to this if statement. The problem is if the error stayed the same, this check would not call notify_topology_changed, which in turn would mean that the operation blocked on server selection would not try again and would not request another immediate check. This means that the blocked operation wouldn't succeed until the next heartbeat regularly scheduled, which could be 10s of seconds in the future.
Note that this new implementation matches the pseudocode in the server monitoring spec as well as the Python implementation. Also, by passing the test added in DRIVERS-1251, the C#, Go and Java likely have this behavior too. The spec does allow for our prior only-notify-if-changed behavior though, so I filed DRIVERS-2329 to require the behavior seen here.
RUST-1274
This backports the fix for
InvalidOptionserrors that are seen whencommitTransactioncommands are retried due to connection check out failures. The fix for this was included onmainin #628.