Skip to content

Commit 47cab04

Browse files
ShaneHarveyShaneHarvey
committed
PYTHON-1407 Exclude non-data bearing servers when considering logicalSessionTimeoutMinutes
1 parent b878ed6 commit 47cab04

File tree

4 files changed

+204
-24
lines changed

4 files changed

+204
-24
lines changed

‎pymongo/topology.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from pymongo.server_selectors import (any_server_selector,
3939
arbiter_server_selector,
4040
secondary_server_selector,
41+
readable_server_selector,
4142
writable_server_selector,
4243
Selection)
4344
from pymongo.client_session import _ServerSessionPool
@@ -402,9 +403,15 @@ def get_server_session(self):
402403
session_timeout = self._description.logical_session_timeout_minutes
403404
if session_timeout is None:
404405
# Maybe we need an initial scan? Can raise ServerSelectionError.
405-
if not self.description.has_known_servers:
406+
if self._description.topology_type == TOPOLOGY_TYPE.Single:
407+
if not self._description.has_known_servers:
408+
self._select_servers_loop(
409+
any_server_selector,
410+
self._settings.server_selection_timeout,
411+
None)
412+
elif not self._description.readable_servers:
406413
self._select_servers_loop(
407-
any_server_selector,
414+
readable_server_selector,
408415
self._settings.server_selection_timeout,
409416
None)
410417

‎pymongo/topology_description.py‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,22 @@ def __init__(self,
9494

9595
break
9696

97-
# Server Discovery And Monitoring Spec: "Whenever a client updates the
97+
# Server Discovery And Monitoring Spec: Whenever a client updates the
9898
# TopologyDescription from an ismaster response, it MUST set
9999
# TopologyDescription.logicalSessionTimeoutMinutes to the smallest
100-
# logicalSessionTimeoutMinutes value among all ServerDescriptions
101-
# of known ServerType. If any ServerDescription of known ServerType has
102-
# a null logicalSessionTimeoutMinutes, then
100+
# logicalSessionTimeoutMinutes value among ServerDescriptions of all
101+
# data-bearing server types. If any have a null
102+
# logicalSessionTimeoutMinutes, then
103103
# TopologyDescription.logicalSessionTimeoutMinutes MUST be set to null.
104-
readable = [s for s in self.known_servers if s.is_readable]
105-
if not readable:
104+
readable_servers = self.readable_servers
105+
if not readable_servers:
106106
self._ls_timeout_minutes = None
107-
elif any(s.logical_session_timeout_minutes is None for s in readable):
107+
elif any(s.logical_session_timeout_minutes is None
108+
for s in readable_servers):
108109
self._ls_timeout_minutes = None
109110
else:
110111
self._ls_timeout_minutes = min(s.logical_session_timeout_minutes
111-
for s in readable)
112+
for s in readable_servers)
112113

113114
def check_compatible(self):
114115
"""Raise ConfigurationError if any server is incompatible.
@@ -195,6 +196,11 @@ def has_known_servers(self):
195196
return any(s for s in self._server_descriptions.values()
196197
if s.is_server_type_known)
197198

199+
@property
200+
def readable_servers(self):
201+
"""List of readable Servers."""
202+
return [s for s in self._server_descriptions.values() if s.is_readable]
203+
198204
@property
199205
def common_wire_version(self):
200206
"""Minimum of all servers' max wire versions, or None."""

‎test/discovery_and_monitoring/rs/ls_timeout.json‎

