Skip to content

Conversation

@isabelatkinson
Copy link
Contributor

@isabelatkinson isabelatkinson commented Jan 8, 2026

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the changes in this file are just shuffling things around; the functional changes are based on this diff from the spec PR.

topology_description: &'a TopologyDescription,
servers: &'a HashMap<ServerAddress, Arc<Server>>,
deprioritized: Option<&ServerAddress>,
deprioritized: &[&ServerAddress],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this a list in anticipation of CSOT and because the tests use lists of multiple addresses.

@isabelatkinson isabelatkinson marked this pull request as ready for review January 8, 2026 17:26
@isabelatkinson isabelatkinson requested a review from a team as a code owner January 8, 2026 17:26
@isabelatkinson isabelatkinson requested a review from abr-egn January 8, 2026 17:26
@isabelatkinson isabelatkinson changed the title RUST-2310 Deprioritize servers for all topologies RUST-2310 Deprioritize servers in server selection for all topologies Jan 8, 2026
Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Some very small questions, no need for a re-review.

fn suitable_servers(
fn filter_servers_in_replica_set<'a>(
&self,
servers: impl Iterator<Item = &'a ServerDescription> + Clone,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just &[&'a ServerDescription]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter_servers_by_selection_criteria (which calls this) already has an iterator with the deprioritized servers filtered out, so accepting that directly allows us to avoid an intermediary structure

#[allow(unused_variables)] // we only use the operation_name for tracing.
operation_name: &str,
deprioritized: Option<&ServerAddress>,
deprioritized: Vec<&ServerAddress>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, it doesn't seem like this needs to be owned?

@isabelatkinson
Copy link
Contributor Author

not sure why but this is still requiring re-approval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants