(core) Fix snapshot migrations

Summary:
Migrations were failing in snapshots due to the sandbox no longer
being started in snapshots. We now start up an instance of the
sandbox whenever there are migrations to run, and immediately shut
it down on completion.

Test Plan: Server test.

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D3898

app/server/lib/ActiveDoc.ts

@@ -97,7 +97,7 @@ import {GRIST_DOC_SQL, GRIST_DOC_WITH_TABLE1_SQL} from 'app/server/lib/initialDo
 import {ISandbox} from 'app/server/lib/ISandbox';
 import log from 'app/server/lib/log';
 import {LogMethods} from "app/server/lib/LogMethods";
-import {NullSandbox} from 'app/server/lib/NullSandbox';
+import {NullSandbox, UnavailableSandboxMethodError} from 'app/server/lib/NullSandbox';
 import {DocRequests} from 'app/server/lib/Requests';
 import {shortDesc} from 'app/server/lib/shortDesc';
 import {TableMetadataLoader} from 'app/server/lib/TableMetadataLoader';
@@ -212,7 +212,7 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
   private _log = new LogMethods('ActiveDoc ', (s: OptDocSession | null) => this.getLogMeta(s));
   private _triggers: DocTriggers;
   private _requests: DocRequests;
-  private _dataEngine: Promise<ISandbox>|undefined;
+  private _dataEngine: Promise<ISandbox>|null = null;
   private _activeDocImport: ActiveDocImport;
   private _onDemandActions: OnDemandActions;
   private _granularAccess: GranularAccess;
@@ -1809,7 +1809,9 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
       await this._beforeMigration(docSession, 'schema', docSchemaVersion, schemaVersion);
       let success: boolean = false;
       try {
-        await this._migrate(docSession);
+        await this._withDataEngine(() => this._migrate(docSession), {
+          shutdownAfter: this._isSnapshot,
+        });
         success = true;
       } finally {
         await this._afterMigration(docSession, 'schema', schemaVersion, success);
@@ -1825,8 +1827,11 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
         "proceeding with fingers crossed", docSchemaVersion, schemaVersion);
     }
 
+    if (!this._isSnapshot) {
+      this._tableMetadataLoader.startStreamingToEngine();
+    }
+
     // Start loading the initial meta tables which determine the document schema.
-    this._tableMetadataLoader.startStreamingToEngine();
     this._tableMetadataLoader.startFetchingTable('_grist_Tables');
     this._tableMetadataLoader.startFetchingTable('_grist_Tables_column');
 
@@ -1977,7 +1982,7 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
         await Promise.all([
           this.docStorage.shutdown(),
           this.docPluginManager?.shutdown(),
-          dataEngine?.shutdown()
+          this._isSnapshot ? undefined : dataEngine?.shutdown(),
         ]);
         // The this.waitForInitialization promise may not yet have resolved, but
         // should do so quickly now we've killed everything it depends on.
@@ -2280,21 +2285,28 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
     try {
       await this._tableMetadataLoader.wait();
       await this._tableMetadataLoader.clean();
-      await this._loadTables(docSession, pendingTableNames);
 
-      const tableStats = await this._pyCall('get_table_stats');
-      log.rawInfo("Loading complete, table statistics retrieved...", {
-        ...this.getLogMeta(docSession),
-        ...tableStats,
-        num_on_demand_tables: onDemandNames.length,
-      });
+      if (this._isSnapshot) {
+        log.rawInfo("Loading complete", {
+          ...this.getLogMeta(docSession),
+          num_on_demand_tables: onDemandNames.length,
+        });
+      } else {
+        await this._loadTables(docSession, pendingTableNames);
+        const tableStats = await this._pyCall('get_table_stats');
+        log.rawInfo("Loading complete, table statistics retrieved...", {
+          ...this.getLogMeta(docSession),
+          ...tableStats,
+          num_on_demand_tables: onDemandNames.length,
+        });
+        await this._pyCall('initialize', this._options?.docUrl);
 
-      await this._pyCall('initialize', this._options?.docUrl);
+        // Calculations are not associated specifically with the user opening the document.
+        // TODO: be careful with which users can create formulas.
+        await this._applyUserActions(makeExceptionalDocSession('system'), [['Calculate']]);
+        await this._reportDataEngineMemory();
+      }
 
-      // Calculations are not associated specifically with the user opening the document.
-      // TODO: be careful with which users can create formulas.
-      await this._applyUserActions(makeExceptionalDocSession('system'), [['Calculate']]);
-      await this._reportDataEngineMemory();
       this._fullyLoaded = true;
       const endTime = Date.now();
       const loadMs = endTime - startTime;
@@ -2553,7 +2565,15 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
    */
   private async _rawPyCall(funcName: string, ...varArgs: unknown[]): Promise<any> {
     const dataEngine = await this._getEngine();
-    return dataEngine.pyCall(funcName, ...varArgs);
+    try {
+      return await dataEngine.pyCall(funcName, ...varArgs);
+    } catch (e) {
+      if (e instanceof UnavailableSandboxMethodError && this._isSnapshot) {
+        throw new UnavailableSandboxMethodError('pyCall is not available in snapshots');
+      }
+
+      throw e;
+    }
   }
 
   /**
@@ -2567,13 +2587,13 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
 
   private async _getEngine(): Promise<ISandbox> {
     if (this._shuttingDown) { throw new Error('shutting down, data engine unavailable'); }
-    this._dataEngine = this._dataEngine || this._makeEngine();
+    if (this._dataEngine) { return this._dataEngine; }
+
+    this._dataEngine = this._isSnapshot ? this._makeNullEngine() : this._makeEngine();
     return this._dataEngine;
   }
 
   private async _makeEngine(): Promise<ISandbox> {
-    if (this._isSnapshot) { return new NullSandbox(); }
-
     // Figure out what kind of engine we need for this document.
     let preferredPythonVersion: '2' | '3' = process.env.PYTHON_VERSION === '3' ? '3' : '2';
 
@@ -2612,6 +2632,10 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
     });
   }
 
+  private async _makeNullEngine(): Promise<ISandbox> {
+    return new NullSandbox();
+  }
+
   /**
    * Throw an error if the provided upload would exceed the total attachment filesize limit for this document.
    */
@@ -2657,6 +2681,41 @@ export class ActiveDoc extends EventEmitter implements AssistanceDoc {
     }
     return this._attachmentColumns;
   }
