Skip to content

Commit dec7cb4

Browse files
DmitryLukyanovDmitryLukyanov
committed
CSHARP-3001: RetryableWrites specification improvements.
1 parent 990b978 commit dec7cb4

16 files changed

+365
-52
lines changed

‎src/MongoDB.Driver.Core/Core/Operations/RetryabilityHelper.cs

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
using System;
1717
using System.Collections.Generic;
18+
using MongoDB.Bson;
1819
using MongoDB.Driver.Core.Misc;
1920

2021
namespace MongoDB.Driver.Core.Operations
@@ -86,17 +87,20 @@ static RetryabilityHelper()
8687
}
8788

8889
// public static methods
89-
public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception)
90+
public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception, SemanticVersion serverVersion)
9091
{
91-
if (ShouldRetryableWriteExceptionLabelBeAdded(exception))
92+
if (ShouldRetryableWriteExceptionLabelBeAdded(exception, serverVersion))
9293
{
9394
exception.AddErrorLabel(RetryableWriteErrorLabel);
9495
}
9596
}
9697

97-
public static bool IsNetworkException(Exception exception)
98+
public static bool IsCommandRetryable(BsonDocument command)
9899
{
99-
return exception is MongoConnectionException mongoConnectionException && mongoConnectionException.IsNetworkException;
100+
return
101+
command.Contains("txnNumber") || // retryWrites=true
102+
command.Contains("commitTransaction") ||
103+
command.Contains("abortTransaction");
100104
}
101105

102106
public static bool IsResumableChangeStreamException(Exception exception, SemanticVersion serverVersion)
@@ -152,9 +156,29 @@ public static bool IsRetryableWriteException(Exception exception)
152156
}
153157

154158
// private static methods
155-
private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception)
159+
private static bool IsNetworkException(Exception exception)
160+
{
161+
return exception is MongoConnectionException mongoConnectionException && mongoConnectionException.IsNetworkException;
162+
}
163+
164+
private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception, SemanticVersion serverVersion)
156165
{
157-
if (__retryableWriteExceptions.Contains(exception.GetType()) || IsNetworkException(exception))
166+
if (!Feature.RetryableWrites.IsSupported(serverVersion))
167+
{
168+
return false;
169+
}
170+
171+
if (IsNetworkException(exception))
172+
{
173+
return true;
174+
}
175+
176+
if (Feature.ServerReturnsRetryableWriteErrorLabel.IsSupported(serverVersion))
177+
{
178+
return false;
179+
}
180+
181+
if (__retryableWriteExceptions.Contains(exception.GetType()))
158182
{
159183
return true;
160184
}

‎src/MongoDB.Driver.Core/Core/WireProtocol/CommandUsingCommandMessageWireProtocol.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,9 @@ private void AddErrorLabelIfRequired(Exception exception, SemanticVersion server
179179
mongoException.AddErrorLabel("TransientTransactionError");
180180
}
181181

182-
if ((exception is MongoConnectionException) || // network error
183-
(Feature.RetryableWrites.IsSupported(serverVersion) &&
184-
!Feature.ServerReturnsRetryableWriteErrorLabel.IsSupported(serverVersion)))
182+
if (RetryabilityHelper.IsCommandRetryable(_command))
185183
{
186-
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException);
184+
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException, serverVersion);
187185
}
188186
}
189187
}

‎tests/MongoDB.Driver.Core.Tests/Core/Operations/RetryabilityHelperTests.cs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,21 @@ public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError
4545
{
4646
var exception = CoreExceptionHelper.CreateMongoWriteConcernException(BsonDocument.Parse($"{{ writeConcernError : {{ code : {errorCode} }} }}"));
4747

48-
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception);
48+
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedVersion);
4949

5050
var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
5151
hasRetryableWriteErrorLabel.Should().Be(shouldAddErrorLabel);
5252
}
5353

54-
[Fact]
55-
public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError_for_network_errors()
54+
[Theory]
55+
[ParameterAttributeData]
56+
public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError_for_network_errors([Values(false, true)] bool serverReturnsRetryableWriteErrorLabel)
5657
{
5758
var exception = (MongoException)CoreExceptionHelper.CreateException(typeof(MongoConnectionException));
59+
var feature = Feature.ServerReturnsRetryableWriteErrorLabel;
60+
var serverVersion = serverReturnsRetryableWriteErrorLabel ? feature.FirstSupportedVersion : feature.LastNotSupportedVersion;
5861

59-
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception);
62+
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, serverVersion);
6063

6164
var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
6265
hasRetryableWriteErrorLabel.Should().BeTrue();
@@ -86,12 +89,47 @@ public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError
8689
exception = CoreExceptionHelper.CreateMongoCommandException((int)exceptionDescription);
8790
}
8891

89-
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception);
92+
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedVersion);
9093

