Skip to content

Commit bce97d4

Browse files
nhachichanhachicha
committed
Apply backpressure labels in CMAP layer; align SDAM with spec
1 parent d423093 commit bce97d4

6 files changed

Lines changed: 174 additions & 122 deletions

File tree

‎driver-core/src/main/com/mongodb/internal/connection/BackpressureErrorLabeler.java‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright 2008-present MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.internal.connection;
18+
19+
import com.mongodb.MongoException;
20+
import com.mongodb.MongoSecurityException;
21+
import com.mongodb.MongoSocketException;
22+
23+
import javax.net.ssl.SSLHandshakeException;
24+
import javax.net.ssl.SSLPeerUnverifiedException;
25+
import javax.net.ssl.SSLProtocolException;
26+
import java.net.UnknownHostException;
27+
import java.security.cert.CertPathBuilderException;
28+
import java.security.cert.CertPathValidatorException;
29+
import java.security.cert.CertificateException;
30+
import java.util.Locale;
31+
32+
/**
33+
* Attaches {@link MongoException#SYSTEM_OVERLOADED_ERROR_LABEL} and
34+
* {@link MongoException#RETRYABLE_ERROR_LABEL} to network errors encountered during connection
35+
* establishment or the hello message, per the CMAP specification.
36+
*
37+
* <p>This is topology-agnostic: it must be invoked from the connection-establishment path so that
38+
* both default SDAM and load-balanced modes are covered.
39+
*/
40+
final class BackpressureErrorLabeler {
41+
42+
private BackpressureErrorLabeler() {
43+
}
44+
45+
static void applyLabelsIfEligible(final Throwable t) {
46+
if (!(t instanceof MongoException)) {
47+
return;
48+
}
49+
if (t instanceof MongoSecurityException) {
50+
return;
51+
}
52+
if (!(t instanceof MongoSocketException)) {
53+
return;
54+
}
55+
if (isDnsLookupFailure(t)) {
56+
return;
57+
}
58+
if (isTlsConfigurationError(t)) {
59+
return;
60+
}
61+
// TODO-BACKPRESSURE Nabil - SOCKS5 Revisit alongside JAVA-5205 (SOCKS5 in async) so both sync and
62+
// async proxy error surfaces can be handled together — likely via a dedicated internal
63+
// exception thrown from the proxy code path.
64+
MongoException mongoException = (MongoException) t;
65+
mongoException.addLabel(MongoException.SYSTEM_OVERLOADED_ERROR_LABEL);
66+
mongoException.addLabel(MongoException.RETRYABLE_ERROR_LABEL);
67+
}
68+
69+
static boolean isDnsLookupFailure(final Throwable t) {
70+
Throwable cause = t.getCause();
71+
while (cause != null) {
72+
if (cause instanceof UnknownHostException) {
73+
return true;
74+
}
75+
cause = cause.getCause();
76+
}
77+
return false;
78+
}
79+
80+
static boolean isTlsConfigurationError(final Throwable t) {
81+
if (!(t instanceof MongoSocketException)) {
82+
return false;
83+
}
84+
Throwable cause = t.getCause();
85+
while (cause != null) {
86+
if (cause instanceof CertificateException
87+
|| cause instanceof CertPathBuilderException
88+
|| cause instanceof CertPathValidatorException
89+
|| cause instanceof SSLPeerUnverifiedException
90+
|| cause instanceof SSLProtocolException) {
91+
return true;
92+
}
93+
if (cause instanceof SSLHandshakeException) {
94+
String message = cause.getMessage();
95+
if (message != null) {
96+
String lowerMessage = message.toLowerCase(Locale.ROOT);
97+
if (lowerMessage.contains("certificate")
98+
|| lowerMessage.contains("verify")
99+
|| lowerMessage.contains("trust")
100+
|| lowerMessage.contains("hostname")
101+
|| lowerMessage.contains("protocol")
102+
|| lowerMessage.contains("cipher")
103+
|| lowerMessage.contains("handshake_failure")) {
104+
return true;
105+
}
106+
}
107+
}
108+
cause = cause.getCause();
109+
}
110+
return false;
111+
}
112+
}

‎driver-core/src/main/com/mongodb/internal/connection/DefaultSdamServerDescriptionManager.java‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.mongodb.internal.connection;
1818

