From 05d1cdf14080bf636b28f679da350c22448e16a9 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Mon, 6 Jun 2022 15:48:54 -0400
Subject: [PATCH] (core) limit retries of uploads to external store in tests

Summary:
If an external store fails completely, Grist will continue to
retry uploading to it. This diff updated the HostedStorageManager
test to limit the extent of these retries to the test itself -
otherwise they continue for all other tests in the same process,
potentially disrupting those that read logs. There are other tests
that use s3, but they aren't run in the same process with delicate
log-reading tests, and it isn't quite as clear what improvement
to make there.

Test Plan:
artificially made external store fail, and checked that
test contamination seen previously no longer occurs.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3469
---
 app/common/KeyedOps.ts                 | 18 ++++++++++++++++--
 app/server/lib/HostedStorageManager.ts |  7 +++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/app/common/KeyedOps.ts b/app/common/KeyedOps.ts
index 58fe6e12..127ed9bf 100644
--- a/app/common/KeyedOps.ts
+++ b/app/common/KeyedOps.ts
@@ -9,6 +9,7 @@ export class KeyedOps {
                                                              // (will accumulate without limit)
   private _changed = new Set<string>();    // set when key needs an operation
   private _operating = new Set<string>();  // set when operation is in progress for key
+  private _stopped: boolean = false;       // set to prohibit all new operations or retries
 
   /**
    * Provide a function to apply operation, and some optional
@@ -74,6 +75,14 @@ export class KeyedOps {
     }
   }
 
+  /**
+   * Don't allow any more operations, or retries of existing operations.
+   */
+  public stopOperations() {
+    this._stopped = true;
+    this.expediteOperations();
+  }
+
   /**
    * Wait for all operations to complete.  This makes most sense to use during
    * shutdown - otherwise it might be a very long wait to reach a moment where
@@ -154,6 +163,11 @@ export class KeyedOps {
     return hist;
   }
 
+  private async _doOp(key: string) {
+    if (this._stopped) { throw new Error('operations forcibly stopped'); }
+    return this._op(key);
+  }
+
   // Implement the next scheduled operation for a resource.
   private _update(key: string) {
     const status = this._getOperationStatus(key);
@@ -171,7 +185,7 @@ export class KeyedOps {
     history.lastStart = Date.now();
 
     // Store a promise for the operation.
-    status.promise = this._op(key).then(() => {
+    status.promise = this._doOp(key).then(() => {
       // Successful push!  Reset failure count, notify callbacks.
       status.failures = 0;
       status.callbacks.forEach(callback => callback());
@@ -179,7 +193,7 @@ export class KeyedOps {
     }).catch(err => {
       // Operation failed.  Increment failure count, notify callbacks.
       status.failures++;
-      if (this._options.retry) {
+      if (this._options.retry && !this._stopped) {
         this._changed.add(key);
       }
       if (this._options.logError) {
diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts
index e129f578..75e371a8 100644
--- a/app/server/lib/HostedStorageManager.ts
+++ b/app/server/lib/HostedStorageManager.ts
@@ -438,6 +438,13 @@ export class HostedStorageManager implements IDocStorageManager {
     this._uploads.expediteOperations();
   }
 
+  // forcibly stop operations that might otherwise retry indefinitely,
+  // for testing purposes.
+  public async testStopOperations() {
+    this._uploads.stopOperations();
+    await this._uploads.wait();
+  }
+
   /**
    * Finalize any operations involving the named document.
    */