Skip to content

Commit 09647f0

Browse files
blink1073blink1073
committed
address review
1 parent fd63597 commit 09647f0

File tree

9 files changed

+66
-181
lines changed

9 files changed

+66
-181
lines changed

‎pymongo/asynchronous/mongo_client.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2802,7 +2802,7 @@ async def run(self) -> T:
28022802
if isinstance(exc, (ConnectionFailure, OperationFailure)):
28032803
# ConnectionFailures do not supply a code property
28042804
exc_code = getattr(exc, "code", None)
2805-
always_retryable = exc.has_error_label("RetryableError")
2805+
always_retryable = exc.has_error_label("RetryableError") and self._retryable
28062806
overloaded = exc.has_error_label("SystemOverloadedError")
28072807
if not always_retryable and (
28082808
self._is_not_eligible_for_retry()
@@ -2825,7 +2825,9 @@ async def run(self) -> T:
28252825
):
28262826
exc_to_check = exc.error
28272827
retryable_write_label = exc_to_check.has_error_label("RetryableWriteError")
2828-
always_retryable = exc_to_check.has_error_label("RetryableError")
2828+
always_retryable = (
2829+
exc_to_check.has_error_label("RetryableError") and self._retryable
2830+
)
28292831
overloaded = exc_to_check.has_error_label("SystemOverloadedError")
28302832
if not self._retryable and not always_retryable:
28312833
raise

‎pymongo/asynchronous/pool.py‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,9 @@ async def backoff(self) -> None:
10541054
backoff_duration_ms = int(backoff_duration_sec * 1000)
10551055
if self.state != PoolState.BACKOFF:
10561056
self.state = PoolState.BACKOFF
1057+
# Cancel other pending connections.
1058+
for context in self.active_contexts:
1059+
context.cancel()
10571060
if self.enabled_for_cmap:
10581061
assert self.opts._event_listeners is not None
10591062
self.opts._event_listeners.publish_pool_backoff(
@@ -1083,10 +1086,6 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
10831086
Note that the pool does not keep a reference to the socket -- you
10841087
must call checkin() when you're done with it.
10851088
"""
1086-
# Mark whether we were in ready state before starting the process, to
1087-
# handle the case of multiple pending connections.
1088-
was_ready = self.state == PoolState.READY
1089-
10901089
async with self.lock:
10911090
conn_id = self.next_connection_id
10921091
self.next_connection_id += 1
@@ -1135,9 +1134,7 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
11351134
reason=_verbose_connection_error_reason(ConnectionClosedReason.ERROR),
11361135
error=ConnectionClosedReason.ERROR,
11371136
)
1138-
if context["has_created_socket"] and not (
1139-
was_ready and self.state == PoolState.BACKOFF
1140-
):
1137+
if context["has_created_socket"]:
11411138
await self._handle_connection_error(error, "handshake")
11421139
if isinstance(error, (IOError, OSError, *SSLErrors)):
11431140
details = _get_timeout_details(self.opts)
@@ -1164,7 +1161,7 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
11641161
except BaseException as e:
11651162
async with self.lock:
11661163
self.active_contexts.discard(conn.cancel_context)
1167-
if not has_completed_hello and not (was_ready and self.state == PoolState.BACKOFF):
1164+
if not has_completed_hello:
11681165
await self._handle_connection_error(e, "hello")
11691166
await conn.close_conn(ConnectionClosedReason.ERROR)
11701167
raise

‎pymongo/synchronous/mongo_client.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2792,7 +2792,7 @@ def run(self) -> T:
27922792
if isinstance(exc, (ConnectionFailure, OperationFailure)):
27932793
# ConnectionFailures do not supply a code property
27942794
exc_code = getattr(exc, "code", None)
2795-
always_retryable = exc.has_error_label("RetryableError")
2795+
always_retryable = exc.has_error_label("RetryableError") and self._retryable
27962796
overloaded = exc.has_error_label("SystemOverloadedError")
27972797
if not always_retryable and (
27982798
self._is_not_eligible_for_retry()
@@ -2815,7 +2815,9 @@ def run(self) -> T:
28152815
):
28162816
exc_to_check = exc.error
28172817
retryable_write_label = exc_to_check.has_error_label("RetryableWriteError")
2818-
always_retryable = exc_to_check.has_error_label("RetryableError")
2818+
always_retryable = (
2819+
exc_to_check.has_error_label("RetryableError") and self._retryable
2820+
)
28192821
overloaded = exc_to_check.has_error_label("SystemOverloadedError")
28202822
if not self._retryable and not always_retryable:
28212823
raise

‎pymongo/synchronous/pool.py‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,9 @@ def backoff(self) -> None:
10521052
backoff_duration_ms = int(backoff_duration_sec * 1000)
10531053
if self.state != PoolState.BACKOFF:
10541054
self.state = PoolState.BACKOFF
1055+
# Cancel other pending connections.
1056+
for context in self.active_contexts:
1057+
context.cancel()
10551058
if self.enabled_for_cmap:
10561059
assert self.opts._event_listeners is not None
10571060
self.opts._event_listeners.publish_pool_backoff(
@@ -1081,10 +1084,6 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
10811084
Note that the pool does not keep a reference to the socket -- you
10821085
must call checkin() when you're done with it.
10831086
"""
1084-
# Mark whether we were in ready state before starting the process, to
1085-
# handle the case of multiple pending connections.
1086-
was_ready = self.state == PoolState.READY
1087-
10881087
with self.lock:
10891088
conn_id = self.next_connection_id
10901089
self.next_connection_id += 1
@@ -1133,9 +1132,7 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
11331132
reason=_verbose_connection_error_reason(ConnectionClosedReason.ERROR),
11341133
error=ConnectionClosedReason.ERROR,
11351134
)
1136-
if context["has_created_socket"] and not (
1137-
was_ready and self.state == PoolState.BACKOFF
1138-
):
1135+
if context["has_created_socket"]:
11391136
self._handle_connection_error(error, "handshake")
11401137
if isinstance(error, (IOError, OSError, *SSLErrors)):
11411138
details = _get_timeout_details(self.opts)
@@ -1162,7 +1159,7 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
11621159
except BaseException as e:
11631160
with self.lock:
11641161
self.active_contexts.discard(conn.cancel_context)
1165-
if not has_completed_hello and not (was_ready and self.state == PoolState.BACKOFF):
1162+
if not has_completed_hello:
11661163
self._handle_connection_error(e, "hello")
11671164
conn.close_conn(ConnectionClosedReason.ERROR)
11681165
raise