19-
import com.mongodb.MongoException;
2019
import com.mongodb.annotations.ThreadSafe;
2120
import com.mongodb.connection.ClusterConnectionMode;
2221
import com.mongodb.connection.ServerDescription;
@@ -138,27 +137,12 @@ private void handleException(final SdamIssue sdamIssue, final boolean beforeHand
138137
serverMonitor.connect();
139138
} else if (sdamIssue.relatedToNetworkNotTimeout()
140139
|| (beforeHandshake && (sdamIssue.relatedToNetworkTimeout() || sdamIssue.relatedToAuth()))) {
141-
// Backpressure spec: Don't clear pool or mark server unknown for connection establishment failures
142-
// (network errors or timeouts during handshake). Authentication errors after handshake should still
143-
// clear the pool as they're not related to overload.
144-
// TLS configuration errors (certificate validation, protocol mismatches) should also clear the pool
145-
// as they indicate configuration issues, not server overload.
146-
if (beforeHandshake && !sdamIssue.relatedToAuth() && !sdamIssue.relatedToTlsConfigurationError()) {
147-
// Don't update server description to Unknown
148-
// Don't invalidate the connection pool
149-
// Apply error labels for backpressure
150-
sdamIssue.exception().ifPresent(exception -> {
151-
if (exception instanceof MongoException) {
152-
MongoException mongoException = (MongoException) exception;
153-
mongoException.addLabel(MongoException.SYSTEM_OVERLOADED_ERROR_LABEL);
154-
mongoException.addLabel(MongoException.RETRYABLE_ERROR_LABEL);
155-
}
156-
});
157-
} else {
158-
updateDescription(sdamIssue.serverDescription());
159-
connectionPool.invalidate(sdamIssue.exception().orElse(null));
160-
serverMonitor.cancelCurrentCheck();
140+
if (sdamIssue.hasBackpressureLabel()) {
141+
return;
161142
}
143+
updateDescription(sdamIssue.serverDescription());
144+
connectionPool.invalidate(sdamIssue.exception().orElse(null));
145+
serverMonitor.cancelCurrentCheck();
162146
} else if (sdamIssue.relatedToWriteConcern() || sdamIssue.relatedToStalePrimary()) {
163147
updateDescription(sdamIssue.serverDescription());
164148
serverMonitor.connect();

‎driver-core/src/main/com/mongodb/internal/connection/InternalStreamConnection.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,11 @@ public void open(final OperationContext originalOperationContext) {
230230
isTrue("Open already called", stream == null);
231231
stream = streamFactory.create(serverId.getAddress());
232232
OperationContext operationContext = originalOperationContext;
233+
boolean beforeHandshake = true;
233234
try {
234235
stream.open(operationContext);
235236
InternalConnectionInitializationDescription initializationDescription = connectionInitializer.startHandshake(this, operationContext);
237+
beforeHandshake = false;
236238

237239
operationContext = operationContext.withOverride(TimeoutContext::withNewlyStartedMaintenanceTimeout);
238240
initAfterHandshakeStart(initializationDescription);
@@ -241,6 +243,9 @@ public void open(final OperationContext originalOperationContext) {
241243
initAfterHandshakeFinish(initializationDescription);
242244
} catch (Throwable t) {
243245
close();
246+
if (beforeHandshake) {
247+
BackpressureErrorLabeler.applyLabelsIfEligible(t);
248+
}
244249
if (t instanceof MongoException) {
245250
throw (MongoException) t;
246251
} else {
@@ -263,6 +268,7 @@ public void completed(@Nullable final Void aVoid) {
263268
(initialResult, initialException) -> {
264269
if (initialException != null) {
265270
close();
271+
BackpressureErrorLabeler.applyLabelsIfEligible(initialException);
266272
callback.onResult(null, initialException);
267273
} else {
268274
assertNotNull(initialResult);
@@ -278,11 +284,13 @@ public void completed(@Nullable final Void aVoid) {
278284
@Override
279285
public void failed(final Throwable t) {
280286
close();
287+
BackpressureErrorLabeler.applyLabelsIfEligible(t);
281288
callback.onResult(null, t);
282289
}
283290
});
284291
} catch (Throwable t) {
285292
close();
293+
BackpressureErrorLabeler.applyLabelsIfEligible(t);
286294
callback.onResult(null, t);
287295
}
288296
}

‎driver-core/src/main/com/mongodb/internal/connection/SdamServerDescriptionManager.java‎

Lines changed: 4 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.mongodb.internal.connection;
1818

1919
import com.mongodb.MongoCommandException;
20+
import com.mongodb.MongoException;
2021
import com.mongodb.MongoNodeIsRecoveringException;
2122
import com.mongodb.MongoNotPrimaryException;
2223
import com.mongodb.MongoSecurityException;
@@ -30,12 +31,6 @@
3031
import com.mongodb.connection.TopologyVersion;
3132
import com.mongodb.lang.Nullable;
3233

33-
import javax.net.ssl.SSLHandshakeException;
34-
import javax.net.ssl.SSLPeerUnverifiedException;
35-
import javax.net.ssl.SSLProtocolException;
36-
import java.security.cert.CertPathBuilderException;
37-
import java.security.cert.CertPathValidatorException;
38-
import java.security.cert.CertificateException;
3934
import java.util.Optional;
4035

4136
import static com.mongodb.assertions.Assertions.assertNotNull;
@@ -168,47 +163,9 @@ boolean relatedToWriteConcern() {
168163
return exception instanceof MongoWriteConcernWithResponseException;
169164
}
170165

171-
/**
172-
* Checks if the exception is related to TLS configuration errors that are NOT due to server overload.
173-
* These include certificate validation failures, protocol mismatches, etc.
174-
*
175-
* @return true if this is a TLS configuration error (not network-related)
176-
*/
177-
boolean relatedToTlsConfigurationError() {
178-
if (!(exception instanceof MongoSocketException)) {
179-
return false;
180-
}
181-
Throwable cause = exception.getCause();
182-
while (cause != null) {
183-
// Check for various certificate validation and TLS configuration errors
184-
if (cause instanceof CertificateException
185-
|| cause instanceof CertPathBuilderException
186-
|| cause instanceof CertPathValidatorException
187-
|| cause instanceof SSLPeerUnverifiedException
188-
|| cause instanceof SSLProtocolException) {
189-
return true;
190-
}
191-
192-
// SSLHandshakeException can be either network or config, so we check the message
193-
if (cause instanceof SSLHandshakeException) {
194-
String message = cause.getMessage();
195-
if (message != null) {
196-
String lowerMessage = message.toLowerCase();
197-
// These indicate configuration issues, not network issues
198-
if (lowerMessage.contains("certificate")
199-
|| lowerMessage.contains("verify")
200-
|| lowerMessage.contains("trust")
201-
|| lowerMessage.contains("hostname")
202-
|| lowerMessage.contains("protocol")
203-
|| lowerMessage.contains("cipher")
204-
|| lowerMessage.contains("handshake_failure")) {
205-
return true;
206-
}
207-
}
208-
}
209-
cause = cause.getCause();
210-
}
211-
return false;
166+
boolean hasBackpressureLabel() {
167+
return exception instanceof MongoException
168+
&& ((MongoException) exception).hasErrorLabel(MongoException.SYSTEM_OVERLOADED_ERROR_LABEL);
212169
}
213170

214171
private static boolean stale(@Nullable final Throwable t, final ServerDescription currentServerDescription) {

‎driver-core/src/test/unit/com/mongodb/internal/connection/DefaultServerSpecification.groovy‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -234,12 +234,10 @@ class DefaultServerSpecification extends Specification {
234234
]
235235
}
236236

237-
def 'network error should not invalidate the pool'() {
237+
def 'failed open should invalidate the server'() {
238238
given:
239239
def connectionPool = Mock(ConnectionPool)
240-
connectionPool.get(_) >> {
241-
throw exceptionToThrow
242-
}
240+
connectionPool.get(_) >> { throw exceptionToThrow }
243241
def serverMonitor = Mock(ServerMonitor)
244242
def server = defaultServer(connectionPool, serverMonitor)
245243

@@ -249,8 +247,8 @@ class DefaultServerSpecification extends Specification {
249247
then:
250248
def e = thrown(MongoException)
251249
e.is(exceptionToThrow)
252-
0 * connectionPool.invalidate(_)
253-
0 * serverMonitor.cancelCurrentCheck()
250+
1 * connectionPool.invalidate(exceptionToThrow)
251+
1 * serverMonitor.cancelCurrentCheck()
254252

255253
where:
256254
exceptionToThrow << [
@@ -283,7 +281,7 @@ class DefaultServerSpecification extends Specification {
283281
]
284282
}
285283

286-
def 'failed open should not invalidate the pool asynchronously'() {
284+
def 'failed open should invalidate the server asynchronously'() {
287285
given:
288286
def connectionPool = Mock(ConnectionPool)
289287
connectionPool.getAsync(_, _) >> { it.last().onResult(null, exceptionToThrow) }
@@ -303,8 +301,8 @@ class DefaultServerSpecification extends Specification {
303301
then:
304302
!receivedConnection
305303
receivedThrowable.is(exceptionToThrow)
306-
0 * connectionPool.invalidate(exceptionToThrow)
307-
0 * serverMonitor.cancelCurrentCheck()
304+
1 * connectionPool.invalidate(exceptionToThrow)
305+
1 * serverMonitor.cancelCurrentCheck()
308306

309307

310308
where:

0 commit comments

Comments
 (0)