Skip to content

Commit 61000da

Browse files
rstamrstam
committed
CSHARP-2960: Unacknowledged writes fail silently when retryWrites=true.
1 parent f8d38a8 commit 61000da

File tree

9 files changed

+806
-27
lines changed

9 files changed

+806
-27
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ public interface IRetryableReadOperation<TResult> : IExecutableInRetryableReadCo
9797
/// <typeparam name="TResult">The type of the result.</typeparam>
9898
public interface IRetryableWriteOperation<TResult> : IExecutableInRetryableWriteContext<TResult>
9999
{
100+
/// <summary>
101+
/// Gets the write concern for the operation.
102+
/// </summary>
103+
WriteConcern WriteConcern { get; }
104+
100105
/// <summary>
101106
/// Executes the first attempt.
102107
/// </summary>

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static TResult Execute<TResult>(IRetryableWriteOperation<TResult> operati
3535

3636
public static TResult Execute<TResult>(IRetryableWriteOperation<TResult> operation, RetryableWriteContext context, CancellationToken cancellationToken)
3737
{
38-
if (!context.RetryRequested || !AreRetryableWritesSupported(context.Channel.ConnectionDescription) || context.Binding.Session.IsInTransaction)
38+
if (!AreRetriesAllowed(operation, context))
3939
{
4040
return operation.ExecuteAttempt(context, 1, null, cancellationToken);
4141
}
@@ -86,7 +86,7 @@ public async static Task<TResult> ExecuteAsync<TResult>(IRetryableWriteOperation
8686

8787
public static async Task<TResult> ExecuteAsync<TResult>(IRetryableWriteOperation<TResult> operation, RetryableWriteContext context, CancellationToken cancellationToken)
8888
{
89-
if (!context.RetryRequested || !AreRetryableWritesSupported(context.Channel.ConnectionDescription) || context.Binding.Session.IsInTransaction)
89+
if (!AreRetriesAllowed(operation, context))
9090
{
9191
return await operation.ExecuteAttemptAsync(context, 1, null, cancellationToken).ConfigureAwait(false);
9292
}
@@ -128,13 +128,34 @@ public static async Task<TResult> ExecuteAsync<TResult>(IRetryableWriteOperation
128128
}
129129

130130
// privates static methods
131+
private static bool AreRetriesAllowed<TResult>(IRetryableWriteOperation<TResult> operation, RetryableWriteContext context)
132+
{
133+
return IsOperationAcknowledged(operation) && DoesContextAllowRetries(context);
134+
}
135+
131136
private static bool AreRetryableWritesSupported(ConnectionDescription connectionDescription)
132137
{
133138
return
134139
connectionDescription.IsMasterResult.LogicalSessionTimeout != null &&
135140
connectionDescription.IsMasterResult.ServerType != ServerType.Standalone;
136141
}
137142

143+
private static bool DoesContextAllowRetries(RetryableWriteContext context)
144+
{
145+
return
146+
context.RetryRequested &&
147+
AreRetryableWritesSupported(context.Channel.ConnectionDescription) &&
148+
!context.Binding.Session.IsInTransaction;
149+
}
150+
151+
private static bool IsOperationAcknowledged<TResult>(IRetryableWriteOperation<TResult> operation)
152+
{
153+
var writeConcern = operation.WriteConcern;
154+
return
155+
writeConcern == null || // null means use server default write concern which implies acknowledged
156+
writeConcern.IsAcknowledged;
157+
}
158+
138159
private static bool ShouldThrowOriginalException(Exception retryException)
139160
{
140161
return retryException is MongoException && !(retryException is MongoConnectionException);

‎tests/MongoDB.Bson.TestHelpers/BsonDocumentAssertions.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ public AndConstraint<BsonDocumentAssertions> Be(string json, string because = ""
5959
return Be(expected, because, reasonArgs);
6060
}
6161

62+
public AndConstraint<BsonDocumentAssertions> Contain(string name, string because = "", params object[] reasonArgs)
63+
{
64+
Execute.Assertion
65+
.BecauseOf(because, reasonArgs)
66+
.ForCondition(Subject.Contains(name))
67+
.FailWith("Expected {context:object} to contain element {0}{reason}.", name);
68+
69+
return new AndConstraint<BsonDocumentAssertions>(this);
70+
}
71+
6272
public AndConstraint<BsonDocumentAssertions> NotBe(BsonDocument unexpected, string because = "", params object[] reasonArgs)
6373
{
6474
Execute.Assertion
@@ -75,6 +85,16 @@ public AndConstraint<BsonDocumentAssertions> NotBe(string json, string because =
7585
return NotBe(expected, because, reasonArgs);
7686
}
7787

88+
public AndConstraint<BsonDocumentAssertions> NotContain(string name, string because = "", params object[] reasonArgs)
89+
{
90+
Execute.Assertion
91+
.BecauseOf(because, reasonArgs)
92+
.ForCondition(!Subject.Contains(name))
93+
.FailWith("Expected {context:object} to not contain element {0}{reason}.", name);
94+
95+
return new AndConstraint<BsonDocumentAssertions>(this);
96+
}
97+
7898
protected override string Context
7999
{
80100
get { return "BsonDocument"; }

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,8 @@ public void Execute_with_an_error_in_the_second_batch_and_ordered_is_true(
10951095
[SkippableTheory]
10961096
[ParameterAttributeData]
10971097
public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_is_false(
1098+
[Values(false, true)]
1099+
bool retryRequested,
10981100
[Values(false, true)]
10991101
bool async)
11001102
{
@@ -1112,6 +1114,7 @@ public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_
11121114
var subject = new BulkMixedWriteOperation(_collectionNamespace, requests, _messageEncoderSettings)
11131115
{
11141116
IsOrdered = false,
1117+
RetryRequested = retryRequested,
11151118
WriteConcern = WriteConcern.Unacknowledged
11161119
};
11171120

@@ -1133,6 +1136,8 @@ public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_
11331136
[SkippableTheory]
11341137
[ParameterAttributeData]
11351138
public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_is_true(
1139+
[Values(false, true)]
1140+
bool retryRequested,
11361141
[Values(false, true)]
11371142
bool async)
11381143
{
@@ -1156,6 +1161,7 @@ public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_
11561161
var subject = new BulkMixedWriteOperation(_collectionNamespace, requests, _messageEncoderSettings)
11571162
{
11581163
IsOrdered = true,
1164+
RetryRequested = retryRequested,
11591165
WriteConcern = WriteConcern.Unacknowledged
11601166
};
11611167

@@ -1176,6 +1182,8 @@ public void Execute_unacknowledged_with_an_error_in_the_first_batch_and_ordered_
11761182
[SkippableTheory]
11771183
[ParameterAttributeData]
11781184
public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered_is_false(
1185+
[Values(false, true)]
1186+
bool retryRequested,
11791187
[Values(false, true)]
11801188
bool async)
11811189
{
@@ -1194,6 +1202,7 @@ public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered
11941202
{
11951203
IsOrdered = false,
11961204
MaxBatchCount = 2,
1205+
RetryRequested = retryRequested,
11971206
WriteConcern = WriteConcern.Unacknowledged
11981207
};
11991208

@@ -1214,6 +1223,8 @@ public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered
12141223
[SkippableTheory]
12151224
[ParameterAttributeData]
12161225
public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered_is_true(
1226+
[Values(false, true)]
1227+
bool retryRequested,
12171228
[Values(false, true)]
12181229
bool async)
12191230
{
@@ -1232,6 +1243,7 @@ public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered
12321243
{
12331244
IsOrdered = true,
12341245
MaxBatchCount = 2,
1246+
RetryRequested = retryRequested,
12351247
WriteConcern = WriteConcern.Unacknowledged
12361248
};
12371249

@@ -1252,6 +1264,7 @@ public void Execute_unacknowledged_with_an_error_in_the_second_batch_and_ordered
12521264
[SkippableTheory]
12531265
[ParameterAttributeData]
12541266
public void Execute_with_delete_should_not_send_session_id_when_unacknowledged_writes(
1267+
[Values(false, true)] bool retryRequested,
12551268
[Values(false, true)] bool useImplicitSession,
12561269
[Values(false, true)] bool async)
12571270
{
@@ -1262,6 +1275,7 @@ public void Execute_with_delete_should_not_send_session_id_when_unacknowledged_w
12621275
var requests = new[] { new DeleteRequest(BsonDocument.Parse("{ x : 1 }")) };
12631276
var subject = new BulkMixedWriteOperation(collectionNamespace, requests, _messageEncoderSettings)
12641277
{
1278+
RetryRequested = retryRequested,
12651279
WriteConcern = WriteConcern.Unacknowledged
12661280
};
12671281

@@ -1284,6 +1298,7 @@ public void Execute_with_delete_should_send_session_id_when_supported(
12841298
[SkippableTheory]
12851299
[ParameterAttributeData]
12861300
public void Execute_with_insert_should_not_send_session_id_when_unacknowledged_writes(
1301+
[Values(false, true)] bool retryRequested,
12871302
[Values(false, true)] bool useImplicitSession,
12881303
[Values(false, true)] bool async)
12891304
{
@@ -1294,6 +1309,7 @@ public void Execute_with_insert_should_not_send_session_id_when_unacknowledged_w
12941309
var requests = new[] { new InsertRequest(BsonDocument.Parse("{ _id : 1, x : 3 }")) };
12951310
var subject = new BulkMixedWriteOperation(collectionNamespace, requests, _messageEncoderSettings)
12961311
{
1312+
RetryRequested = retryRequested,
12971313
WriteConcern = WriteConcern.Unacknowledged
12981314
};
12991315

@@ -1316,6 +1332,7 @@ public void Execute_with_insert_should_send_session_id_when_supported(
13161332
[SkippableTheory]
13171333
[ParameterAttributeData]
13181334
public void Execute_with_update_should_not_send_session_id_when_unacknowledged_writes(
1335+
[Values(false, true)] bool retryRequested,
13191336
[Values(false, true)] bool useImplicitSession,
13201337
[Values(false, true)] bool async)
13211338
{
@@ -1326,6 +1343,7 @@ public void Execute_with_update_should_not_send_session_id_when_unacknowledged_w
13261343
var requests = new[] { new UpdateRequest(UpdateType.Update, BsonDocument.Parse("{ x : 1 }"), BsonDocument.Parse("{ $set : { a : 1 } }")) };
13271344
var subject = new BulkMixedWriteOperation(collectionNamespace, requests, _messageEncoderSettings)
13281345
{
1346+
RetryRequested = retryRequested,
13291347
WriteConcern = WriteConcern.Unacknowledged
13301348
};
13311349

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

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/* Copyright 2020-present MongoDB Inc.
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS,
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
using System.Linq;
17+
using System.Net;
18+
using System.Reflection;
19+
using System.Threading;
20+
using FluentAssertions;
21+
using MongoDB.Bson;
22+
using MongoDB.Bson.TestHelpers;
23+
using MongoDB.Driver.Core.Bindings;
24+
using MongoDB.Driver.Core.Clusters;
25+
using MongoDB.Driver.Core.Connections;
26+
using MongoDB.Driver.Core.Operations;
27+
using MongoDB.Driver.Core.Servers;
28+
using Moq;
29+
using Xunit;
30+
31+
namespace MongoDB.Driver.Core.Tests.Core.Operations
32+
{
33+
public class RetryableWriteOperationExecutorTests
34+
{
35+
[Theory]
36+
[InlineData(false, false, false, false)]
37+
[InlineData(false, false, true, false)]
38+
[InlineData(false, true, false, false)]
39+
[InlineData(false, true, true, false)]
40+
[InlineData(true, false, false, false)]
41+
[InlineData(true, false, true, false)]
42+
[InlineData(true, true, false, true)]
43+
[InlineData(true, true, true, false)]
44+
public void DoesContextAllowRetries_should_return_expected_result(
45+
bool retryRequested,
46+
bool areRetryableWritesSupported,
47+
bool isInTransaction,
48+
bool expectedResult)
49+
{
50+
var context = CreateContext(retryRequested, areRetryableWritesSupported, isInTransaction);
51+
52+
var result = RetryableWriteOperationExecutorReflector.DoesContextAllowRetries(context);
53+
54+
result.Should().Be(expectedResult);
55+
}
56+
57+
[Theory]
58+
[InlineData(false, false, true)]
59+
[InlineData(false, true, true)]
60+
[InlineData(true, false, false)]
61+
[InlineData(true, true, true)]
62+
public void IsOperationAcknowledged_should_return_expected_result(
63+
bool withWriteConcern,
64+
bool isAcknowledged,
65+
bool expectedResult)
66+
{
67+
var operation = CreateOperation(withWriteConcern, isAcknowledged);
68+
69+
var result = RetryableWriteOperationExecutorReflector.IsOperationAcknowledged(operation);
70+
71+
result.Should().Be(expectedResult);
72+
}
73+
74+
// private methods
75+
private IWriteBinding CreateBinding(bool areRetryableWritesSupported, bool isInTransaction)
76+
{
77+
var mockBinding = new Mock<IWriteBinding>();
78+
var session = CreateSession(isInTransaction);
79+
var channelSource = CreateChannelSource(areRetryableWritesSupported);
80+
mockBinding.SetupGet(m => m.Session).Returns(session);
81+
mockBinding.Setup(m => m.GetWriteChannelSource(CancellationToken.None)).Returns(channelSource);
82+
return mockBinding.Object;
83+
}
84+
85+
private IChannelHandle CreateChannel(bool areRetryableWritesSupported)
86+
{
87+
var mockChannel = new Mock<IChannelHandle>();
88+
var connectionDescription = CreateConnectionDescription(areRetryableWritesSupported);
89+
mockChannel.SetupGet(m => m.ConnectionDescription).Returns(connectionDescription);
90+
return mockChannel.Object;
91+
}
92+
93+
private IChannelSourceHandle CreateChannelSource(bool areRetryableWritesSupported)
94+
{
95+
var mockChannelSource = new Mock<IChannelSourceHandle>();
96+
var channel = CreateChannel(areRetryableWritesSupported);
97+
mockChannelSource.Setup(m => m.GetChannel(CancellationToken.None)).Returns(channel);
98+
return mockChannelSource.Object;
99+
}
100+
101+
private ConnectionDescription CreateConnectionDescription(bool areRetryableWritesSupported)
102+
{
103+
var clusterId = new ClusterId(1);
104+
var endPoint = new DnsEndPoint("localhost", 27017);
105+
var serverId = new ServerId(clusterId, endPoint);
106+
var connectionId = new ConnectionId(serverId, 1);
107+
var isMasterResultDocument = BsonDocument.Parse("{ ok : 1 }");
108+
if (areRetryableWritesSupported)
109+
{
110+
isMasterResultDocument["logicalSessionTimeoutMinutes"] = 1;
111+
isMasterResultDocument["msg"] = "isdbgrid"; // mongos
112+
}
113+
var isMasterResult = new IsMasterResult(isMasterResultDocument);
114+
var buildInfoResult = new BuildInfoResult(BsonDocument.Parse("{ ok : 1, version : '4.2.0' }"));
115+
var connectionDescription = new ConnectionDescription(connectionId, isMasterResult, buildInfoResult);
116+
return connectionDescription;
117+
}
118+
119+
private RetryableWriteContext CreateContext(bool retryRequested, bool areRetryableWritesSupported, bool isInTransaction)
120+
{
121+
var binding = CreateBinding(areRetryableWritesSupported, isInTransaction);
122+
return RetryableWriteContext.Create(binding, retryRequested, CancellationToken.None);
123+
}
124+
125+
private IRetryableWriteOperation<BsonDocument> CreateOperation(bool withWriteConcern, bool isAcknowledged)
126+
{
127+
var mockOperation = new Mock<IRetryableWriteOperation<BsonDocument>>();
128+
var writeConcern = withWriteConcern ? (isAcknowledged ? WriteConcern.Acknowledged : WriteConcern.Unacknowledged) : null;
129+
mockOperation.SetupGet(m => m.WriteConcern).Returns(writeConcern);
130+
return mockOperation.Object;
131+
}
132+
133+
private ICoreSessionHandle CreateSession(bool isInTransaction)
134+
{
135+
var mockSession = new Mock<ICoreSessionHandle>();
136+
mockSession.SetupGet(m => m.IsInTransaction).Returns(isInTransaction);
137+
return mockSession.Object;
138+
}
139+
}
140+
141+
// nested types
142+
public static class RetryableWriteOperationExecutorReflector
143+
{
144+
public static bool DoesContextAllowRetries(RetryableWriteContext context) =>
145+
(bool)Reflector.InvokeStatic(typeof(RetryableWriteOperationExecutor), nameof(DoesContextAllowRetries), context);
146+
147+
public static bool IsOperationAcknowledged(IRetryableWriteOperation<BsonDocument> operation)
148+
{
149+
var methodInfoDefinition = typeof(RetryableWriteOperationExecutor).GetMethods(BindingFlags.NonPublic | BindingFlags.Static)
150+
.Where(m => m.Name == nameof(IsOperationAcknowledged))
151+
.Single();
152+
var methodInfo = methodInfoDefinition.MakeGenericMethod(typeof(BsonDocument));
153+
try
154+
{
155+
return (bool)methodInfo.Invoke(null, new object[] { operation });
156+
}
157+
catch (TargetInvocationException exception)
158+
{
159+
throw exception.InnerException;
160+
}
161+
}
162+
}
163+
}

0 commit comments

Comments
 (0)