Skip to content

fix(firestore): retry idempotent commits on DEADLINE_EXCEEDED #15010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
MarkDuckworth wants to merge 2 commits into
base: main
Choose a base branch
from
+542 −13
Draft

fix(firestore): retry idempotent commits on DEADLINE_EXCEEDED #15010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
691d7b7
fix(firestore): retry idempotent commits on DEADLINE_EXCEEDED
MarkDuckworth Nov 13, 2026
3074461
Merge branch 'main' into feat/retry-idempotent-commits
MarkDuckworth Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions ...re/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,45 +31,53 @@
final class DocumentTransform {

private final SortedMap<FieldPath, FieldTransform> transforms; // Sorted for testing.
private final boolean isIdempotent;

private DocumentTransform(SortedMap<FieldPath, FieldTransform> transforms) {
private DocumentTransform(SortedMap<FieldPath, FieldTransform> transforms, boolean isIdempotent) {
this.transforms = transforms;
this.isIdempotent = isIdempotent;
}

static DocumentTransform fromFieldPathMap(Map<FieldPath, Object> values) {
SortedMap<FieldPath, FieldTransform> transforms = new TreeMap<>();
boolean isIdempotent = true;

for (Map.Entry<FieldPath, Object> entry : values.entrySet()) {
FieldPath path = entry.getKey();
Object value = entry.getValue();
if (value instanceof FieldValue) {
FieldValue fieldValue = (FieldValue) value;
isIdempotent &= fieldValue.isIdempotent();
if (fieldValue.includeInDocumentTransform()) {
transforms.put(path, fieldValue.toProto(path));
}
} else if (value instanceof Map) {
transforms.putAll(
extractFromMap((Map<String, Object>) value, path, /* allowTransforms= */ true));
DocumentTransform subTransform =
extractFromMap((Map<String, Object>) value, path, /* allowTransforms= */ true);
transforms.putAll(subTransform.transforms);
isIdempotent &= subTransform.isIdempotent;
} else if (entry.getValue() instanceof List) {
// DocumentTransforms cannot be contained in Lists. We still process the List for
// validation.
validateArray((List<Object>) entry.getValue(), path);
}
}

return new DocumentTransform(transforms);
return new DocumentTransform(transforms, isIdempotent);
}

private static SortedMap<FieldPath, FieldTransform> extractFromMap(
private static DocumentTransform extractFromMap(
Map<String, Object> values, FieldPath path, boolean allowTransforms) {
SortedMap<FieldPath, FieldTransform> transforms = new TreeMap<>();
boolean isIdempotent = true;

for (Map.Entry<String, Object> entry : values.entrySet()) {
Object value = entry.getValue();
path = path.append(FieldPath.of(entry.getKey()));
if (value instanceof FieldValue) {
FieldValue fieldValue = (FieldValue) value;
if (allowTransforms) {
isIdempotent &= fieldValue.isIdempotent();
if (fieldValue.includeInDocumentTransform()) {
transforms.put(path, fieldValue.toProto(path));
}
Expand All @@ -78,14 +86,17 @@ private static SortedMap<FieldPath, FieldTransform> extractFromMap(
fieldValue.getMethodName() + " is not supported inside of an array.");
}
} else if (value instanceof Map) {
transforms.putAll(extractFromMap((Map<String, Object>) value, path, allowTransforms));
DocumentTransform subTransform =
extractFromMap((Map<String, Object>) value, path, allowTransforms);
transforms.putAll(subTransform.transforms);
isIdempotent &= subTransform.isIdempotent;
} else if (entry.getValue() instanceof List) {
validateArray((List<Object>) entry.getValue(), path);
}
path = path.getParent();
}

return transforms;
return new DocumentTransform(transforms, isIdempotent);
}

private static void validateArray(List<Object> values, FieldPath path) {
Expand All @@ -108,6 +119,10 @@ boolean isEmpty() {
return transforms.isEmpty();
}

boolean isIdempotent() {
return isIdempotent;
}

Set<FieldPath> getFields() {
return Collections.unmodifiableSet(transforms.keySet());
}
Expand Down
38 changes: 38 additions & 0 deletions ...firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.serverTimestamp()";
Expand All @@ -66,6 +71,11 @@ boolean includeInDocumentTransform() {
return false;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.delete()";
Expand Down Expand Up @@ -95,6 +105,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return false;
}

@Override
String getMethodName() {
return "FieldValue.increment()";
Expand Down Expand Up @@ -144,6 +159,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.minimum()";
Expand Down Expand Up @@ -193,6 +213,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.maximum()";
Expand Down Expand Up @@ -242,6 +267,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.arrayUnion()";
Expand Down Expand Up @@ -298,6 +328,11 @@ boolean includeInDocumentTransform() {
return true;
}

@Override
boolean isIdempotent() {
return true;
}

@Override
String getMethodName() {
return "FieldValue.arrayRemove()";
Expand Down Expand Up @@ -491,6 +526,9 @@ public static VectorValue vector(@Nonnull double[] values) {
/** Whether this FieldTransform should be included in the list of document transforms. */
abstract boolean includeInDocumentTransform();

/** Whether this FieldValue operation is idempotent and safe to retry on DEADLINE_EXCEEDED. */
abstract boolean isIdempotent();

/** The name of the method that returned this FieldValue instance. */
abstract String getMethodName();

Expand Down
120 changes: 114 additions & 6 deletions ...estore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,20 @@
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.core.InternalExtensionOnly;
import com.google.api.gax.grpc.GrpcCallContext;
import com.google.api.gax.rpc.StatusCode;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.cloud.firestore.UserDataConverter.EncodingOptions;
import com.google.cloud.firestore.encoding.CustomClassMapper;
import com.google.cloud.firestore.telemetry.TelemetryConstants;
import com.google.cloud.firestore.telemetry.TraceUtil;
import com.google.cloud.firestore.telemetry.TraceUtil.Scope;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.firestore.v1.CommitRequest;
import com.google.firestore.v1.CommitResponse;
import com.google.firestore.v1.DocumentTransform.FieldTransform;
import com.google.firestore.v1.Write;
import com.google.protobuf.ByteString;
import com.google.protobuf.Timestamp;
Expand All @@ -58,13 +63,25 @@
*/
@InternalExtensionOnly
public abstract class UpdateBuilder<T> {
static final Set<StatusCode.Code> IDEMPOTENT_COMMIT_RETRYABLE_CODES =
ImmutableSet.of(
StatusCode.Code.RESOURCE_EXHAUSTED,
StatusCode.Code.UNAVAILABLE,
StatusCode.Code.DEADLINE_EXCEEDED);

static final class WriteOperation {
final Write write;
final DocumentReference documentReference;
final boolean isIdempotent;

WriteOperation(DocumentReference documentReference, Write write) {
this(documentReference, write, isIdempotent(write));
}

WriteOperation(DocumentReference documentReference, Write write, boolean isIdempotent) {
this.documentReference = documentReference;
this.write = write;
this.isIdempotent = isIdempotent;
}

@Override
Expand Down Expand Up @@ -165,7 +182,7 @@ private T performCreate(
write.addAllUpdateTransforms(documentTransform.toPb());
}

return addWrite(documentReference, write);
return addWrite(documentReference, write, documentTransform.isIdempotent());
}

/**
Expand Down Expand Up @@ -289,11 +306,19 @@ private T performSet(
write.setUpdateMask(documentMask.toPb());
}

return addWrite(documentReference, write);
return addWrite(documentReference, write, documentTransform.isIdempotent());
}

private T addWrite(DocumentReference documentReference, Write.Builder write) {
WriteOperation operation = new WriteOperation(documentReference, write.build());
return addWrite(documentReference, write, /* isTransformIdempotent= */ true);
}

private T addWrite(
DocumentReference documentReference, Write.Builder write, boolean isTransformIdempotent) {
Write builtWrite = write.build();
WriteOperation operation =
new WriteOperation(
documentReference, builtWrite, isTransformIdempotent && isIdempotent(builtWrite));
int writeIndex;
synchronized (writes) {
Preconditions.checkState(
Expand Down Expand Up @@ -571,7 +596,7 @@ public boolean allowTransform() {
write.addAllUpdateTransforms(documentTransform.toPb());
}

return addWrite(documentReference, write);
return addWrite(documentReference, write, documentTransform.isIdempotent());
}

/**
Expand Down Expand Up @@ -640,8 +665,19 @@ ApiFuture<List<WriteResult>> commit(@Nullable ByteString transactionId) {
committed = true;
CommitRequest request = buildCommitRequest(transactionId);

ApiFuture<CommitResponse> response =
firestore.sendRequest(request, firestore.getClient().commitCallable());
UnaryCallable<CommitRequest, CommitResponse> callable =
firestore.getClient().commitCallable();
if (callable != null
&& transactionId == null
&& isIdempotentBatch()
&& isIdempotentCommit(request)) {
callable =
callable.withDefaultCallContext(
GrpcCallContext.createDefault()
.withRetryableCodes(IDEMPOTENT_COMMIT_RETRYABLE_CODES));
}

ApiFuture<CommitResponse> response = firestore.sendRequest(request, callable);

ApiFuture<List<WriteResult>> returnValue =
ApiFutures.transform(
Expand Down Expand Up @@ -671,6 +707,78 @@ private CommitRequest buildCommitRequest(ByteString transactionId) {
return builder.build();
}

boolean isIdempotentBatch() {
synchronized (writes) {
for (WriteOperation writeOperation : writes) {
if (!writeOperation.isIdempotent) {
return false;
}
}
return true;
}
}

static boolean isIdempotentCommit(CommitRequest request) {
if (!request.getTransaction().isEmpty()) {
return false;
}
for (Write write : request.getWritesList()) {
if (!isIdempotent(write)) {
return false;
}
}
return true;
}

static boolean isIdempotent(Write write) {
if (write.hasCurrentDocument()) {
com.google.firestore.v1.Precondition currentDocument = write.getCurrentDocument();
switch (currentDocument.getConditionTypeCase()) {
case UPDATE_TIME:
return false;
case EXISTS:
if (!currentDocument.getExists()) {
return false;
}
if (write.getOperationCase() == Write.OperationCase.DELETE) {
return false;
}
break;
case CONDITIONTYPE_NOT_SET:
default:
break;
}
}
for (FieldTransform transform : write.getUpdateTransformsList()) {
if (!isIdempotentTransform(transform)) {
return false;
}
}
if (write.hasTransform()) {
for (FieldTransform transform : write.getTransform().getFieldTransformsList()) {
if (!isIdempotentTransform(transform)) {
return false;
}
}
}
return true;
}

private static boolean isIdempotentTransform(FieldTransform transform) {
switch (transform.getTransformTypeCase()) {
case SET_TO_SERVER_VALUE:
case APPEND_MISSING_ELEMENTS:
case REMOVE_ALL_FROM_ARRAY:
case MAXIMUM:
case MINIMUM:
return true;
case INCREMENT:
case TRANSFORMTYPE_NOT_SET:
default:
return false;
}
}

/** Checks whether any updates have been queued. */
boolean isEmpty() {
synchronized (writes) {
Expand Down
Loading
Loading