+
+  /**
+   * Waits for the data engine to be ready before calling `cb`, creating the
+   * engine if needed.
+   *
+   * Optionally shuts down and removes the engine after.
+   *
+   * NOTE: This method should be used with care, particularly when `shutdownAfter`
+   * is set. Currently, it's only used to run migrations on snapshots (which don't
+   * normally start the data engine).
+   */
+  private async _withDataEngine(
+    cb: () => Promise<void>,
+    options: {shutdownAfter?: boolean} = {}
+  ) {
+    const {shutdownAfter} = options;
+    this._dataEngine = this._dataEngine || this._makeEngine();
+    let engine = await this._dataEngine;
+    if (engine instanceof NullSandbox) {
+      // Make sure the current engine isn't a stub, which may be the case when the
+      // document is a snapshot. Normally, `shutdownAfter` will be true in such
+      // scenarios, so we'll revert back to using a stubbed engine after calling `cb`.
+      this._dataEngine = this._makeEngine();
+      engine = await this._dataEngine;
+    }
+
+    try {
+      await cb();
+    } finally {
+      if (shutdownAfter) {
+        await (await this._dataEngine)?.shutdown();
+        this._dataEngine = null;
+      }
+    }
+  }
 }
 
 // Helper to initialize a sandbox action bundle with no values.

app/server/lib/NullSandbox.ts

@@ -1,15 +1,21 @@
 import {ISandbox} from 'app/server/lib/ISandbox';
 
+export class UnavailableSandboxMethodError extends Error {
+  constructor(message: string) {
+    super(message);
+  }
+}
+
 export class NullSandbox implements ISandbox {
   public async shutdown(): Promise<unknown> {
-    return undefined;
+    throw new UnavailableSandboxMethodError('shutdown is not available');
   }
 
   public async pyCall(_funcName: string, ..._varArgs: unknown[]) {
-    return undefined;
+    throw new UnavailableSandboxMethodError('pyCall is not available');
   }
 
   public async reportMemoryUsage() {
-    return undefined;
+    throw new UnavailableSandboxMethodError('reportMemoryUsage is not available');
   }
 }

test/server/lib/HostedStorageManager.ts

@@ -1,5 +1,6 @@
 import {ErrorOrValue, freezeError, mapGetOrSet, MapWithTTL} from 'app/common/AsyncCreate';
 import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot';
+import {SCHEMA_VERSION} from 'app/common/schema';
 import {DocWorkerMap} from 'app/gen-server/lib/DocWorkerMap';
 import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager';
 import {ActiveDoc} from 'app/server/lib/ActiveDoc';
@@ -817,6 +818,31 @@ describe('HostedStorageManager', function() {
         await store.end();
       });
 
+      it('can access snapshots with old schema versions', async function() {
+        const snapshotId = `World~v=1`;
+        await workers.assignDocWorker(snapshotId);
+        await store.begin();
+        // Pretend we have a snapshot of World-v33.grist and fetch/load it.
+        await useFixtureDoc('World-v33.grist', store.storageManager, `${snapshotId}.grist`);
+        const doc = await store.docManager.fetchDoc(docSession, snapshotId);
+
+        // Check that the snapshot isn't broken.
+        assert.doesNotThrow(async () => await doc.waitForInitialization());
+
+        // Check that the snapshot was migrated to the latest schema version.
+        assert.equal(
+          SCHEMA_VERSION,
+          (await doc.docStorage.get("select schemaVersion from _grist_DocInfo where id = 1"))!.schemaVersion
+        );
+
+        // Check that the document is actually a snapshot.
+        await assert.isRejected(doc.replace(docSession, {sourceDocId: 'docId'}),
+          /Snapshots cannot be replaced/);
+        await assert.isRejected(doc.applyUserActions(docSession, [['AddTable', 'NewTable', [{id: 'A'}]]]),
+          /pyCall is not available in snapshots/);
+        await store.end();
+      });
+
       it('can prune snapshots', async function() {
         const versions = 8;