Skip to content

Commit 2d9b4d4

Browse files
author
Brian Chen
committed
standardize transaction retries to attempts
1 parent 011b65b commit 2d9b4d4

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/TransactionTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.android.gms.tasks.TaskCompletionSource;
3030
import com.google.android.gms.tasks.Tasks;
3131
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
32+
import com.google.firebase.firestore.core.TransactionRunner;
3233
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
3334
import com.google.firebase.firestore.util.AsyncQueue.TimerId;
3435
import java.util.ArrayList;
@@ -632,6 +633,26 @@ public void testDoesNotRetryOnPermanentError() {
632633
assertEquals(1, count.get());
633634
}
634635

636+
@Test
637+
public void testMakesDefaultMaxAttempts() {
638+
FirebaseFirestore firestore = testFirestore();
639+
DocumentReference doc1 = firestore.collection("counters").document();
640+
AtomicInteger count = new AtomicInteger(0);
641+
waitFor(doc1.set(map("count", 15)));
642+
Task<Void> transactionTask = firestore.runTransaction(
643+
transaction -> {
644+
// Get the first doc.
645+
transaction.get(doc1);
646+
// Do a write outside of the transaction to cause the transaction to fail.
647+
waitFor(doc1.set(map("count", 1234 + count.incrementAndGet())));
648+
return null;
649+
});
650+
651+
Exception e = waitForException(transactionTask);
652+
assertEquals(Code.FAILED_PRECONDITION, ((FirebaseFirestoreException) e).getCode());
653+
assertEquals(TransactionRunner.DEFAULT_MAX_ATTEMPTS_COUNT, count.get());
654+
}
655+
635656
@Test
636657
public void testSuccessWithNoTransactionOperations() {
637658
FirebaseFirestore firestore = testFirestore();

firebase-firestore/src/main/java/com/google/firebase/firestore/core/TransactionRunner.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727

2828
/** TransactionRunner encapsulates the logic needed to run and retry transactions with backoff. */
2929
public class TransactionRunner<TResult> {
30-
private static final int RETRY_COUNT = 5;
30+
public static final int DEFAULT_MAX_ATTEMPTS_COUNT = 5;
3131
private AsyncQueue asyncQueue;
3232
private RemoteStore remoteStore;
3333
private Function<Transaction, Task<TResult>> updateFunction;
34-
private int retriesLeft;
34+
private int attemptsRemaining;
3535

3636
private ExponentialBackoff backoff;
3737
private TaskCompletionSource<TResult> taskSource = new TaskCompletionSource<>();
@@ -44,7 +44,7 @@ public TransactionRunner(
4444
this.asyncQueue = asyncQueue;
4545
this.remoteStore = remoteStore;
4646
this.updateFunction = updateFunction;
47-
this.retriesLeft = RETRY_COUNT;
47+
this.attemptsRemaining = DEFAULT_MAX_ATTEMPTS_COUNT;
4848

4949
backoff = new ExponentialBackoff(asyncQueue, TimerId.RETRY_TRANSACTION);
5050
}
@@ -56,6 +56,7 @@ public Task<TResult> run() {
5656
}
5757

5858
private void runWithBackoff() {
59+
attemptsRemaining -= 1;
5960
backoff.backoffAndRun(
6061
() -> {
6162
final Transaction transaction = remoteStore.createTransaction();
@@ -84,8 +85,7 @@ private void runWithBackoff() {
8485
}
8586

8687
private void handleTransactionError(Task task) {
87-
if (retriesLeft > 0 && isRetryableTransactionError(task.getException())) {
88-
retriesLeft -= 1;
88+
if (attemptsRemaining > 0 && isRetryableTransactionError(task.getException())) {
8989
runWithBackoff();
9090
} else {
9191
taskSource.setException(task.getException());

0 commit comments

Comments
 (0)