9194
var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
9295
hasRetryableWriteErrorLabel.Should().Be(shouldAddErrorLabel);
9396
}
9497

98+
[Theory]
99+
[ParameterAttributeData]
100+
public void AddRetryableWriteErrorLabelIfRequired_should_not_add_error_label_for_non_retryWrites_server(
101+
[Values(false, true)] bool isNetworkError)
102+
{
103+
MongoException exception = null;
104+
if (isNetworkError)
105+
{
106+
exception = (MongoException)CoreExceptionHelper.CreateException(typeof(MongoConnectionException));
107+
}
108+
else
109+
{
110+
exception = CoreExceptionHelper.CreateMongoCommandException((int)ServerErrorCode.HostNotFound);
111+
}
112+
113+
RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, Feature.RetryableWrites.LastNotSupportedVersion);
114+
115+
var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
116+
hasRetryableWriteErrorLabel.Should().BeFalse();
117+
}
118+
119+
[Theory]
120+
[InlineData("{ txnNumber : 1 }", true)]
121+
[InlineData("{ commitTransaction : 1 }", true)]
122+
[InlineData("{ abortTransaction : 1 }", true)]
123+
[InlineData("{ ping : 1 }", false)]
124+
public void IsCommandRetryable_should_return_expected_result(string command, bool isRetryable)
125+
{
126+
var commandDocument = BsonDocument.Parse(command);
127+
128+
var result = RetryabilityHelper.IsCommandRetryable(commandDocument);
129+
130+
result.Should().Be(isRetryable);
131+
}
132+
95133
[Theory]
96134
[InlineData(typeof(MongoConnectionException), true)]
97135
[InlineData(typeof(MongoConnectionClosedException), false)]

‎tests/MongoDB.Driver.Tests/RetryableWritesTests.cs

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
using System;
1717
using FluentAssertions;
1818
using MongoDB.Bson;
19-
using MongoDB.Bson.TestHelpers.XunitExtensions;
2019
using MongoDB.Driver.Core;
20+
using MongoDB.Driver.Core.Bindings;
2121
using MongoDB.Driver.Core.Clusters;
2222
using MongoDB.Driver.Core.Configuration;
2323
using MongoDB.Driver.Core.Events;
24+
using MongoDB.Driver.Core.Misc;
25+
using MongoDB.Driver.Core.TestHelpers;
2426
using MongoDB.Driver.Core.TestHelpers.XunitExtensions;
2527
using MongoDB.Driver.TestHelpers;
2628
using Xunit;
@@ -43,6 +45,48 @@ public void Insert_with_RetryWrites_true_should_work_whether_retryable_writes_ar
4345
}
4446
}
4547

48+
[SkippableFact]
49+
public void Retryable_write_errorlabel_should_not_be_added_with_retryWrites_false()
50+
{
51+
if (CoreTestConfiguration.Cluster.Description.Type == ClusterType.Sharded)
52+
{
53+
RequireServer
54+
.Check()
55+
.Supports(Feature.RetryableWrites, Feature.FailPointsFailCommandForSharded)
56+
.ClusterTypes(ClusterType.Sharded);
57+
}
58+
else
59+
{
60+
RequireServer
61+
.Check()
62+
.Supports(Feature.RetryableWrites, Feature.FailPointsFailCommand)
63+
.ClusterTypes(ClusterType.ReplicaSet);
64+
}
65+
66+
var failPointWithRetryableError = @"
67+
{
68+
configureFailPoint : 'failCommand',
69+
mode : { times : 1 },
70+
data : {
71+
failCommands : ['insert'],
72+
errorCode : 262
73+
}
74+
}"; // no error label
75+
76+
using (var client = GetClient(retryWrites: false))
77+
using (ConfigureFailPoint(failPointWithRetryableError))
78+
{
79+
var database = client.GetDatabase(DriverTestConfiguration.DatabaseNamespace.DatabaseName);
80+
var collection = database.GetCollection<BsonDocument>(DriverTestConfiguration.CollectionNamespace.CollectionName);
81+
var document = new BsonDocument("x", 1);
82+
83+
var exception = Record.Exception(() => collection.InsertOne(document));
84+
85+
var e = exception.Should().BeOfType<MongoCommandException>().Subject;
86+
e.HasErrorLabel("RetryableWriteError").Should().BeFalse();
87+
}
88+
}
89+
4690
[SkippableFact]
4791
public void Retryable_write_operation_should_throw_custom_exception_on_servers_using_mmapv1()
4892
{
@@ -196,17 +240,18 @@ public void TxnNumber_should_be_included_with_UpdateOne()
196240
}
197241
}
198242

199-
private DisposableMongoClient GetClient()
243+
// private methods
244+
private DisposableMongoClient GetClient(bool retryWrites = true)
200245
{
201-
return GetClient(cb => { });
246+
return GetClient(cb => { }, retryWrites);
202247
}
203248

