Retry WebStorage operations (#2879)

Author: schmidt-sebastian

packages/firestore/CHANGELOG.md

@@ -1,4 +1,8 @@
 # Unreleased
+- [fixed] Fixed a source of IndexedDB-related crashes for tabs that receive 
+  multi-tab notifications while the file system is locked.
+
+# 1.10.2
 - [fixed] Temporarily reverted the use of window.crypto to generate document
   IDs to address compatibility issues with IE 11, WebWorkers, and React Native.
 - [changed] Firestore now limits the number of concurrent document lookups it

packages/firestore/src/core/component_provider.ts

@@ -40,7 +40,6 @@ import { IndexFreeQueryEngine } from '../local/index_free_query_engine';
 import { IndexedDbPersistence } from '../local/indexeddb_persistence';
 import {
   MemoryEagerDelegate,
-  MemoryLruDelegate,
   MemoryPersistence
 } from '../local/memory_persistence';
 
@@ -189,23 +188,6 @@ export class MemoryComponentProvider {
   }
 }
 
-/**
- * Provides all components needed for Firestore with in-memory persistence.
- * Uses LRU garbage collection.
- */
-export class MemoryLruComponentProvider extends MemoryComponentProvider {
-  createPersistence(cfg: ComponentConfiguration): Persistence {
-    debugAssert(
-      !cfg.persistenceSettings.durable,
-      'Can only start memory persistence'
-    );
-    return new MemoryPersistence(
-      cfg.clientId,
-      p => new MemoryLruDelegate(p, LruParams.DEFAULT)
-    );
-  }
-}
-
 /**
  * Provides all components needed for Firestore with IndexedDB persistence.
  */

packages/firestore/src/core/transaction_runner.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2019 Google Inc.
+ * Copyright 2019 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,7 +42,7 @@ export class TransactionRunner<T> {
   ) {
     this.backoff = new ExponentialBackoff(
       this.asyncQueue,
-      TimerId.RetryTransaction
+      TimerId.TransactionRetry
     );
   }
 

packages/firestore/src/local/shared_client_state.ts

@@ -755,7 +755,7 @@ export class WebStorageSharedClientState implements SharedClientState {
         return;
       }
 
