Skip to content

Commit abb8367

Browse files
author
Brian Chen
committed
working everything
1 parent e58f07c commit abb8367

File tree

6 files changed

+110
-37
lines changed

6 files changed

+110
-37
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ public void testHandleReadingOneDocAndWritingAnother() {
546546
@Test
547547
public void testReadingADocTwiceWithDifferentVersions() {
548548
FirebaseFirestore firestore = testFirestore();
549+
firestore.removeTransactionBackoffs();
549550
DocumentReference doc = firestore.collection("counters").document();
550551
waitFor(doc.set(map("count", 15.0)));
551552
AtomicInteger counter = new AtomicInteger(0);

‎firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ public interface InstanceRegistry {
7171
private FirebaseFirestoreSettings settings;
7272
private volatile FirestoreClient client;
7373

74+
private boolean skipTransactionBackoffs = false;
75+
7476
@NonNull
7577
public static FirebaseFirestore getInstance() {
7678
FirebaseApp app = FirebaseApp.getInstance();
@@ -279,7 +281,7 @@ private <ResultT> Task<ResultT> runTransaction(
279281
updateFunction.apply(
280282
new Transaction(internalTransaction, FirebaseFirestore.this)));
281283

282-
return client.transaction(wrappedUpdateFunction, 5);
284+
return client.transaction(wrappedUpdateFunction, 5, skipTransactionBackoffs);
283285
}
284286

285287
/**
@@ -385,6 +387,14 @@ AsyncQueue getAsyncQueue() {
385387
return asyncQueue;
386388
}
387389

390+
/**
391+
* For tests: Skip all transaction backoffs to make integration tests complete faster.
392+
*/
393+
@VisibleForTesting
394+
void removeTransactionBackoffs() {
395+
this.skipTransactionBackoffs = true;
396+
}
397+
388398
/**
389399
* Re-enables network usage for this instance after a prior call to {@link #disableNetwork()}.
390400
*

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@
5151
import com.google.firebase.firestore.remote.RemoteSerializer;
5252
import com.google.firebase.firestore.remote.RemoteStore;
5353
import com.google.firebase.firestore.util.AsyncQueue;
54+
import com.google.firebase.firestore.util.AsyncQueue.TimerId;
55+
import com.google.firebase.firestore.util.ExponentialBackoff;
5456
import com.google.firebase.firestore.util.Logger;
5557
import io.grpc.Status;
5658
import java.util.Collections;
@@ -217,11 +219,20 @@ public Task<Void> write(final List<Mutation> mutations) {
217219

218220
/** Tries to execute the transaction in updateFunction up to retries times. */
219221
public <TResult> Task<TResult> transaction(
220-
Function<Transaction, Task<TResult>> updateFunction, int retries) {
222+
Function<Transaction, Task<TResult>> updateFunction,
223+
int retries,
224+
boolean skippedTransactionBackoffs) {
221225
this.verifyNotShutdown();
222-
return AsyncQueue.callTask(
223-
asyncQueue.getExecutor(),
224-
() -> syncEngine.transaction(asyncQueue, updateFunction, retries));
226+
ExponentialBackoff backoff;
227+
if (skippedTransactionBackoffs) {
228+
backoff = new ExponentialBackoff(asyncQueue, TimerId.RETRY_TRANSACTION, 1, 2, 10);
229+
} else {
230+
backoff = new ExponentialBackoff(asyncQueue, AsyncQueue.TimerId.RETRY_TRANSACTION);
231+
}
232+
final TaskCompletionSource<TResult> txTaskSource = new TaskCompletionSource<>();
233+
asyncQueue.enqueueAndForget(
234+
() -> syncEngine.transaction(asyncQueue, backoff, txTaskSource, updateFunction, retries));
235+
return txTaskSource.getTask();
225236
}
226237

227238
/**

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

Lines changed: 57 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
import static com.google.firebase.firestore.util.Assert.fail;
1818
import static com.google.firebase.firestore.util.Assert.hardAssert;
1919

20+
import androidx.annotation.NonNull;
2021
import androidx.annotation.Nullable;
2122
import androidx.annotation.VisibleForTesting;
23+
import com.google.android.gms.tasks.OnCompleteListener;
2224
import com.google.android.gms.tasks.Task;
2325
import com.google.android.gms.tasks.TaskCompletionSource;
2426
import com.google.android.gms.tasks.Tasks;
@@ -47,6 +49,7 @@
4749
import com.google.firebase.firestore.remote.RemoteStore;
4850
import com.google.firebase.firestore.remote.TargetChange;
4951
import com.google.firebase.firestore.util.AsyncQueue;
52+
import com.google.firebase.firestore.util.ExponentialBackoff;
5053
import com.google.firebase.firestore.util.Logger;
5154
import com.google.firebase.firestore.util.Util;
5255
import io.grpc.Status;
@@ -250,9 +253,10 @@ private void addUserCallback(int batchId, TaskCompletionSource<Void> userTask) {
250253
/**
251254
* Takes an updateFunction in which a set of reads and writes can be performed atomically. In the
252255
* updateFunction, the client can read and write values using the supplied transaction object.
253-
* After the updateFunction, all changes will be committed. If some other client has changed any
254-
* of the data referenced, then the updateFunction will be called again. If the updateFunction
255-
* still fails after the given number of retries, then the transaction will be rejected.
256+
* After the updateFunction, all changes will be committed. If a retryable error occurs (ex: some
257+
* other client has changed any of the data referenced), then the updateFunction will be called
258+
* again after a backoff. If the updateFunction still fails after the given number of retires,
259+
* then the transaction will be rejected.
256260
*
257261
* <p>The transaction object passed to the updateFunction contains methods for accessing documents
258262
* and collections. Unlike other datastore access, data accessed with the transaction will not
@@ -261,36 +265,58 @@ private void addUserCallback(int batchId, TaskCompletionSource<Void> userTask) {
261265
*
262266
* <p>The Task returned is resolved when the transaction is fully committed.
263267
*/
264-
public <TResult> Task<TResult> transaction(
265-
AsyncQueue asyncQueue, Function<Transaction, Task<TResult>> updateFunction, int retries) {
268+
public <TResult> void transaction(
269+
AsyncQueue asyncQueue,
270+
ExponentialBackoff backoff,
271+
TaskCompletionSource<TResult> txTaskSource,
272+
Function<Transaction, Task<TResult>> updateFunction,
273+
int retries) {
266274
hardAssert(retries >= 0, "Got negative number of retries for transaction.");
267-
final Transaction transaction = remoteStore.createTransaction();
268-
return updateFunction
269-
.apply(transaction)
270-
.continueWithTask(
271-
asyncQueue.getExecutor(),
272-
userTask -> {
273-
if (!userTask.isSuccessful()) {
274-
if (retries > 0 && isRetryableTransactionError(userTask.getException())) {
275-
return transaction(asyncQueue, updateFunction, retries - 1);
276-
}
277-
return userTask;
278-
}
279-
return transaction
280-
.commit()
281-
.continueWithTask(
282-
asyncQueue.getExecutor(),
283-
commitTask -> {
284-
if (commitTask.isSuccessful()) {
285-
return Tasks.forResult(userTask.getResult());
286-
}
287-
Exception e = commitTask.getException();
288-
if (retries > 0 && isRetryableTransactionError(e)) {
289-
return transaction(asyncQueue, updateFunction, retries - 1);
275+
276+
backoff.backoffAndRun(
277+
() -> {
278+
final Transaction transaction = remoteStore.createTransaction();
279+
updateFunction
280+
.apply(transaction)
281+
.addOnCompleteListener(
282+
asyncQueue.getExecutor(),
283+
new OnCompleteListener<TResult>() {
284+
@Override
285+
public void onComplete(@NonNull Task<TResult> userTask) {
286+
if (!userTask.isSuccessful()) {
287+
if (retries > 0 && isRetryableTransactionError(userTask.getException())) {
288+
transaction(
289+
asyncQueue, backoff, txTaskSource, updateFunction, retries - 1);
290+
} else {
291+
txTaskSource.setException(userTask.getException());
290292
}
291-
return Tasks.forException(e);
292-
});
293-
});
293+
} else {
294+
transaction
295+
.commit()
296+
.addOnCompleteListener(
297+
asyncQueue.getExecutor(),
298+
new OnCompleteListener<Void>() {
299+
@Override
300+
public void onComplete(@NonNull Task<Void> commitTask) {
301+
if (commitTask.isSuccessful()) {
302+
txTaskSource.setResult(userTask.getResult());
303+
} else if (retries > 0
304+
&& isRetryableTransactionError(commitTask.getException())) {
305+
transaction(
306+
asyncQueue,
307+
backoff,
308+
txTaskSource,
309+
updateFunction,
310+
retries - 1);
311+
} else {
312+
txTaskSource.setException(commitTask.getException());
313+
}
314+
}
315+
});
316+
}
317+
}
318+
});
319+
});
294320
}
295321

296322
/** Called by FirestoreClient to notify us of a new remote event. */

‎firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ public enum TimerId {
6969
*/
7070
ONLINE_STATE_TIMEOUT,
7171
/** A timer used to periodically attempt LRU Garbage collection */
72-
GARBAGE_COLLECTION
72+
GARBAGE_COLLECTION,
73+
/** A timer used to retry transactions */
74+
RETRY_TRANSACTION
7375
}
7476

7577
/**

‎firebase-firestore/src/main/java/com/google/firebase/firestore/util/ExponentialBackoff.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@
2020

2121
/** Helper to implement exponential backoff. */
2222
public class ExponentialBackoff {
23+
24+
/**
25+
* Initial backoff time in milliseconds after an error. Set to 1s according to
26+
* https://cloud.google.com/apis/design/errors.
27+
*/
28+
public static final long DEFAULT_BACKOFF_INITIAL_DELAY_MS = 1000;
29+
30+
public static final double DEFAULT_BACKOFF_FACTOR = 1.5;
31+
32+
/** Maximum backoff time in milliseconds */
33+
public static final long DEFAULT_BACKOFF_MAX_DELAY_MS = 60 * 1000;
34+
2335
private final AsyncQueue queue;
2436
private final TimerId timerId;
2537
private final long initialDelayMs;
@@ -64,6 +76,17 @@ public ExponentialBackoff(
6476
reset();
6577
}
6678

79+
public ExponentialBackoff(AsyncQueue queue, AsyncQueue.TimerId timerId) {
80+
this.queue = queue;
81+
this.timerId = timerId;
82+
this.initialDelayMs = DEFAULT_BACKOFF_INITIAL_DELAY_MS;
83+
this.backoffFactor = DEFAULT_BACKOFF_FACTOR;
84+
this.maxDelayMs = DEFAULT_BACKOFF_MAX_DELAY_MS;
85+
this.lastAttemptTime = new Date().getTime();
86+
87+
reset();
88+
}
89+
6790
/**
6891
* Resets the backoff delay.
6992
*

0 commit comments

Comments
 (0)