204-
private DisposableMongoClient GetClient(Action<ClusterBuilder> clusterConfigurator)
249+
private DisposableMongoClient GetClient(Action<ClusterBuilder> clusterConfigurator, bool retryWrites = true)
205250
{
206251
return DriverTestConfiguration.CreateDisposableClient((MongoClientSettings clientSettings) =>
207252
{
208253
clientSettings.ClusterConfigurator = clusterConfigurator;
209-
clientSettings.RetryWrites = true;
254+
clientSettings.RetryWrites = retryWrites;
210255
});
211256
}
212257

@@ -215,6 +260,13 @@ private DisposableMongoClient GetClient(EventCapturer capturer)
215260
return GetClient(cb => cb.Subscribe(capturer));
216261
}
217262

263+
private FailPoint ConfigureFailPoint(string failpointCommand)
264+
{
265+
var cluster = DriverTestConfiguration.Client.Cluster;
266+
var session = NoCoreSession.NewHandle();
267+
return FailPoint.Configure(cluster, session, BsonDocument.Parse(failpointCommand));
268+
}
269+
218270
private void RequireSupportForRetryableWrites()
219271
{
220272
RequireServer.Check()

‎tests/MongoDB.Driver.Tests/Specifications/retryable-writes/tests/README.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ Each YAML file has the following keys:
205205
result object if their BulkWriteException (or equivalent) provides access
206206
to a write result object.
207207

208+
- ``errorLabelsContain``: A list of error label strings that the
209+
error is expected to have.
210+
211+
- ``errorLabelsOmit``: A list of error label strings that the
212+
error is expected not to have.
213+
208214
- ``collection``:
209215

210216
- ``name`` (optional): The name of the collection to verify. If this isn't
@@ -311,10 +317,16 @@ and sharded clusters.
311317
retryWrites=false to your connection string.
312318

313319
and the error code is 20.
320+
321+
**Note**: Drivers that rely on ``serverStatus`` to determine the storage engine
322+
in use MAY skip this test for sharded clusters, since ``mongos`` does not report
323+
this information in its ``serverStatus`` response.
314324

315325
Changelog
316326
=========
317327

328+
:2019-10-21: Add ``errorLabelsContain`` and ``errorLabelsContain`` fields to ``result``
329+
318330
:2019-08-07: Add Prose Tests section
319331

320332
:2019-06-07: Mention $merge stage for aggregate alongside $out

‎tests/MongoDB.Driver.Tests/Specifications/retryable-writes/tests/bulkWrite-serverErrors.json

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@
3535
"failCommands": [
3636
"update"
3737
],
38-
"errorCode": 189
38+
"errorCode": 189,
39+
"errorLabels": [
40+
"RetryableWriteError"
41+
]
3942
}
4043
},
4144
"operation": {
@@ -117,7 +120,10 @@
117120
],
118121
"writeConcernError": {
119122
"code": 91,
120-
"errmsg": "Replication is being shut down"
123+
"errmsg": "Replication is being shut down",
124+
"errorLabels": [
125+
"RetryableWriteError"
126+
]
121127
}
122128
}
123129
},
@@ -186,6 +192,81 @@
186192
]
187193
}
188194
}
195+
},
196+
{
197+
"description": "BulkWrite fails with a RetryableWriteError label after two connection failures",
198+
"failPoint": {
199+
"configureFailPoint": "failCommand",
200+
"mode": {
201+
"times": 2
202+
},
203+
"data": {
204+
"failCommands": [
205+
"update"
206+
],
207+
"closeConnection": true
208+
}
209+
},
210+
"operation": {
211+
"name": "bulkWrite",
212+
"arguments": {
213+
"requests": [
214+
{
215+
"name": "deleteOne",
216+
"arguments": {
217+
"filter": {
218+
"_id": 1
219+
}
220+
}
221+
},
222+
{
223+
"name": "insertOne",
224+
"arguments": {
225+
"document": {
226+
"_id": 3,
227+
"x": 33
228+
}
229+
}
230+
},
231+
{
232+
"name": "updateOne",
233+
"arguments": {
234+
"filter": {
235+
"_id": 2
236+
},
237+
"update": {
238+
"$inc": {
239+
"x": 1
240+
}
241+
}
242+
}
243+
}
244+
],
245+
"options": {
246+
"ordered": true
247+
}
248+
}
249+
},
250+
"outcome": {
251+
"error": true,
252+
"result": {
253+
"errorLabelsContain": [
254+
"RetryableWriteError"
255+
]
256+
},
257+
"collection": {
258+
"data": [
259+
{
260+
"_id": 2,
261+
"x": 22
262+
},
263+
{
264+
"_id": 3,
265+
"x": 33
266+
}
267+
]
268+
}
269+
}
189270
}
190271
]
191272
}

0 commit comments

Comments
 (0)