-      this.queue.enqueueAndForget(async () => {
+      this.queue.enqueueRetryable(async () => {
         if (!this.started) {
           this.earlyEvents.push(event);
           return;

packages/firestore/src/remote/backoff.ts

@@ -119,7 +119,7 @@ export class ExponentialBackoff {
       desiredDelayWithJitterMs - delaySoFarMs
     );
 
-    if (this.currentBaseMs > 0) {
+    if (remainingDelayMs > 0) {
       logDebug(
         LOG_TAG,
         `Backing off for ${remainingDelayMs} ms ` +

packages/firestore/src/util/async_queue.ts

@@ -17,8 +17,12 @@
 
 import { debugAssert, fail } from './assert';
 import { Code, FirestoreError } from './error';
-import { logError } from './log';
+import { logDebug, logError } from './log';
 import { CancelablePromise, Deferred } from './promise';
+import { ExponentialBackoff } from '../remote/backoff';
+import { PlatformSupport } from '../platform/platform';
+
+const LOG_TAG = 'AsyncQueue';
 
 // Accept any return type from setTimeout().
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -66,7 +70,13 @@ export const enum TimerId {
    * A timer used to retry transactions. Since there can be multiple concurrent
    * transactions, multiple of these may be in the queue at a given time.
    */
-  RetryTransaction = 'retry_transaction'
+  TransactionRetry = 'transaction_retry',
+
+  /**
+   * A timer used to retry operations scheduled via retryable AsyncQueue
+   * operations.
+   */
+  AsyncQueueRetry = 'async_queue_retry'
 }
 
 /**
@@ -195,6 +205,10 @@ export class AsyncQueue {
   // The last promise in the queue.
   private tail: Promise<unknown> = Promise.resolve();
 
+  // The last retryable operation. Retryable operation are run in order and
+  // retried with backoff.
+  private retryableTail: Promise<void> = Promise.resolve();
+
   // Is this AsyncQueue being shut down? Once it is set to true, it will not
   // be changed again.
   private _isShuttingDown: boolean = false;
@@ -213,6 +227,24 @@ export class AsyncQueue {
   // List of TimerIds to fast-forward delays for.
   private timerIdsToSkip: TimerId[] = [];
 
+  // Backoff timer used to schedule retries for retryable operations
+  private backoff = new ExponentialBackoff(this, TimerId.AsyncQueueRetry);
+
+  // Visibility handler that triggers an immediate retry of all retryable
+  // operations. Meant to speed up recovery when we regain file system access
+  // after page comes into foreground.
+  private visibilityHandler = (): void => {
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    this.runDelayedOperationsEarly(TimerId.AsyncQueueRetry);
+  };
+
+  constructor() {
+    const window = PlatformSupport.getPlatform().window;
+    if (window) {
+      window.addEventListener('visibilitychange', this.visibilityHandler);
+    }
+  }
+
   // Is this AsyncQueue being shut down? If true, this instance will not enqueue
   // any new operations, Promises from enqueue requests will not resolve.
   get isShuttingDown(): boolean {
@@ -262,6 +294,10 @@ export class AsyncQueue {
     this.verifyNotFailed();
     if (!this._isShuttingDown) {
       this._isShuttingDown = true;
+      const window = PlatformSupport.getPlatform().window;
+      if (window) {
+        window.removeEventListener('visibilitychange', this.visibilityHandler);
+      }
       await this.enqueueEvenAfterShutdown(op);
     }
   }
@@ -279,6 +315,37 @@ export class AsyncQueue {
     return this.enqueueInternal(op);
   }
 
+  /**
+   * Enqueue a retryable operation.
+   *
+   * A retryable operation is rescheduled with backoff if it fails with any
+   * exception. All retryable operations are executed in order and only run
+   * if all prior operations were retried successfully.
+   */
+  enqueueRetryable(op: () => Promise<void>): void {
+    this.verifyNotFailed();
+
+    if (this._isShuttingDown) {
+      return;
+    }
+
+    this.retryableTail = this.retryableTail.then(() => {
+      const deferred = new Deferred<void>();
+      const retryingOp = async (): Promise<void> => {
+        try {
+          await op();
+          deferred.resolve();
+          this.backoff.reset();
+        } catch (e) {
+          logDebug(LOG_TAG, 'Retryable operation failed: ' + e.message);
+          this.backoff.backoffAndRun(retryingOp);
+        }
+      };
+      this.enqueueAndForget(retryingOp);
+      return deferred.promise;
+    });
+  }
+
   private enqueueInternal<T extends unknown>(op: () => Promise<T>): Promise<T> {
     const newTail = this.tail.then(() => {
       this.operationInProgress = true;
@@ -399,12 +466,6 @@ export class AsyncQueue {
   runDelayedOperationsEarly(lastTimerId: TimerId): Promise<void> {
     // Note that draining may generate more delayed ops, so we do that first.
     return this.drain().then(() => {
-      debugAssert(
-        lastTimerId === TimerId.All ||
-          this.containsDelayedOperation(lastTimerId),
-        `Attempted to drain to missing operation ${lastTimerId}`
-      );
-
       // Run ops in the same order they'd run if they ran naturally.
       this.delayedOperations.sort((a, b) => a.targetTimeMs - b.targetTimeMs);
 

packages/firestore/test/integration/api_internal/transaction.test.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2019 Google Inc.
+ * Copyright 2019 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,7 +36,7 @@ apiDescribe(
       let started = 0;
 
       return integrationHelpers.withTestDb(persistence, db => {
-        asyncQueue(db).skipDelaysForTimerId(TimerId.RetryTransaction);
+        asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry);
         const doc = db.collection('counters').doc();
         return doc
           .set({
@@ -93,7 +93,7 @@ apiDescribe(
       let counter = 0;
 
       return integrationHelpers.withTestDb(persistence, db => {
-        asyncQueue(db).skipDelaysForTimerId(TimerId.RetryTransaction);
+        asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry);
         const doc = db.collection('counters').doc();
         return doc
           .set({
@@ -148,7 +148,7 @@ apiDescribe(
 
     it('handle reading a doc twice with different versions', () => {
       return integrationHelpers.withTestDb(persistence, db => {
-        asyncQueue(db).skipDelaysForTimerId(TimerId.RetryTransaction);
+        asyncQueue(db).skipDelaysForTimerId(TimerId.TransactionRetry);
         const doc = db.collection('counters').doc();
         let counter = 0;
         return doc

packages/firestore/test/unit/specs/recovery_spec.test.ts

@@ -0,0 +1,79 @@
+/**
+ * @license
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describeSpec, specTest } from './describe_spec';
+import { client } from './spec_builder';
+import { TimerId } from '../../../src/util/async_queue';
+import { Query } from '../../../src/core/query';
+import { path } from '../../util/helpers';
+
+describeSpec(
+  'Persistence Recovery',
+  ['durable-persistence', 'no-ios', 'no-android'],
+  () => {
+    specTest(
+      'Write is acknowledged by primary client (with recovery)',
+      ['multi-client'],
+      () => {
+        return (
+          client(0)
+            .expectPrimaryState(true)
+            .client(1)
+            .expectPrimaryState(false)
+            .userSets('collection/a', { v: 1 })
+            .failDatabase()
+            .client(0)
+            .writeAcks('collection/a', 1, { expectUserCallback: false })
+            .client(1)
+            // Client 1 has received the WebStorage notification that the write
+            // has been acknowledged, but failed to process the change. Hence,
+            // we did not get a user callback. We schedule the first retry and
+            // make sure that it also does not get processed until 
+            // `recoverDatabase` is called.
+            .runTimer(TimerId.AsyncQueueRetry)
+            .recoverDatabase()
+            .runTimer(TimerId.AsyncQueueRetry)
+            .expectUserCallbacks({
+              acknowledged: ['collection/a']
+            })
+        );
+      }
+    );
+
+    specTest(
+      'Query raises events in secondary client  (with recovery)',
+      ['multi-client'],
+      () => {
+        const query = Query.atPath(path('collection'));
+
+        return client(0)
+          .expectPrimaryState(true)
+          .client(1)
+          .expectPrimaryState(false)
+          .userListens(query)
+          .failDatabase()
+          .client(0)
+          .expectListen(query)
+          .watchAcksFull(query, 1000)
+          .client(1)
+          .recoverDatabase()
+          .runTimer(TimerId.AsyncQueueRetry)
+          .expectEvents(query, {});
+      }
+    );
+  }
+);

packages/firestore/test/unit/specs/spec_builder.ts

@@ -418,6 +418,24 @@ export class SpecBuilder {
     return this;
   }
 
+  /** Fails all database operations until `recoverDatabase()` is called. */
+  failDatabase(): this {
+    this.nextStep();
+    this.currentStep = {
+      failDatabase: true
+    };
+    return this;
+  }
+
+  /** Stops failing database operations. */
+  recoverDatabase(): this {
+    this.nextStep();
+    this.currentStep = {
+      failDatabase: false
+    };
+    return this;
+  }
+
   expectIsShutdown(): this {
     this.assertStep('Active target expectation requires previous step');
     const currentStep = this.currentStep!;