RUST-981 Merge new load balancer spec tests #480
Conversation
| @@ -442,12 +437,8 @@ impl PinnedConnectionHandle { | |||
| }) | |||
| } | |||
|
|
|||
| /// Return the pinned connection to the normal connection pool. | |||
| pub(crate) async fn unpin_connection(&self) { | |||
There was a problem hiding this comment.
This was previously called by cursors on completion, but with the connection potentially shared by a cursor and transaction the unpin now happens (implicitly) when the final PinnedConnectionHandle for a given Connection is dropped via the final Arc causing the channel endpoint to drop.
| @@ -155,7 +155,7 @@ impl<'de> Deserialize<'de> for ReadPreference { | |||
| "PrimaryPreferred" => Ok(ReadPreference::PrimaryPreferred { | |||
| options: preference.options, | |||
| }), | |||
| "SecondaryPreferred" => Ok(ReadPreference::SecondaryPreferred { | |||
| "SecondaryPreferred" | "secondaryPreferred" => Ok(ReadPreference::SecondaryPreferred { | |||
There was a problem hiding this comment.
One of the spec tests uses "secondaryPreferred" as a value; not sure whether this was meant to be case sensitive or not.
| @@ -245,6 +255,8 @@ pub async fn run_unified_format_test(test_file: TestFile) { | |||
| } | |||
| } | |||
|
|
|||
| test_runner.fail_point_guards.clear(); | |||
There was a problem hiding this comment.
This had to go after the event expectation evaluation because clearing the guards can now generate events in itself, which the tests don't take into account.
| @@ -1817,6 +1817,7 @@ impl TestOperation for AssertNumberConnectionsCheckedOut { | |||
| ) -> BoxFuture<'a, ()> { | |||
| async move { | |||
| let client = test_runner.get_client(&self.client); | |||
| client.sync_workers().await; | |||
There was a problem hiding this comment.
connections_checked_out() relies on the worker queue having been processed to be accurate.
|
The change in the previous PR to revert to skipping the wait queue clear for load-balanced topologies is causing another test failure here - it expects a pool cleared event that doesn't happen. Since this is a test specific to load balancers I'm inclined to go back to clearing the wait queue unconditionally and update that in the future if needed once the spec question gets ironed out. |
| @@ -1482,6 +1482,7 @@ impl TestOperation for AssertSameLsidOnLastTwoCommands { | |||
| ) -> BoxFuture<'a, ()> { | |||
| async move { | |||
| let client = test_runner.entities.get(&self.client).unwrap().as_client(); | |||
| client.sync_workers().await; | |||
There was a problem hiding this comment.
This reduced but did not entirely remove flakiness of the test depending on this; as far as I can tell there are two ways this could fail: events not yet received (fixed here) or session non-reuse equivalent to the connection non-reuse elsewhere (not fixed). I've filed RUST-1056 to address the latter.
abr-egn
force-pushed
the
RUST-981/lb-new-tests
branch
from
bd51976 to
c2505c0
Compare
RUST-981
This integrates the new load balancer spec tests, and fixes various bugs (largely around load balanced connection lifecycle) that they exposed.
This also fixes RUST-1037 and RUST-1042.