Skip to content

Commit 4b1acf8

Browse files
jskeetjskeet
committed
feat: Add configurable retry timing for RunTransactionAsync
There are further changes to come in terms of transaction retry, but this is at least a start. Note that this changes the default backoff from "none at all" to "100ms initial, with a multiplier of 1.3". That's a more reasonable default, and it seems unlikely that customers would actually *depend* on there being no backoff. Fixes #10513
1 parent 1aa1ab8 commit 4b1acf8

4 files changed

Lines changed: 94 additions & 19 deletions

File tree

‎apis/Google.Cloud.Firestore/Google.Cloud.Firestore.Tests/FirestoreDbTest.RunTransactionAsync.cs‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017, Google Inc. All rights reserved.
1+
// Copyright 2017, Google Inc. All rights reserved.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -12,11 +12,16 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using Google.Api.Gax;
16+
using Google.Api.Gax.Grpc;
17+
using Google.Api.Gax.Testing;
1518
using Google.Cloud.Firestore.V1;
1619
using Google.Protobuf;
1720
using Grpc.Core;
1821
using System;
22+
using System.Collections.Generic;
1923
using System.IO;
24+
using System.Linq;
2025
using System.Threading.Tasks;
2126
using Xunit;
2227
using static Google.Cloud.Firestore.Tests.ProtoHelpers;
@@ -138,6 +143,35 @@ public async Task RunTransactionAsync_TooManyRetries(int? userSpecifiedAttempts)
138143
Assert.Equal(actualAttempts, client.RollbackRequests.Count);
139144
}
140145

146+
[Fact]
147+
public async Task RunTransactionAsync_CustomRetrySettings()
148+
{
149+
var start = new DateTime(2000, 1, 1, 0, 0, 0, DateTimeKind.Utc);
150+
var clock = new FakeClock(start);
151+
var scheduler = new FakeScheduler(clock);
152+
153+
var settings = new FirestoreSettings
154+
{
155+
Scheduler = scheduler,
156+
Clock = scheduler.Clock
157+
};
158+
159+
// 6 failures, so 7 RPCs in total.
160+
var client = new TransactionTestingClient(6, CreateRpcException(StatusCode.Aborted), settings);
161+
var db = FirestoreDb.Create("proj", "db", client);
162+
163+
// Backoffs will be 1, 2, 4, 5, 5, 5.
164+
// Timestamps will be 0, 1, 3, 7, 12, 17, 22.
165+
var retrySettings = RetrySettings.FromExponentialBackoff(10, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(5), 2.0, ex => true, RetrySettings.NoJitter);
166+
var options = TransactionOptions.ForRetrySettings(retrySettings);
167+
168+
var timestamps = await scheduler.RunAsync(() => db.RunTransactionAsync(CreateTimestampingCallback(scheduler.Clock), options));
169+
170+
var timestampSecondsSinceStart = timestamps.Select(ts => (ts - start).TotalSeconds).ToArray();
171+
double[] expectedSecondsSinceStart = { 0.0, 1.0, 3.0, 7.0, 12.0, 17.0, 22.0 };
172+
Assert.Equal(expectedSecondsSinceStart, timestampSecondsSinceStart);
173+
}
174+
141175
/// <summary>
142176
/// Creates a request that creates projects/proj/databases/db/documents/col/doc1 and
143177
/// deletes projects/proj/databases/db/documents/col/doc2 - the operations performed in
@@ -184,6 +218,19 @@ private Func<Transaction, Task<int>> CreateCountingCallback()
184218
};
185219
}
186220

221+
private Func<Transaction, Task<List<DateTime>>> CreateTimestampingCallback(IClock clock)
222+
{
223+
List<DateTime> ret = new();
224+
return async transaction =>
225+
{
226+
var db = transaction.Database;
227+
ret.Add(clock.GetCurrentDateTimeUtc());
228+
await transaction.GetSnapshotAsync(db.Document("col/x"));
229+
transaction.Create(db.Document("col/doc1"), new { Name = "Test" });
230+
transaction.Delete(db.Document("col/doc2"));
231+
return ret;
232+
};
233+
}
187234

188235
private static RollbackRequest CreateRollbackRequest(string transactionId) =>
189236
new RollbackRequest

‎apis/Google.Cloud.Firestore/Google.Cloud.Firestore.Tests/TransactionTestingClient.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017, Google Inc. All rights reserved.
1+
// Copyright 2017, Google Inc. All rights reserved.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -57,9 +57,9 @@ public TransactionTestingClient() : this(0, null)
5757
/// <summary>
5858
/// Creates a client which will respond to the first <paramref name="failures"/> commit calls with <paramref name="exception"/>.
5959
/// </summary>
60-
public TransactionTestingClient(int failures, Exception exception)
60+
public TransactionTestingClient(int failures, Exception exception, FirestoreSettings settings = null)
6161
{
62-
Settings = FirestoreSettings.GetDefault();
62+
Settings = settings ?? FirestoreSettings.GetDefault();
6363
_failures = failures;
6464
_exception = exception;
6565
}