‎test/discovery_and_monitoring/unified/backoff-heartbeat-failure.json‎

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -132,39 +132,21 @@
132132
"arguments": {
133133
"client": "client",
134134
"event": {
135-
"serverHeartbeatFailedEvent": {}
135+
"poolBackoffEvent": {}
136136
},
137137
"count": 1
138138
}
139-
}
140-
],
141-
"expectEvents": [
139+
},
142140
{
143-
"client": "client",
144-
"eventType": "cmap",
145-
"events": [
146-
{
147-
"poolBackoffEvent": {}
148-
},
149-
{
150-
"poolBackoffEvent": {}
151-
},
152-
{
153-
"poolBackoffEvent": {}
154-
},
155-
{
156-
"poolBackoffEvent": {}
157-
},
158-
{
159-
"poolBackoffEvent": {}
160-
},
161-
{
162-
"poolBackoffEvent": {}
141+
"name": "waitForEvent",
142+
"object": "testRunner",
143+
"arguments": {
144+
"client": "client",
145+
"event": {
146+
"serverHeartbeatFailedEvent": {}
163147
},
164-
{
165-
"poolClearedEvent": {}
166-
}
167-
]
148+
"count": 1
149+
}
168150
}
169151
]
170152
}

‎test/discovery_and_monitoring/unified/backoff-heartbeat-success.json‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,17 @@
126126
"isError": true
127127
}
128128
},
129+
{
130+
"name": "waitForEvent",
131+
"object": "testRunner",
132+
"arguments": {
133+
"client": "client",
134+
"event": {
135+
"poolBackoffEvent": {}
136+
},
137+
"count": 1
138+
}
139+
},
129140
{
130141
"name": "failPoint",
131142
"object": "testRunner",
@@ -148,32 +159,6 @@
148159
"count": 1
149160
}
150161
}
151-
],
152-
"expectEvents": [
153-
{
154-
"client": "client",
155-
"eventType": "cmap",
156-
"events": [
157-
{
158-
"poolBackoffEvent": {}
159-
},
160-
{
161-
"poolBackoffEvent": {}
162-
},
163-
{
164-
"poolBackoffEvent": {}
165-
},
166-
{
167-
"poolBackoffEvent": {}
168-
},
169-
{
170-
"poolBackoffEvent": {}
171-
},
172-
{
173-
"poolBackoffEvent": {}
174-
}
175-
]
176-
}
177162
]
178163
}
179164
]

‎test/discovery_and_monitoring/unified/backoff-network-error-fail.json‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,23 @@
126126
"isError": true
127127
}
128128
},
129+
{
130+
"name": "insertMany",
131+
"object": "collection",
132+
"arguments": {
133+
"documents": [
134+
{
135+
"_id": 3
136+
},
137+
{
138+
"_id": 4
139+
}
140+
]
141+
},
142+
"expectError": {
143+
"isError": true
144+
}
145+
},
129146
{
130147
"name": "waitForEvent",
131148
"object": "testRunner",
@@ -134,7 +151,7 @@
134151
"event": {
135152
"poolBackoffEvent": {}
136153
},
137-
"count": 5
154+
"count": 1
138155
}
139156
}
140157
]
@@ -238,7 +255,7 @@
238255
"event": {
239256
"poolBackoffEvent": {}
240257
},
241-
"count": 5
258+
"count": 1
242259
}
243260
},
244261
{

‎test/discovery_and_monitoring/unified/backoff-network-error.json‎

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@
125125
"_id": 4
126126
}
127127
]
128+
},
129+
"expectError": {
130+
"isError": true
128131
}
129132
},
130133
{
@@ -139,64 +142,6 @@
139142
},
140143
"count": 1
141144
}
142-
},
143-
{
144-
"name": "waitForEvent",
145-
"object": "testRunner",
146-
"arguments": {
147-
"client": "client",
148-
"event": {
149-
"poolBackoffEvent": {
150-
"attempt": 2
151-
}
152-
},
153-
"count": 1
154-
}
155-
}
156-
],
157-
"expectEvents": [
158-
{
159-
"client": "client",
160-
"eventType": "command",
161-
"events": [
162-
{
163-
"commandStartedEvent": {
164-
"command": {
165-
"insert": "backoff-network-error",
166-
"documents": [
167-
{
168-
"_id": 3
169-
},
170-
{
171-
"_id": 4
172-
}
173-
]
174-
},
175-
"commandName": "insert",
176-
"databaseName": "sdam-tests"
177-
}
178-
}
179-
]
180-
}
181-
],
182-
"outcome": [
183-
{
184-
"collectionName": "backoff-network-error",
185-
"databaseName": "sdam-tests",
186-
"documents": [
187-
{
188-
"_id": 1
189-
},
190-
{
191-
"_id": 2
192-
},
193-
{
194-
"_id": 3
195-
},
196-
{
197-
"_id": 4
198-
}
199-
]
200145
}
201146
]
202147
}

0 commit comments

Comments
 (0)