Skip to content

Commit 79a8dd2

Browse files
schmidt-sebastianschmidt-sebastian
committed
feat: base transaction on error codes
Port of googleapis/nodejs-firestore#953
1 parent fa8ddc1 commit 79a8dd2

File tree

11 files changed

+563
-271
lines changed

11 files changed

+563
-271
lines changed

‎google-cloud-firestore/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
<dependency>
6969
<groupId>com.google.api</groupId>
7070
<artifactId>api-common</artifactId>
71+
<version>1.8.2-SNAPSHOT</version>
7172
</dependency>
7273
<dependency>
7374
<groupId>io.grpc</groupId>

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreException.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,16 @@ private FirestoreException(String reason, Status status, @Nullable Throwable cau
3737
this.status = status;
3838
}
3939

40-
private FirestoreException(IOException exception, boolean retryable) {
41-
super(exception, retryable);
40+
private FirestoreException(String reason, ApiException exception) {
41+
super(
42+
reason,
43+
exception,
44+
exception.getStatusCode().getCode().getHttpStatusCode(),
45+
exception.isRetryable());
4246
}
4347

44-
private FirestoreException(ApiException exception) {
45-
super(exception);
48+
private FirestoreException(IOException exception, boolean retryable) {
49+
super(exception, retryable);
4650
}
4751

4852
/**
@@ -91,7 +95,16 @@ static FirestoreException networkException(IOException exception, boolean retrya
9195
* @return The FirestoreException
9296
*/
9397
static FirestoreException apiException(ApiException exception) {
94-
return new FirestoreException(exception);
98+
return new FirestoreException(exception.getMessage(), exception);
99+
}
100+
101+
/**
102+
* Creates a FirestoreException from an ApiException.
103+
*
104+
* @return The FirestoreException
105+
*/
106+
static FirestoreException apiException(ApiException exception, String message) {
107+
return new FirestoreException(message, exception);
95108
}
96109

97110
@InternalApi

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java‎

Lines changed: 7 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
package com.google.cloud.firestore;
1818

1919
import com.google.api.core.ApiFuture;
20-
import com.google.api.core.ApiFutureCallback;
21-
import com.google.api.core.ApiFutures;
2220
import com.google.api.core.SettableApiFuture;
23-
import com.google.api.gax.rpc.ApiException;
2421
import com.google.api.gax.rpc.ApiStreamObserver;
2522
import com.google.api.gax.rpc.BidiStreamingCallable;
2623
import com.google.api.gax.rpc.ServerStreamingCallable;
@@ -29,16 +26,12 @@
2926
import com.google.cloud.firestore.spi.v1.FirestoreRpc;
3027
import com.google.common.base.Preconditions;
3128
import com.google.common.collect.ImmutableMap;
32-
import com.google.common.util.concurrent.MoreExecutors;
3329
import com.google.firestore.v1.BatchGetDocumentsRequest;
3430
import com.google.firestore.v1.BatchGetDocumentsResponse;
3531
import com.google.firestore.v1.DatabaseRootName;
3632
import com.google.protobuf.ByteString;
3733
import io.grpc.Context;
38-
import io.grpc.Status;
39-
import io.opencensus.common.Scope;
4034
import io.opencensus.trace.AttributeValue;
41-
import io.opencensus.trace.Span;
4235
import io.opencensus.trace.Tracer;
4336
import io.opencensus.trace.Tracing;
4437
import java.util.ArrayList;
@@ -47,7 +40,6 @@
4740
import java.util.Map;
4841
import java.util.Random;
4942
import java.util.concurrent.Executor;
50-
import java.util.logging.Logger;
5143
import javax.annotation.Nonnull;
5244
import javax.annotation.Nullable;
5345

@@ -62,10 +54,7 @@ class FirestoreImpl implements Firestore {
6254
private static final String AUTO_ID_ALPHABET =
6355
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
6456

65-
private static final Logger LOGGER = Logger.getLogger("Firestore");
6657
private static final Tracer tracer = Tracing.getTracer();
67-
private static final io.opencensus.trace.Status TOO_MANY_RETRIES_STATUS =
68-
io.opencensus.trace.Status.ABORTED.withDescription("too many retries");
6958

7059
private final FirestoreRpc firestoreClient;
7160
private final FirestoreOptions firestoreOptions;
@@ -298,146 +287,17 @@ public <T> ApiFuture<T> runTransaction(@Nonnull final Transaction.Function<T> up
298287
public <T> ApiFuture<T> runTransaction(
299288
@Nonnull final Transaction.Function<T> updateFunction,
300289
@Nonnull TransactionOptions transactionOptions) {
301-
SettableApiFuture<T> resultFuture = SettableApiFuture.create();
302-
runTransaction(updateFunction, resultFuture, transactionOptions);
303-
return resultFuture;
304-
}
305-
306-
/** Transaction functions that returns its result in the provided SettableFuture. */
307-
private <T> void runTransaction(
308-
final Transaction.Function<T> transactionCallback,
309-
final SettableApiFuture<T> resultFuture,
310-
final TransactionOptions options) {
311-
// span is intentionally not ended here. It will be ended by runTransactionAttempt on success
312-
// or error.
313-
Span span = tracer.spanBuilder("CloudFirestore.Transaction").startSpan();
314-
try (Scope s = tracer.withSpan(span)) {
315-
runTransactionAttempt(transactionCallback, resultFuture, options, span);
316-
}
317-
}
318-
319-
private <T> void runTransactionAttempt(
320-
final Transaction.Function<T> transactionCallback,
321-
final SettableApiFuture<T> resultFuture,
322-
final TransactionOptions options,
323-
final Span span) {
324-
final Transaction transaction = new Transaction(this, options.getPreviousTransactionId());
325290
final Executor userCallbackExecutor =
326291
Context.currentContextExecutor(
327-
options.getExecutor() != null ? options.getExecutor() : firestoreClient.getExecutor());
292+
transactionOptions.getExecutor() != null
293+
? transactionOptions.getExecutor()
294+
: firestoreClient.getExecutor());
328295

329-
final int attemptsRemaining = options.getNumberOfAttempts() - 1;
330-
span.addAnnotation(
331-
"Start runTransaction",
332-
ImmutableMap.of("attemptsRemaining", AttributeValue.longAttributeValue(attemptsRemaining)));
296+
TransactionRunner<T> transactionRunner =
297+
new TransactionRunner<>(
298+
this, updateFunction, userCallbackExecutor, transactionOptions.getNumberOfAttempts());
333299

334-
ApiFutures.addCallback(
335-
transaction.begin(),
336-
new ApiFutureCallback<Void>() {
337-
@Override
338-
public void onFailure(Throwable throwable) {
339-
// Don't retry failed BeginTransaction requests.
340-
rejectTransaction(throwable);
341-
}
342-
343-
@Override
344-
public void onSuccess(Void ignored) {
345-
ApiFutures.addCallback(
346-
invokeUserCallback(),
347-
new ApiFutureCallback<T>() {
348-
@Override
349-
public void onFailure(Throwable throwable) {
350-
// This was a error in the user callback, forward the throwable.
351-
rejectTransaction(throwable);
352-
}
353-
354-
@Override
355-
public void onSuccess(final T userResult) {
356-
// Commit the transaction
357-
ApiFutures.addCallback(
358-
transaction.commit(),
359-
new ApiFutureCallback<List<WriteResult>>() {
360-
@Override
361-
public void onFailure(Throwable throwable) {
362-
// Retry failed commits.
363-
maybeRetry(throwable);
364-
}
365-
366-
@Override
367-
public void onSuccess(List<WriteResult> writeResults) {
368-
span.setStatus(io.opencensus.trace.Status.OK);
369-
span.end();
370-
resultFuture.set(userResult);
371-
}
372-
},
373-
MoreExecutors.directExecutor());
374-
}
375-
},
376-
MoreExecutors.directExecutor());
377-
}
378-
379-
private SettableApiFuture<T> invokeUserCallback() {
380-
// Execute the user callback on the provided executor.
381-
final SettableApiFuture<T> callbackResult = SettableApiFuture.create();
382-
userCallbackExecutor.execute(
383-
new Runnable() {
384-
@Override
385-
public void run() {
386-
try {
387-
callbackResult.set(transactionCallback.updateCallback(transaction));
388-
} catch (Throwable t) {
389-
callbackResult.setException(t);
390-
}
391-
}
392-
});
393-
return callbackResult;
394-
}
395-
396-
private void maybeRetry(Throwable throwable) {
397-
if (attemptsRemaining > 0) {
398-
span.addAnnotation("retrying");
399-
runTransactionAttempt(
400-
transactionCallback,
401-
resultFuture,
402-
new TransactionOptions(
403-
attemptsRemaining, options.getExecutor(), transaction.getTransactionId()),
404-
span);
405-
} else {
406-
span.setStatus(TOO_MANY_RETRIES_STATUS);
407-
rejectTransaction(
408-
FirestoreException.serverRejected(
409-
Status.ABORTED,
410-
throwable,
411-
"Transaction was cancelled because of too many retries."));
412-
}
413-
}
414-
415-
private void rejectTransaction(final Throwable throwable) {
416-
if (throwable instanceof ApiException) {
417-
span.setStatus(TraceUtil.statusFromApiException((ApiException) throwable));
418-
}
419-
span.end();
420-
if (transaction.isPending()) {
421-
ApiFutures.addCallback(
422-
transaction.rollback(),
423-
new ApiFutureCallback<Void>() {
424-
@Override
425-
public void onFailure(Throwable throwable) {
426-
resultFuture.setException(throwable);
427-
}
428-
429-
@Override
430-
public void onSuccess(Void ignored) {
431-
resultFuture.setException(throwable);
432-
}
433-
},
434-
MoreExecutors.directExecutor());
435-
} else {
436-
resultFuture.setException(throwable);
437-
}
438-
}
439-
},
440-
MoreExecutors.directExecutor());
300+
return transactionRunner.run();
441301
}
442302

443303
/** Returns whether the user has opted into receiving dates as com.google.cloud.Timestamp. */

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,25 +51,15 @@ public interface Function<T> {
5151
T updateCallback(Transaction transaction) throws Exception;
5252
}
5353

54-
private final ByteString previousTransactionId;
5554
private ByteString transactionId;
56-
private boolean pending;
55+
private @Nullable ByteString previousTransactionId;
5756

58-
Transaction(FirestoreImpl firestore, @Nullable ByteString previousTransactionId) {
57+
Transaction(FirestoreImpl firestore, @Nullable Transaction previousTransaction) {
5958
super(firestore);
60-
this.previousTransactionId = previousTransactionId;
59+
previousTransactionId = previousTransaction != null ? previousTransaction.transactionId : null;
6160
}
6261

63-
@Nullable
64-
ByteString getTransactionId() {
65-
return transactionId;
66-
}
67-
68-
boolean isPending() {
69-
return pending;
70-
}
71-
72-
/** Starts a transaction and obtains the transaction id from the server. */
62+
/** Starts a transaction and obtains the transaction id. */
7363
ApiFuture<Void> begin() {
7464
BeginTransactionRequest.Builder beginTransaction = BeginTransactionRequest.newBuilder();
7565
beginTransaction.setDatabase(firestore.getDatabaseName());
@@ -91,7 +81,6 @@ ApiFuture<Void> begin() {
9181
@Override
9282
public Void apply(BeginTransactionResponse beginTransactionResponse) {
9383
transactionId = beginTransactionResponse.getTransaction();
94-
pending = true;
9584
return null;
9685
}
9786
},
@@ -100,14 +89,11 @@ public Void apply(BeginTransactionResponse beginTransactionResponse) {
10089

10190
/** Commits a transaction. */
10291
ApiFuture<List<WriteResult>> commit() {
103-
pending = false;
10492
return super.commit(transactionId);
10593
}
10694

10795
/** Rolls a transaction back and releases all read locks. */
10896
ApiFuture<Void> rollback() {
109-
pending = false;
110-
11197
RollbackRequest.Builder reqBuilder = RollbackRequest.newBuilder();
11298
reqBuilder.setTransaction(transactionId);
11399
reqBuilder.setDatabase(firestore.getDatabaseName());

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionOptions.java‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.google.cloud.firestore;
1818

1919
import com.google.common.base.Preconditions;
20-
import com.google.protobuf.ByteString;
2120
import java.util.concurrent.Executor;
2221
import javax.annotation.Nonnull;
2322
import javax.annotation.Nullable;
@@ -29,12 +28,10 @@ public final class TransactionOptions {
2928

3029
private final int numberOfAttempts;
3130
private final Executor executor;
32-
private final ByteString previousTransactionId;
3331

34-
TransactionOptions(int maxAttempts, Executor executor, ByteString previousTransactionId) {
32+
TransactionOptions(int maxAttempts, Executor executor) {
3533
this.numberOfAttempts = maxAttempts;
3634
this.executor = executor;
37-
this.previousTransactionId = previousTransactionId;
3835
}
3936

4037
public int getNumberOfAttempts() {
@@ -46,11 +43,6 @@ public Executor getExecutor() {
4643
return executor;
4744
}
4845

49-
@Nullable
50-
ByteString getPreviousTransactionId() {
51-
return previousTransactionId;
52-
}
53-
5446
/**
5547
* Create a default set of options suitable for most use cases. Transactions will be attempted 5
5648
* times.
@@ -59,7 +51,7 @@ ByteString getPreviousTransactionId() {
5951
*/
6052
@Nonnull
6153
public static TransactionOptions create() {
62-
return new TransactionOptions(DEFAULT_NUM_ATTEMPTS, null, null);
54+
return new TransactionOptions(DEFAULT_NUM_ATTEMPTS, null);
6355
}
6456

6557
/**
@@ -71,7 +63,7 @@ public static TransactionOptions create() {
7163
@Nonnull
7264
public static TransactionOptions create(int numberOfAttempts) {
7365
Preconditions.checkArgument(numberOfAttempts > 0, "You must allow at least one attempt");
74-
return new TransactionOptions(numberOfAttempts, null, null);
66+
return new TransactionOptions(numberOfAttempts, null);
7567
}
7668

7769
/**
@@ -82,7 +74,7 @@ public static TransactionOptions create(int numberOfAttempts) {
8274
*/
8375
@Nonnull
8476
public static TransactionOptions create(@Nonnull Executor executor) {
85-
return new TransactionOptions(DEFAULT_NUM_ATTEMPTS, executor, null);
77+
return new TransactionOptions(DEFAULT_NUM_ATTEMPTS, executor);
8678
}
8779

8880
/**
@@ -95,6 +87,6 @@ public static TransactionOptions create(@Nonnull Executor executor) {
9587
@Nonnull
9688
public static TransactionOptions create(@Nonnull Executor executor, int numberOfAttempts) {
9789
Preconditions.checkArgument(numberOfAttempts > 0, "You must allow at least one attempt");
98-
return new TransactionOptions(numberOfAttempts, executor, null);
90+
return new TransactionOptions(numberOfAttempts, executor);
9991
}
10092
}

0 commit comments

Comments
 (0)