Lines changed: 180 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,127 @@
1111
"ismaster": true,
1212
"hosts": [
1313
"a:27017",
14-
"b:27017"
14+
"b:27017",
15+
"c:27017",
16+
"d:27017",
17+
"e:27017"
1518
],
1619
"setName": "rs",
17-
"logicalSessionTimeoutMinutes": 1,
20+
"logicalSessionTimeoutMinutes": 3,
21+
"minWireVersion": 0,
22+
"maxWireVersion": 6
23+
}
24+
]
25+
],
26+
"outcome": {
27+
"servers": {
28+
"a:27017": {
29+
"type": "RSPrimary",
30+
"setName": "rs"
31+
},
32+
"b:27017": {
33+
"type": "Unknown"
34+
},
35+
"c:27017": {
36+
"type": "Unknown"
37+
},
38+
"d:27017": {
39+
"type": "Unknown"
40+
},
41+
"e:27017": {
42+
"type": "Unknown"
43+
}
44+
},
45+
"topologyType": "ReplicaSetWithPrimary",
46+
"logicalSessionTimeoutMinutes": 3,
47+
"setName": "rs"
48+
}
49+
},
50+
{
51+
"responses": [
52+
[
53+
"d:27017",
54+
{
55+
"ok": 1,
56+
"ismaster": false,
57+
"isreplicaset": true,
1858
"minWireVersion": 0,
1959
"maxWireVersion": 6
2060
}
21-
],
61+
]
62+
],
63+
"outcome": {
64+
"servers": {
65+
"a:27017": {
66+
"type": "RSPrimary",
67+
"setName": "rs"
68+
},
69+
"b:27017": {
70+
"type": "Unknown"
71+
},
72+
"c:27017": {
73+
"type": "Unknown"
74+
},
75+
"d:27017": {
76+
"type": "RSGhost"
77+
},
78+
"e:27017": {
79+
"type": "Unknown"
80+
}
81+
},
82+
"topologyType": "ReplicaSetWithPrimary",
83+
"logicalSessionTimeoutMinutes": 3,
84+
"setName": "rs"
85+
}
86+
},
87+
{
88+
"responses": [
89+
[
90+
"e:27017",
91+
{
92+
"ok": 1,
93+
"ismaster": false,
94+
"hosts": [
95+
"a:27017",
96+
"b:27017",
97+
"c:27017",
98+
"d:27017",
99+
"e:27017"
100+
],
101+
"setName": "rs",
102+
"arbiterOnly": true,
103+
"minWireVersion": 0,
104+
"maxWireVersion": 6
105+
}
106+
]
107+
],
108+
"outcome": {
109+
"servers": {
110+
"a:27017": {
111+
"type": "RSPrimary",
112+
"setName": "rs"
113+
},
114+
"b:27017": {
115+
"type": "Unknown"
116+
},
117+
"c:27017": {
118+
"type": "Unknown"
119+
},
120+
"d:27017": {
121+
"type": "RSGhost"
122+
},
123+
"e:27017": {
124+
"type": "RSArbiter",
125+
"setName": "rs"
126+
}
127+
},
128+
"topologyType": "ReplicaSetWithPrimary",
129+
"logicalSessionTimeoutMinutes": 3,
130+
"setName": "rs"
131+
}
132+
},
133+
{
134+
"responses": [
22135
[
23136
"b:27017",
24137
{
@@ -27,7 +140,10 @@
27140
"secondary": true,
28141
"hosts": [
29142
"a:27017",
30-
"b:27017"
143+
"b:27017",
144+
"c:27017",
145+
"d:27017",
146+
"e:27017"
31147
],
32148
"setName": "rs",
33149
"logicalSessionTimeoutMinutes": 2,
@@ -45,30 +161,67 @@
45161
"b:27017": {
46162
"type": "RSSecondary",
47163
"setName": "rs"
164+
},
165+
"c:27017": {
166+
"type": "Unknown"
167+
},
168+
"d:27017": {
169+
"type": "RSGhost"
170+
},
171+
"e:27017": {
172+
"type": "RSArbiter",
173+
"setName": "rs"
48174
}
49175
},
50176
"topologyType": "ReplicaSetWithPrimary",
51-
"logicalSessionTimeoutMinutes": 1,
177+
"logicalSessionTimeoutMinutes": 2,
52178
"setName": "rs"
53179
}
54180
},
55181
{
56182
"responses": [
57183
[
58-
"a:27017",
184+
"c:27017",
59185
{
60186
"ok": 1,
61-
"ismaster": true,
62-
"hosts": [
63-
"a:27017",
64-
"b:27017"
65-
],
187+
"ismaster": false,
66188
"setName": "rs",
189+
"hidden": true,
67190
"logicalSessionTimeoutMinutes": 1,
68191
"minWireVersion": 0,
69192
"maxWireVersion": 6
70193
}
71-
],
194+
]
195+
],
196+
"outcome": {
197+
"servers": {
198+
"a:27017": {
199+
"type": "RSPrimary",
200+
"setName": "rs"
201+
},
202+
"b:27017": {
203+
"type": "RSSecondary",
204+
"setName": "rs"
205+
},
206+
"c:27017": {
207+
"type": "RSOther",
208+
"setName": "rs"
209+
},
210+
"d:27017": {
211+
"type": "RSGhost"
212+
},
213+
"e:27017": {
214+
"type": "RSArbiter",
215+
"setName": "rs"
216+
}
217+
},
218+
"topologyType": "ReplicaSetWithPrimary",
219+
"logicalSessionTimeoutMinutes": 2,
220+
"setName": "rs"
221+
}
222+
},
223+
{
224+
"responses": [
72225
[
73226
"b:27017",
74227
{
@@ -77,7 +230,10 @@
77230
"secondary": true,
78231
"hosts": [
79232
"a:27017",
80-
"b:27017"
233+
"b:27017",
234+
"c:27017",
235+
"d:27017",
236+
"e:27017"
81237
],
82238
"setName": "rs",
83239
"logicalSessionTimeoutMinutes": null,
@@ -95,6 +251,17 @@
95251
"b:27017": {
96252
"type": "RSSecondary",
97253
"setName": "rs"
254+
},
255+
"c:27017": {
256+
"type": "RSOther",
257+
"setName": "rs"
258+
},
259+
"d:27017": {
260+
"type": "RSGhost"
261+
},
262+
"e:27017": {
263+
"type": "RSArbiter",
264+
"setName": "rs"
98265
}
99266
},
100267
"topologyType": "ReplicaSetWithPrimary",

‎test/test_discovery_and_monitoring.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def check_outcome(self, topology, outcome):
156156
server_type_name(actual_server_description.server_type))
157157

158158
self.assertEqual(
159-
expected_server['setName'],
159+
expected_server.get('setName'),
160160
actual_server_description.replica_set_name)
161161

162162
self.assertEqual(

0 commit comments

Comments
 (0)