Skip to content

Commit

Permalink
Standardize transaction retries to attempts (#5075)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Chen authored Jun 29, 2021
1 parent 5d31e21 commit 74ed43d
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
9 changes: 5 additions & 4 deletions packages/firestore/src/core/transaction_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ import { isNullOrUndefined } from '../util/types';

import { Transaction } from './transaction';

const RETRY_COUNT = 5;
export const DEFAULT_MAX_ATTEMPTS_COUNT = 5;

/**
* TransactionRunner encapsulates the logic needed to run and retry transactions
* with backoff.
*/
export class TransactionRunner<T> {
private retries = RETRY_COUNT;
private attemptsRemaining = DEFAULT_MAX_ATTEMPTS_COUNT;
private backoff: ExponentialBackoff;

constructor(
Expand All @@ -49,6 +49,7 @@ export class TransactionRunner<T> {

/** Runs the transaction and sets the result on deferred. */
run(): void {
this.attemptsRemaining -= 1;
this.runWithBackOff();
}

Expand Down Expand Up @@ -99,8 +100,8 @@ export class TransactionRunner<T> {
}

private handleTransactionError(error: Error): void {
if (this.retries > 0 && this.isRetryableTransactionError(error)) {
this.retries -= 1;
if (this.attemptsRemaining > 0 && this.isRetryableTransactionError(error)) {
this.attemptsRemaining -= 1;
this.asyncQueue.enqueueAndForget(() => {
this.runWithBackOff();
return Promise.resolve();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import * as firestore from '@firebase/firestore-types';
import { expect } from 'chai';

import { DEFAULT_MAX_ATTEMPTS_COUNT } from '../../../src/core/transaction_runner';
import { TimerId } from '../../../src/util/async_queue';
import { Deferred } from '../../util/promise';
import * as integrationHelpers from '../util/helpers';
Expand Down Expand Up @@ -183,6 +184,7 @@ apiDescribe(
.then(() => doc.get())
.then(snapshot => {
expect(snapshot.data()!['count']).to.equal(1234 + counter);
expect(counter).to.equal(DEFAULT_MAX_ATTEMPTS_COUNT);
});
});
});
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/test/integration/util/internal_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export function getDefaultDatabaseInfo(): DatabaseInfo {
DEFAULT_SETTINGS.host!,
!!DEFAULT_SETTINGS.ssl,
!!DEFAULT_SETTINGS.experimentalForceLongPolling,
!!DEFAULT_SETTINGS.experimentalAutoDetectLongPolling
!!DEFAULT_SETTINGS.experimentalAutoDetectLongPolling,
/*use FetchStreams= */ false
);
}

Expand Down

0 comments on commit 74ed43d

Please sign in to comment.