From 99b86b32a032b9b39f9fec07a90095958b977bb0 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Thu, 21 May 2020 08:58:00 -0400 Subject: [PATCH] fix: filter servers before applying reducers In order to accurately reduce the set of viable servers for server selection, we need to filter the set by the read preference BEFORE we reduce by max staleness, tagSet and latency window. NODE-2407 --- lib/sdam/server_selection.js | 54 +++++++------------ .../Unknown/SmallMaxStaleness.json | 3 +- .../Unknown/SmallMaxStaleness.yml | 1 + .../ReplicaSetWithPrimary/read/Nearest.json | 18 +++++-- .../ReplicaSetWithPrimary/read/Nearest.yml | 9 +++- .../ReplicaSetWithPrimary/read/Secondary.json | 8 +-- .../ReplicaSetWithPrimary/read/Secondary.yml | 6 +-- 7 files changed, 48 insertions(+), 51 deletions(-) diff --git a/lib/sdam/server_selection.js b/lib/sdam/server_selection.js index 6286ab5d3a..84da156b11 100644 --- a/lib/sdam/server_selection.js +++ b/lib/sdam/server_selection.js @@ -47,7 +47,7 @@ function maxStalenessReducer(readPreference, topologyDescription, servers) { } if (topologyDescription.type === TopologyType.ReplicaSetWithPrimary) { - const primary = servers.filter(primaryFilter)[0]; + const primary = Array.from(topologyDescription.servers.values()).filter(primaryFilter)[0]; return servers.reduce((result, server) => { const stalenessMS = server.lastUpdateTime - @@ -196,50 +196,32 @@ function readPreferenceServerSelector(readPreference) { return latencyWindowReducer(topologyDescription, servers.filter(knownFilter)); } - if (readPreference.mode === ReadPreference.PRIMARY) { + const mode = readPreference.mode; + if (mode === ReadPreference.PRIMARY) { return servers.filter(primaryFilter); } - if (readPreference.mode === ReadPreference.SECONDARY) { - return latencyWindowReducer( - topologyDescription, - tagSetReducer( - readPreference, - maxStalenessReducer(readPreference, topologyDescription, servers) - ) - ).filter(secondaryFilter); - } else if (readPreference.mode === ReadPreference.NEAREST) { - return latencyWindowReducer( - topologyDescription, - tagSetReducer( - readPreference, - maxStalenessReducer(readPreference, topologyDescription, servers) - ) - ).filter(nearestFilter); - } else if (readPreference.mode === ReadPreference.SECONDARY_PREFERRED) { - const result = latencyWindowReducer( - topologyDescription, - tagSetReducer( - readPreference, - maxStalenessReducer(readPreference, topologyDescription, servers) - ) - ).filter(secondaryFilter); - - return result.length === 0 ? servers.filter(primaryFilter) : result; - } else if (readPreference.mode === ReadPreference.PRIMARY_PREFERRED) { + if (mode === ReadPreference.PRIMARY_PREFERRED) { const result = servers.filter(primaryFilter); if (result.length) { return result; } + } + + const filter = mode === ReadPreference.NEAREST ? nearestFilter : secondaryFilter; + const selectedServers = latencyWindowReducer( + topologyDescription, + tagSetReducer( + readPreference, + maxStalenessReducer(readPreference, topologyDescription, servers.filter(filter)) + ) + ); - return latencyWindowReducer( - topologyDescription, - tagSetReducer( - readPreference, - maxStalenessReducer(readPreference, topologyDescription, servers) - ) - ).filter(secondaryFilter); + if (mode === ReadPreference.SECONDARY_PREFERRED && selectedServers.length === 0) { + return servers.filter(primaryFilter); } + + return selectedServers; }; } diff --git a/test/spec/max-staleness/Unknown/SmallMaxStaleness.json b/test/spec/max-staleness/Unknown/SmallMaxStaleness.json index 73957ab6e8..bf6174b8e4 100644 --- a/test/spec/max-staleness/Unknown/SmallMaxStaleness.json +++ b/test/spec/max-staleness/Unknown/SmallMaxStaleness.json @@ -5,7 +5,8 @@ "servers": [ { "address": "a:27017", - "type": "Unknown" + "type": "Unknown", + "maxWireVersion": 5 } ] }, diff --git a/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml b/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml index 7daaa1fb4a..3a39615ac0 100644 --- a/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml +++ b/test/spec/max-staleness/Unknown/SmallMaxStaleness.yml @@ -7,6 +7,7 @@ topology_description: - &1 address: a:27017 type: Unknown + maxWireVersion: 5 read_preference: mode: Nearest maxStalenessSeconds: 1 diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json index cfe4965938..45b3725585 100644 --- a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json +++ b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.json @@ -4,7 +4,7 @@ "servers": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" @@ -20,11 +20,19 @@ }, { "address": "a:27017", - "avg_rtt_ms": 26, + "avg_rtt_ms": 37, "type": "RSPrimary", "tags": { "data_center": "nyc" } + }, + { + "address": "d:27017", + "avg_rtt_ms": 5, + "type": "RSOther", + "tags": { + "data_center": "nyc" + } } ] }, @@ -40,7 +48,7 @@ "suitable_servers": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" @@ -48,7 +56,7 @@ }, { "address": "a:27017", - "avg_rtt_ms": 26, + "avg_rtt_ms": 37, "type": "RSPrimary", "tags": { "data_center": "nyc" @@ -66,7 +74,7 @@ "in_latency_window": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml index 5e8e0499d2..1ffb3df32c 100644 --- a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml +++ b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Nearest.yml @@ -3,7 +3,7 @@ topology_description: servers: - &1 address: b:27017 - avg_rtt_ms: 5 + avg_rtt_ms: 21 type: RSSecondary tags: data_center: nyc @@ -15,10 +15,15 @@ topology_description: data_center: nyc - &2 address: a:27017 - avg_rtt_ms: 26 + avg_rtt_ms: 37 type: RSPrimary tags: data_center: nyc + - address: d:27017 + avg_rtt_ms: 5 + type: RSOther + tags: + data_center: nyc operation: read read_preference: mode: Nearest diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json index 23864a278c..a2e35a769c 100644 --- a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json +++ b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.json @@ -4,7 +4,7 @@ "servers": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" @@ -20,7 +20,7 @@ }, { "address": "a:27017", - "avg_rtt_ms": 26, + "avg_rtt_ms": 5, "type": "RSPrimary", "tags": { "data_center": "nyc" @@ -40,7 +40,7 @@ "suitable_servers": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" @@ -58,7 +58,7 @@ "in_latency_window": [ { "address": "b:27017", - "avg_rtt_ms": 5, + "avg_rtt_ms": 21, "type": "RSSecondary", "tags": { "data_center": "nyc" diff --git a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml index 85afe014a1..031643e814 100644 --- a/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml +++ b/test/spec/server-selection/server_selection/ReplicaSetWithPrimary/read/Secondary.yml @@ -3,18 +3,18 @@ topology_description: servers: - &1 address: b:27017 - avg_rtt_ms: 5 + avg_rtt_ms: 21 # outside the latency window if primary is considered type: RSSecondary tags: data_center: nyc - &2 address: c:27017 - avg_rtt_ms: 100 + avg_rtt_ms: 100 # outside the latency window if both secondaries are considered type: RSSecondary tags: data_center: nyc - address: a:27017 - avg_rtt_ms: 26 + avg_rtt_ms: 5 type: RSPrimary tags: data_center: nyc