Add support for server selection's deprioritized servers to all topologies. #1860
Conversation
|
|
||
| protected void updateDescription(final ClusterDescription newDescription) { | ||
| @VisibleForTesting(otherwise = PROTECTED) | ||
| public void updateDescription(final ClusterDescription newDescription) { |
There was a problem hiding this comment.
It seems we can avoid making this method public by overriding it in the subclass. We won't even need to make it public in the subclass, because the test code using the method will be in the same package as the subclass. However, this requires the subclass to not be anonymous (if we could use var, the proposed approach would have worked even for an anonymous subclass).
| } catch (MongoTimeoutException mongoTimeoutException) { | ||
| List<ServerDescription> inLatencyWindowServers = buildServerDescriptions(definition.getArray("in_latency_window")); | ||
| assertTrue("Expected emtpy but was " + inLatencyWindowServers.size() + " " + definition.toJson( | ||
| JsonWriterSettings.builder() | ||
| .indent(true).build()), inLatencyWindowServers.isEmpty()); | ||
| return; |
There was a problem hiding this comment.
Since we now perform the full server-selection path (via BaseCluster), the behavior of "no servers selected: is observed differently than before.
Previously, these tests invoked the Selector directly, got an empty list, and asserted on that result. With BaseCluster, server selection runs the normal selection loop: it will retry until either a server becomes selectable or the selection timeout elapses.
In this setup, a server selection timeout is the expected signal that no servers are available/eligible for selection. The timeout is set to 200ms to keep the test fast, while giving enough headroom to avoid any flakiness.
| List<ServerDescription> latencyBasedSelectedServers = latencyBasedServerSelector.select(clusterDescription); | ||
| assertServers(latencyBasedSelectedServers, inLatencyWindowServers); | ||
| assertNotNull(serverTuple); | ||
| assertTrue(inLatencyWindowServers.stream().anyMatch(s -> s.getAddress().equals(serverTuple.getServerDescription().getAddress()))); |
There was a problem hiding this comment.
There’s an ambiguity in the server selection spec docs about what drivers must assert for in_latency_window:
tests/README.md says to verify the set of servers in in_latency_window.
"Drivers implementing server selection MUST test that their implementation correctly returns the set of servers in in_latency_window."
server-selection-tests.mdsays to verify the selection returns one of the servers in in_latency_window.
"Drivers implementing server selection MUST test that their implementations correctly return one of the servers in in_latency_window."
This test follows server-selection-tests.md, so asserting that the selected server is within the expected in_latency_window set is consistent with the requirements in the spec.
P.S: Both files were created in this single commit with contradicting requirements:
Feb 6, 2015 - Commit 6b63123a - "Add Server Selection Spec"
File 1: server-selection-tests.rst
Drivers implementing server selection MUST test that their implementations
correctly return one of the servers in ``in_latency_window``.
File 2: tests/README.rst
Drivers implementing server selection MUST test that their implementations
correctly return the set of servers in ``in_latency_window``.
2 participants
JAVA-6021