‎apis/Google.Cloud.Firestore/Google.Cloud.Firestore/FirestoreDb.cs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,9 +387,12 @@ public Task RunTransactionAsync(Func<Transaction, Task> callback, TransactionOpt
387387
public async Task<T> RunTransactionAsync<T>(Func<Transaction, Task<T>> callback, TransactionOptions options = null, CancellationToken cancellationToken = default)
388388
{
389389
ByteString previousTransactionId = null;
390-
options = options ?? TransactionOptions.Default;
391-
var attemptsLeft = options.MaxAttempts;
392-
TimeSpan backoff = TimeSpan.FromSeconds(1);
390+
options ??= TransactionOptions.Default;
391+
392+
var retrySettings = options.RetrySettings;
393+
var attemptsLeft = retrySettings.MaxAttempts;
394+
TimeSpan backoff = retrySettings.InitialBackoff;
395+
var scheduler = Client.Settings.Scheduler ?? SystemScheduler.Instance;
393396

394397
while (true)
395398
{
@@ -408,8 +411,12 @@ public async Task<T> RunTransactionAsync<T>(Func<Transaction, Task<T>> callback,
408411
}
409412
catch (RpcException e) when (CheckRetry(e, ref rollback))
410413
{
411-
// On to the next iteration...
414+
// On to the next iteration after a backoff.
412415
}
416+
417+
// This is essentially the inner loop of RetryAttempt.CreateRetrySequence.
418+
await scheduler.Delay(retrySettings.BackoffJitter.GetDelay(backoff), cancellationToken).ConfigureAwait(false);
419+
backoff = retrySettings.NextBackoff(backoff);
413420
}
414421
finally
415422
{

‎apis/Google.Cloud.Firestore/Google.Cloud.Firestore/TransactionOptions.cs‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017, Google Inc. All rights reserved.
1+
// Copyright 2017, Google Inc. All rights reserved.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414

1515
using Google.Api.Gax;
16+
using Google.Api.Gax.Grpc;
17+
using System;
1618

1719
namespace Google.Cloud.Firestore
1820
{
@@ -24,27 +26,46 @@ public sealed class TransactionOptions
2426
/// <summary>
2527
/// The transaction options that are used if nothing is specified by the caller.
2628
/// </summary>
27-
public static TransactionOptions Default { get; } = new TransactionOptions(5);
29+
public static TransactionOptions Default { get; } = ForMaxAttempts(5);
2830

2931
/// <summary>
3032
/// The number of times the transaction will be attempted before failing.
33+
/// This is equivalent to <see cref="RetrySettings.MaxAttempts"/>.
3134
/// </summary>
32-
public int MaxAttempts { get; }
35+
public int MaxAttempts => RetrySettings.MaxAttempts;
3336

34-
private TransactionOptions(int maxAttempts)
37+
/// <summary>
38+
/// The settings to control the timing of retries within the transaction.
39+
/// The <see cref="RetrySettings.RetryFilter"/> property is ignored. This property is never null.
40+
/// </summary>
41+
public RetrySettings RetrySettings { get; }
42+
43+
private TransactionOptions(RetrySettings retrySettings)
3544
{
36-
MaxAttempts = maxAttempts;
45+
RetrySettings = retrySettings;
3746
}
3847

3948
/// <summary>
40-
/// Creates an instance with the given maximum number of attempts.
49+
/// Creates an instance with the given maximum number of attempts. The default retry
50+
/// timing will be used.
4151
/// </summary>
4252
/// <param name="maxAttempts">The number of times a transaction will be attempted before failing. Must be positive.</param>
4353
/// <returns>A new options object.</returns>
44-
public static TransactionOptions ForMaxAttempts(int maxAttempts)
45-
{
46-
GaxPreconditions.CheckArgumentRange(maxAttempts, nameof(maxAttempts), 1, int.MaxValue);
47-
return new TransactionOptions(maxAttempts);
48-
}
54+
public static TransactionOptions ForMaxAttempts(int maxAttempts) =>
55+
new TransactionOptions(RetrySettings.FromExponentialBackoff(
56+
maxAttempts: maxAttempts,
57+
initialBackoff: TimeSpan.FromMilliseconds(100),
58+
maxBackoff: TimeSpan.FromMinutes(1),
59+
backoffMultiplier: 1.3,
60+
retryFilter: x => true)); // Ignored
61+
62+
/// <summary>
63+
/// Creates an instance with the given retry settings, including maximum number of attempts.
64+
/// The <see cref="RetrySettings.RetryFilter"/> property is not used.
65+
/// </summary>
66+
/// <param name="retrySettings">The retry settings to use. Must not be null.</param>
67+
/// <returns>A new options object.</returns>
68+
public static TransactionOptions ForRetrySettings(RetrySettings retrySettings) =>
69+
new TransactionOptions(GaxPreconditions.CheckNotNull(retrySettings, nameof(retrySettings)));
4970
}
5071
}

0 commit comments

Comments
 (0)