From cf3b483e4402aab5a7651eb8a0417191859f87bc Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Fri, 7 Oct 2022 18:31:06 -0600 Subject: [PATCH 1/7] Propagate the original error for write errors labeled `NoWritesPerformed` JAVA-4701 --- .../operation/CommandOperationHelper.java | 5 +- .../client/RetryableWritesProseTest.java | 9 +++ .../client/RetryableWritesProseTest.java | 70 ++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java b/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java index 1985e41a5ce..8d63e3fa0fd 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java +++ b/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java @@ -163,7 +163,9 @@ static Throwable chooseRetryableWriteException( return mostRecentAttemptException.getCause(); } return mostRecentAttemptException; - } else if (mostRecentAttemptException instanceof ResourceSupplierInternalException) { + } else if (mostRecentAttemptException instanceof ResourceSupplierInternalException + || (mostRecentAttemptException instanceof MongoException + && ((MongoException)mostRecentAttemptException).hasErrorLabel(NO_WRITES_PERFORMED_ERROR_LABEL))) { return previouslyChosenException; } else { return mostRecentAttemptException; @@ -571,6 +573,7 @@ private static boolean isRetryWritesEnabled(@Nullable final BsonDocument command } static final String RETRYABLE_WRITE_ERROR_LABEL = "RetryableWriteError"; + private static final String NO_WRITES_PERFORMED_ERROR_LABEL = "NoWritesPerformed"; private static boolean decideRetryableAndAddRetryableWriteErrorLabel(final Throwable t, @Nullable final Integer maxWireVersion) { if (!(t instanceof MongoException)) { diff --git a/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java index 904137a06dc..5b38c8fbc47 100644 --- a/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java +++ b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java @@ -98,6 +98,15 @@ public void poolClearedExceptionMustBeRetryable() throws InterruptedException, E mongoCollection -> mongoCollection.insertOne(new Document()), "insert", true); } + /** + * Prose test #3. + */ + @Test + public void originalErrorMustBePropagatedIfNoWritesPerformed() throws InterruptedException { + com.mongodb.client.RetryableWritesProseTest.originalErrorMustBePropagatedIfNoWritesPerformed( + mongoClientSettings -> new SyncMongoClient(MongoClients.create(mongoClientSettings))); + } + private boolean canRunTests() { Document storageEngine = (Document) getServerStatus().get("storageEngine"); diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index 4944a9c3856..98365174f6a 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -19,7 +19,12 @@ import com.mongodb.Function; import com.mongodb.MongoClientException; import com.mongodb.MongoClientSettings; +import com.mongodb.MongoCommandException; import com.mongodb.MongoException; +import com.mongodb.ServerAddress; +import com.mongodb.assertions.Assertions; +import com.mongodb.event.CommandFailedEvent; +import com.mongodb.event.CommandListener; import com.mongodb.event.ConnectionCheckOutFailedEvent; import com.mongodb.event.ConnectionCheckedOutEvent; import com.mongodb.event.ConnectionPoolClearedEvent; @@ -34,12 +39,17 @@ import org.junit.Before; import org.junit.Test; +import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiFunction; +import java.util.stream.Collectors; import static com.mongodb.ClusterFixture.getServerStatus; import static com.mongodb.ClusterFixture.isDiscoverableReplicaSet; @@ -55,6 +65,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; @@ -138,7 +149,6 @@ public static void poolClearedExceptionMustBeRetryable( * As a result, the client has to wait for at least its heartbeat delay until it hears back from a server * (while it waits for a response, calling `ServerMonitor.connect` has no effect). * Thus, we want to use small heartbeat delay to reduce delays in the test. */ - .minHeartbeatFrequency(50, TimeUnit.MILLISECONDS) .heartbeatFrequency(50, TimeUnit.MILLISECONDS)) .retryReads(true) .retryWrites(true) @@ -179,6 +189,64 @@ public static void poolClearedExceptionMustBeRetryable( } } + /** + * Prose test #3. + */ + @Test + public void originalErrorMustBePropagatedIfNoWritesPerformed() throws InterruptedException { + originalErrorMustBePropagatedIfNoWritesPerformed(MongoClients::create); + } + + public static void originalErrorMustBePropagatedIfNoWritesPerformed( + final Function clientCreator) throws InterruptedException { + assumeTrue(serverVersionAtLeast(6, 0) && isDiscoverableReplicaSet()); + BiFunction, BsonDocument> configureFailPointDocCreator = (errorCode, errorLabels) -> + new BsonDocument() + .append("configureFailPoint", new BsonString("failCommand")) + .append("mode", new BsonDocument() + .append("times", new BsonInt32(1))) + .append("data", new BsonDocument() + .append("failCommands", new BsonArray(singletonList(new BsonString("insert")))) + .append("errorCode", new BsonInt32(errorCode)) + .append("errorLabels", new BsonArray(errorLabels.stream().map(BsonString::new).collect(Collectors.toList())))); + ServerAddress primaryServerAddress = Fixture.getPrimary(); + CompletableFuture futureFailPoint = new CompletableFuture<>(); + CommandListener commandListener = new CommandListener() { + private final AtomicBoolean configureFailPoint = new AtomicBoolean(true); + + @Override + public void commandFailed(final CommandFailedEvent event) { + if (event.getCommandName().equals("insert") && configureFailPoint.compareAndSet(true, false)) { + Assertions.assertTrue(futureFailPoint.complete(FailPoint.enable( + configureFailPointDocCreator.apply(10107, asList("RetryableWriteError", "NoWritesPerformed")), + primaryServerAddress + ))); + } + } + }; + try (MongoClient client = clientCreator.apply(getMongoClientSettingsBuilder() + .retryWrites(true) + .addCommandListener(commandListener) + .applyToServerSettings(builder -> + // see `poolClearedExceptionMustBeRetryable` for the explanation + builder.heartbeatFrequency(50, TimeUnit.MILLISECONDS)) + .build()); + FailPoint ignored = FailPoint.enable(configureFailPointDocCreator.apply(91, singletonList("RetryableWriteError")), client)) { + MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) + .getCollection("originalErrorMustBePropagatedIfErrorWithRetryableWriteErrorLabelHappens"); + collection.drop(); + try { + collection.insertOne(new Document()); + } catch (MongoCommandException e) { + assertEquals(e.getErrorCode(), 91); + return; + } + fail("must not reach"); + } finally { + futureFailPoint.thenAccept(FailPoint::close); + } + } + private boolean canRunTests() { Document storageEngine = (Document) getServerStatus().get("storageEngine"); From f9759ae9cd6ab4d89bbfe903a3f02b047dcb5a23 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 11 Oct 2022 11:38:44 -0600 Subject: [PATCH 2/7] Fix the first fail point in the test JAVA-4701 --- .../client/RetryableWritesProseTest.java | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index 98365174f6a..68d14f087ab 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -19,15 +19,15 @@ import com.mongodb.Function; import com.mongodb.MongoClientException; import com.mongodb.MongoClientSettings; -import com.mongodb.MongoCommandException; import com.mongodb.MongoException; import com.mongodb.ServerAddress; import com.mongodb.assertions.Assertions; -import com.mongodb.event.CommandFailedEvent; import com.mongodb.event.CommandListener; +import com.mongodb.event.CommandSucceededEvent; import com.mongodb.event.ConnectionCheckOutFailedEvent; import com.mongodb.event.ConnectionCheckedOutEvent; import com.mongodb.event.ConnectionPoolClearedEvent; +import com.mongodb.internal.connection.MongoWriteConcernWithResponseException; import com.mongodb.internal.connection.TestCommandListener; import com.mongodb.internal.connection.TestConnectionPoolListener; import org.bson.BsonArray; @@ -39,7 +39,6 @@ import org.junit.Before; import org.junit.Test; -import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -48,8 +47,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.BiFunction; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.mongodb.ClusterFixture.getServerStatus; import static com.mongodb.ClusterFixture.isDiscoverableReplicaSet; @@ -200,30 +199,39 @@ public void originalErrorMustBePropagatedIfNoWritesPerformed() throws Interrupte public static void originalErrorMustBePropagatedIfNoWritesPerformed( final Function clientCreator) throws InterruptedException { assumeTrue(serverVersionAtLeast(6, 0) && isDiscoverableReplicaSet()); - BiFunction, BsonDocument> configureFailPointDocCreator = (errorCode, errorLabels) -> - new BsonDocument() - .append("configureFailPoint", new BsonString("failCommand")) - .append("mode", new BsonDocument() - .append("times", new BsonInt32(1))) - .append("data", new BsonDocument() - .append("failCommands", new BsonArray(singletonList(new BsonString("insert")))) - .append("errorCode", new BsonInt32(errorCode)) - .append("errorLabels", new BsonArray(errorLabels.stream().map(BsonString::new).collect(Collectors.toList())))); ServerAddress primaryServerAddress = Fixture.getPrimary(); - CompletableFuture futureFailPoint = new CompletableFuture<>(); + CompletableFuture futureFailPointFromListener = new CompletableFuture<>(); CommandListener commandListener = new CommandListener() { private final AtomicBoolean configureFailPoint = new AtomicBoolean(true); @Override - public void commandFailed(final CommandFailedEvent event) { + public void commandSucceeded(final CommandSucceededEvent event) { if (event.getCommandName().equals("insert") && configureFailPoint.compareAndSet(true, false)) { - Assertions.assertTrue(futureFailPoint.complete(FailPoint.enable( - configureFailPointDocCreator.apply(10107, asList("RetryableWriteError", "NoWritesPerformed")), + Assertions.assertTrue(futureFailPointFromListener.complete(FailPoint.enable( + new BsonDocument() + .append("configureFailPoint", new BsonString("failCommand")) + .append("mode", new BsonDocument() + .append("times", new BsonInt32(1))) + .append("data", new BsonDocument() + .append("failCommands", new BsonArray(singletonList(new BsonString("insert")))) + .append("errorCode", new BsonInt32(10107)) + .append("errorLabels", new BsonArray(Stream.of("RetryableWriteError", "NoWritesPerformed") + .map(BsonString::new).collect(Collectors.toList())))), primaryServerAddress ))); } } }; + BsonDocument failPointDocument = new BsonDocument() + .append("configureFailPoint", new BsonString("failCommand")) + .append("mode", new BsonDocument() + .append("times", new BsonInt32(1))) + .append("data", new BsonDocument() + .append("writeConcernError", new BsonDocument() + .append("code", new BsonInt32(91)) + .append("errorLabels", new BsonArray(Stream.of("RetryableWriteError") + .map(BsonString::new).collect(Collectors.toList())))) + .append("failCommands", new BsonArray(singletonList(new BsonString("insert"))))); try (MongoClient client = clientCreator.apply(getMongoClientSettingsBuilder() .retryWrites(true) .addCommandListener(commandListener) @@ -231,19 +239,19 @@ public void commandFailed(final CommandFailedEvent event) { // see `poolClearedExceptionMustBeRetryable` for the explanation builder.heartbeatFrequency(50, TimeUnit.MILLISECONDS)) .build()); - FailPoint ignored = FailPoint.enable(configureFailPointDocCreator.apply(91, singletonList("RetryableWriteError")), client)) { + FailPoint ignored = FailPoint.enable(failPointDocument, client)) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) - .getCollection("originalErrorMustBePropagatedIfErrorWithRetryableWriteErrorLabelHappens"); + .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); collection.drop(); try { collection.insertOne(new Document()); - } catch (MongoCommandException e) { - assertEquals(e.getErrorCode(), 91); + } catch (MongoWriteConcernWithResponseException e) { + assertEquals(e.getCode(), 91); return; } fail("must not reach"); } finally { - futureFailPoint.thenAccept(FailPoint::close); + futureFailPointFromListener.thenAccept(FailPoint::close); } } From 7cc1aaec66b2cc27782dedf24311e1bc5ce7e8ee Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Tue, 11 Oct 2022 12:17:51 -0600 Subject: [PATCH 3/7] Fix the first fail point in the test and document some error labels JAVA-4701 --- .../src/main/com/mongodb/MongoException.java | 16 ++++++++++++++++ .../operation/CommandOperationHelper.java | 2 +- .../mongodb/client/RetryableWritesProseTest.java | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/MongoException.java b/driver-core/src/main/com/mongodb/MongoException.java index 798ceda09b1..c96e253c34d 100644 --- a/driver-core/src/main/com/mongodb/MongoException.java +++ b/driver-core/src/main/com/mongodb/MongoException.java @@ -171,6 +171,22 @@ public void removeLabel(final String errorLabel) { /** * Gets the set of error labels associated with this exception. + * The following list of labels is not exhaustive, the labels may also be both added and removed in the future. + * + * + * + * + * + * + * + * + * + * + * + *
Some error labels
Error labelDescription
{@code RetryableWriteError}Tells the driver, not the user of the driver, that the command may be retried. + * It is not necessary safe for a user to retry the corresponding failed command, unless either the error + * also has the {@code NoWritesPerformed} label, or the error is explicitly documented as safely retryable, + * like {@link MongoConnectionPoolClearedException}.
{@code NoWritesPerformed}Tells the user of the driver that the command may be safely retried.
* * @return the error labels, which may not be null but may be empty * @since 3.8 diff --git a/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java b/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java index 8d63e3fa0fd..eb0f8e6817d 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java +++ b/driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java @@ -165,7 +165,7 @@ static Throwable chooseRetryableWriteException( return mostRecentAttemptException; } else if (mostRecentAttemptException instanceof ResourceSupplierInternalException || (mostRecentAttemptException instanceof MongoException - && ((MongoException)mostRecentAttemptException).hasErrorLabel(NO_WRITES_PERFORMED_ERROR_LABEL))) { + && ((MongoException) mostRecentAttemptException).hasErrorLabel(NO_WRITES_PERFORMED_ERROR_LABEL))) { return previouslyChosenException; } else { return mostRecentAttemptException; diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index 68d14f087ab..ab9e793dea8 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -196,6 +196,7 @@ public void originalErrorMustBePropagatedIfNoWritesPerformed() throws Interrupte originalErrorMustBePropagatedIfNoWritesPerformed(MongoClients::create); } + @SuppressWarnings("try") public static void originalErrorMustBePropagatedIfNoWritesPerformed( final Function clientCreator) throws InterruptedException { assumeTrue(serverVersionAtLeast(6, 0) && isDiscoverableReplicaSet()); From 612b7e7899a6ca9bb23bbce9b83419183b80cb54 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Wed, 12 Oct 2022 01:28:04 -0600 Subject: [PATCH 4/7] Is this the fix? --- ...ixedBulkWriteOperationSpecification.groovy | 8 ++-- .../com/mongodb/client/FailPoint.java | 23 ++++-------- .../com/mongodb/client/Fixture.java | 37 +++++++++++++++++-- .../client/RetryableWritesProseTest.java | 4 +- 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy b/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy index 685e7db1963..543a4a00e46 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy +++ b/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy @@ -846,14 +846,14 @@ class MixedBulkWriteOperationSpecification extends OperationFunctionalSpecificat given: getCollectionHelper().insertDocuments(getTestInserts()) def operation = new MixedBulkWriteOperation(getNamespace(), - [new InsertRequest(new BsonDocument('_id', new BsonInt32(7))), - new InsertRequest(new BsonDocument('_id', new BsonInt32(1))) // duplicate key - ], false, ACKNOWLEDGED, true) + [new DeleteRequest(new BsonDocument('_id', new BsonInt32(2))), // existing key + new InsertRequest(new BsonDocument('_id', new BsonInt32(1))) // existing (duplicate) key + ], true, ACKNOWLEDGED, true) def failPoint = BsonDocument.parse('''{ "configureFailPoint": "failCommand", "mode": {"times": 2 }, - "data": { "failCommands": ["insert"], + "data": { "failCommands": ["delete"], "writeConcernError": {"code": 91, "errmsg": "Replication is being shut down"}}}''') configureFailPoint(failPoint) diff --git a/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java b/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java index bc88b8eab33..4138a0e2133 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java +++ b/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java @@ -29,17 +29,17 @@ public final class FailPoint implements AutoCloseable { private final BsonDocument failPointDocument; private final MongoClient client; - private final boolean close; - private FailPoint(final BsonDocument failPointDocument, final MongoClient client, final boolean close) { + private FailPoint(final BsonDocument failPointDocument, final MongoClient client) { this.failPointDocument = failPointDocument.toBsonDocument(); this.client = client; - this.close = close; } /** * @param configureFailPointDoc A document representing {@code configureFailPoint} command to be issued as is via * {@link com.mongodb.client.MongoDatabase#runCommand(Bson)}. + * @param serverAddress One may use {@link Fixture#getPrimary(MongoClient)} to get the address of a primary server + * if that is what is needed. */ public static FailPoint enable(final BsonDocument configureFailPointDoc, final ServerAddress serverAddress) { MongoClientSettings clientSettings = getMongoClientSettingsBuilder() @@ -48,18 +48,11 @@ public static FailPoint enable(final BsonDocument configureFailPointDoc, final S .hosts(Collections.singletonList(serverAddress))) .build(); MongoClient client = MongoClients.create(clientSettings); - return enable(configureFailPointDoc, client, true); + return enable(configureFailPointDoc, client); } - /** - * @see #enable(BsonDocument, ServerAddress) - */ - public static FailPoint enable(final BsonDocument configureFailPointDoc, final MongoClient client) { - return enable(configureFailPointDoc, client, false); - } - - private static FailPoint enable(final BsonDocument configureFailPointDoc, final MongoClient client, final boolean close) { - FailPoint result = new FailPoint(configureFailPointDoc, client, close); + private static FailPoint enable(final BsonDocument configureFailPointDoc, final MongoClient client) { + FailPoint result = new FailPoint(configureFailPointDoc, client); client.getDatabase("admin").runCommand(configureFailPointDoc); return result; } @@ -71,9 +64,7 @@ public void close() { .append("configureFailPoint", failPointDocument.getString("configureFailPoint")) .append("mode", new BsonString("off"))); } finally { - if (close) { - client.close(); - } + client.close(); } } } diff --git a/driver-sync/src/test/functional/com/mongodb/client/Fixture.java b/driver-sync/src/test/functional/com/mongodb/client/Fixture.java index 468dddb617b..4853ca82a92 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/Fixture.java +++ b/driver-sync/src/test/functional/com/mongodb/client/Fixture.java @@ -20,10 +20,13 @@ import com.mongodb.MongoClientSettings; import com.mongodb.ServerAddress; import com.mongodb.client.internal.MongoClientImpl; +import com.mongodb.connection.ClusterDescription; import com.mongodb.connection.ServerDescription; +import java.lang.reflect.InvocationTargetException; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import static com.mongodb.ClusterFixture.getConnectionString; import static com.mongodb.ClusterFixture.getMultiMongosConnectionString; @@ -110,11 +113,39 @@ public static MongoClientSettings.Builder getMongoClientSettings(final Connectio } public static ServerAddress getPrimary() throws InterruptedException { - getMongoClient(); - List serverDescriptions = getPrimaries(((MongoClientImpl) mongoClient).getCluster().getDescription()); + return getPrimary(getMongoClient()); + } + + /** + * Beware of a potential race condition hiding here: the primary you discover may differ from the one used by the {@code client} + * when performing some operations, as the primary may change. + */ + public static ServerAddress getPrimary(final MongoClient client) + throws InterruptedException { + final Supplier clusterDescriptionSupplier; + if (client instanceof MongoClientImpl) { + clusterDescriptionSupplier = () -> ((MongoClientImpl) client).getClusterDescription(); + } else { + // com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient + try { + Object wrappedReactiveClient = client.getClass().getMethod("getWrapped").invoke(client); + clusterDescriptionSupplier = + () -> { + try { + return (ClusterDescription) wrappedReactiveClient.getClass().getMethod("getClusterDescription") + .invoke(wrappedReactiveClient); + } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); + } + }; + } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + List serverDescriptions = getPrimaries(clusterDescriptionSupplier.get()); while (serverDescriptions.isEmpty()) { Thread.sleep(100); - serverDescriptions = getPrimaries(((MongoClientImpl) mongoClient).getCluster().getDescription()); + serverDescriptions = getPrimaries(clusterDescriptionSupplier.get()); } return serverDescriptions.get(0).getAddress(); } diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index ab9e793dea8..a037e36a4f6 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -167,7 +167,7 @@ public static void poolClearedExceptionMustBeRetryable( .append("blockTimeMS", new BsonInt32(1000))); int timeoutSeconds = 5; try (MongoClient client = clientCreator.apply(clientSettings); - FailPoint ignored = FailPoint.enable(configureFailPoint, client)) { + FailPoint ignored = FailPoint.enable(configureFailPoint, Fixture.getPrimary(client))) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("poolClearedExceptionMustBeRetryable"); collection.drop(); @@ -240,7 +240,7 @@ public void commandSucceeded(final CommandSucceededEvent event) { // see `poolClearedExceptionMustBeRetryable` for the explanation builder.heartbeatFrequency(50, TimeUnit.MILLISECONDS)) .build()); - FailPoint ignored = FailPoint.enable(failPointDocument, client)) { + FailPoint ignored = FailPoint.enable(failPointDocument, Fixture.getPrimary(client))) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); collection.drop(); From 5c4dedad44a43a4859db7df4adc3dfa3684a637b Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Wed, 12 Oct 2022 08:25:08 -0600 Subject: [PATCH 5/7] Undo javadoc changes because error labels are not part of the public API. --- .../src/main/com/mongodb/MongoException.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/driver-core/src/main/com/mongodb/MongoException.java b/driver-core/src/main/com/mongodb/MongoException.java index c96e253c34d..798ceda09b1 100644 --- a/driver-core/src/main/com/mongodb/MongoException.java +++ b/driver-core/src/main/com/mongodb/MongoException.java @@ -171,22 +171,6 @@ public void removeLabel(final String errorLabel) { /** * Gets the set of error labels associated with this exception. - * The following list of labels is not exhaustive, the labels may also be both added and removed in the future. - * - * - * - * - * - * - * - * - * - * - * - *
Some error labels
Error labelDescription
{@code RetryableWriteError}Tells the driver, not the user of the driver, that the command may be retried. - * It is not necessary safe for a user to retry the corresponding failed command, unless either the error - * also has the {@code NoWritesPerformed} label, or the error is explicitly documented as safely retryable, - * like {@link MongoConnectionPoolClearedException}.
{@code NoWritesPerformed}Tells the user of the driver that the command may be safely retried.
* * @return the error labels, which may not be null but may be empty * @since 3.8 From 224315e37119280eebce3c86b8370c040422b1e2 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Wed, 12 Oct 2022 16:18:26 -0600 Subject: [PATCH 6/7] Address review concerns --- .../com/mongodb/client/Fixture.java | 36 +++---------------- .../client/RetryableWritesProseTest.java | 12 ++++--- 2 files changed, 11 insertions(+), 37 deletions(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/Fixture.java b/driver-sync/src/test/functional/com/mongodb/client/Fixture.java index 4853ca82a92..3b0d45dca88 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/Fixture.java +++ b/driver-sync/src/test/functional/com/mongodb/client/Fixture.java @@ -19,14 +19,10 @@ import com.mongodb.ConnectionString; import com.mongodb.MongoClientSettings; import com.mongodb.ServerAddress; -import com.mongodb.client.internal.MongoClientImpl; -import com.mongodb.connection.ClusterDescription; import com.mongodb.connection.ServerDescription; -import java.lang.reflect.InvocationTargetException; import java.util.List; import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; import static com.mongodb.ClusterFixture.getConnectionString; import static com.mongodb.ClusterFixture.getMultiMongosConnectionString; @@ -112,40 +108,16 @@ public static MongoClientSettings.Builder getMongoClientSettings(final Connectio return builder; } - public static ServerAddress getPrimary() throws InterruptedException { - return getPrimary(getMongoClient()); - } - /** * Beware of a potential race condition hiding here: the primary you discover may differ from the one used by the {@code client} * when performing some operations, as the primary may change. */ - public static ServerAddress getPrimary(final MongoClient client) - throws InterruptedException { - final Supplier clusterDescriptionSupplier; - if (client instanceof MongoClientImpl) { - clusterDescriptionSupplier = () -> ((MongoClientImpl) client).getClusterDescription(); - } else { - // com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient - try { - Object wrappedReactiveClient = client.getClass().getMethod("getWrapped").invoke(client); - clusterDescriptionSupplier = - () -> { - try { - return (ClusterDescription) wrappedReactiveClient.getClass().getMethod("getClusterDescription") - .invoke(wrappedReactiveClient); - } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { - throw new RuntimeException(e); - } - }; - } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { - throw new RuntimeException(e); - } - } - List serverDescriptions = getPrimaries(clusterDescriptionSupplier.get()); + public static ServerAddress getPrimary() throws InterruptedException { + MongoClient client = getMongoClient(); + List serverDescriptions = getPrimaries(client.getClusterDescription()); while (serverDescriptions.isEmpty()) { Thread.sleep(100); - serverDescriptions = getPrimaries(clusterDescriptionSupplier.get()); + serverDescriptions = getPrimaries(client.getClusterDescription()); } return serverDescriptions.get(0).getAddress(); } diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index a037e36a4f6..0d7e9cd2019 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -27,7 +27,6 @@ import com.mongodb.event.ConnectionCheckOutFailedEvent; import com.mongodb.event.ConnectionCheckedOutEvent; import com.mongodb.event.ConnectionPoolClearedEvent; -import com.mongodb.internal.connection.MongoWriteConcernWithResponseException; import com.mongodb.internal.connection.TestCommandListener; import com.mongodb.internal.connection.TestConnectionPoolListener; import org.bson.BsonArray; @@ -167,7 +166,7 @@ public static void poolClearedExceptionMustBeRetryable( .append("blockTimeMS", new BsonInt32(1000))); int timeoutSeconds = 5; try (MongoClient client = clientCreator.apply(clientSettings); - FailPoint ignored = FailPoint.enable(configureFailPoint, Fixture.getPrimary(client))) { + FailPoint ignored = FailPoint.enable(configureFailPoint, Fixture.getPrimary())) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("poolClearedExceptionMustBeRetryable"); collection.drop(); @@ -207,7 +206,10 @@ public static void originalErrorMustBePropagatedIfNoWritesPerformed( @Override public void commandSucceeded(final CommandSucceededEvent event) { - if (event.getCommandName().equals("insert") && configureFailPoint.compareAndSet(true, false)) { + if (event.getCommandName().equals("insert") + && event.getResponse().getDocument("writeConcernError", new BsonDocument()) + .getInt32("code", new BsonInt32(-1)).intValue() == 91 + && configureFailPoint.compareAndSet(true, false)) { Assertions.assertTrue(futureFailPointFromListener.complete(FailPoint.enable( new BsonDocument() .append("configureFailPoint", new BsonString("failCommand")) @@ -240,13 +242,13 @@ public void commandSucceeded(final CommandSucceededEvent event) { // see `poolClearedExceptionMustBeRetryable` for the explanation builder.heartbeatFrequency(50, TimeUnit.MILLISECONDS)) .build()); - FailPoint ignored = FailPoint.enable(failPointDocument, Fixture.getPrimary(client))) { + FailPoint ignored = FailPoint.enable(failPointDocument, primaryServerAddress)) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); collection.drop(); try { collection.insertOne(new Document()); - } catch (MongoWriteConcernWithResponseException e) { + } catch (MongoException e) { assertEquals(e.getCode(), 91); return; } From 58438ffb9998237701f07136582ca9311f272765 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Fri, 14 Oct 2022 14:28:58 -0600 Subject: [PATCH 7/7] Fix javadocs link --- .../src/test/functional/com/mongodb/client/FailPoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java b/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java index 4138a0e2133..52e8fe9ff58 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java +++ b/driver-sync/src/test/functional/com/mongodb/client/FailPoint.java @@ -38,7 +38,7 @@ private FailPoint(final BsonDocument failPointDocument, final MongoClient client /** * @param configureFailPointDoc A document representing {@code configureFailPoint} command to be issued as is via * {@link com.mongodb.client.MongoDatabase#runCommand(Bson)}. - * @param serverAddress One may use {@link Fixture#getPrimary(MongoClient)} to get the address of a primary server + * @param serverAddress One may use {@link Fixture#getPrimary()} to get the address of a primary server * if that is what is needed. */ public static FailPoint enable(final BsonDocument configureFailPointDoc, final ServerAddress serverAddress) {