(core) When parsing ActionHistory into ActionGroups, avoid keeping many large actions in memory.

Summary:
Add a unittest that start the server with limited memory, and build just enough
of ActionHistory to crash the server before this fix, and not after.

Test Plan: Tested manually with various memory prints, and added a test.

Reviewers: paulfitz

Reviewed By: paulfitz

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

app/server/lib/ActionHistory.ts

@@ -21,6 +21,18 @@ import {DocState} from 'app/common/UserAPI';
 import toPairs = require('lodash/toPairs');
 import {summarizeAction} from './ActionSummary';
 
+export interface ActionGroupOptions {
+  // If set, inspect the action in detail in order to include a summary of
+  // changes made within the action.  Otherwise, the actionSummary returned is empty.
+  summarize?: boolean;
+
+  // The client for which the action group is being prepared, if known.
+  client?: {clientId: string}|null;
+
+  // Values returned by the action, if known.
+  retValues?: any[];
+}
+
 export abstract class ActionHistory {
   /**
    * Initialize the ActionLog by reading the database. No other methods may be used until the
@@ -85,9 +97,18 @@ export abstract class ActionHistory {
    * Get the most recent actions from the history.  Results are ordered by
    * earliest actions first, later actions later.  If `maxActions` is supplied,
    * at most that number of actions are returned.
+   *
+   * This method should be avoid in production, since it may convert and keep in memory many large
+   * actions. (It has in the past led to exhausting memory and crashing node.)
    */
   public abstract getRecentActions(maxActions?: number): Promise<LocalActionBundle[]>;
 
+  /**
+   * Same as getRecentActions, but converts each to an ActionGroup using asActionGroup with the
+   * supplied options.
+   */
+  public abstract getRecentActionGroups(maxActions: number, options: ActionGroupOptions): Promise<ActionGroup[]>;
+
   /**
    * Get the most recent states from the history.  States are just
    * actions without any content.  Results are ordered by most recent
@@ -194,18 +215,11 @@ export function humanDescription(actions: UserAction[]): string {
  * actions on the client.
  * @param history: interface to action history
  * @param act: action to convert
- * @param options.summarize: if set, inspect the action in detail in order to include a summary of
- *   changes made within the action.  Otherwise, the actionSummary returned is empty.
- * @param options.client: the client for which the action group is being prepared, if known.
- * @param options.retValues: values returned by the action, if known.
+ * @param options: options to construct the ActionGroup; see its documentation above.
  */
 export function asActionGroup(history: ActionHistory,
                               act: LocalActionBundle,
-                              options: {
-                                summarize?: boolean
-                                client?: {clientId: string}|null,
-                                retValues?: any[],
-                              }): ActionGroup {
+                              options: ActionGroupOptions): ActionGroup {
   const {summarize, client, retValues} = options;
   const info = act.info[1];
   const fromSelf = (client && client.clientId && act.actionHash) ?

app/server/lib/ActionHistoryImpl.ts

@@ -2,12 +2,13 @@
  * Minimal ActionHistory implementation
  */
 import {LocalActionBundle} from 'app/common/ActionBundle';
+import {ActionGroup} from 'app/common/ActionGroup';
 import * as marshaller from 'app/common/marshal';
 import {DocState} from 'app/common/UserAPI';
 import * as crypto from 'crypto';
 import keyBy = require('lodash/keyBy');
 import mapValues = require('lodash/mapValues');
-import {ActionHistory} from './ActionHistory';
+import {ActionGroupOptions, ActionHistory, asActionGroup} from './ActionHistory';
 import {ISQLiteDB, ResultRow} from './SQLiteDB';
 
 // History will from time to time be pruned back to within these limits
@@ -370,16 +371,13 @@ export class ActionHistoryImpl implements ActionHistory {
   }
 
   public async getRecentActions(maxActions?: number): Promise<LocalActionBundle[]> {
-    const branches = await this._getBranches();
-    const actions = await this._fetchParts(null,
-                                           branches.local_unsent,
-                                           "_gristsys_ActionHistory.id, actionNum, actionHash, body",
-                                           maxActions,
-                                           true);
-    const result = actions.map(decodeActionFromRow);
-    result.reverse();  // Implementation note: this could be optimized away when `maxActions`
-                       // is not specified, by simply asking _fetchParts for ascending order.
-    return result;
+    const actions = await this._getRecentActionRows(maxActions);
+    return actions.map(decodeActionFromRow);
+  }
+
+  public async getRecentActionGroups(maxActions: number, options: ActionGroupOptions): Promise<ActionGroup[]> {
+    const actions = await this._getRecentActionRows(maxActions);
+    return actions.map(row => asActionGroup(this, decodeActionFromRow(row), options));
   }
 
   public async getRecentStates(maxStates?: number): Promise<DocState[]> {
@@ -438,6 +436,22 @@ export class ActionHistoryImpl implements ActionHistory {
     return this._actionClient.get(actionHash);
   }
 
+  /**
+   * Fetches the most recent action row from the history, orderd with earlier actions first.
+   * If `maxActions` is supplied, at most that number of actions are returned.
+   */
+  private async _getRecentActionRows(maxActions?: number): Promise<ResultRow[]> {
+    const branches = await this._getBranches();
+    const result = await this._fetchParts(null,
+                                           branches.local_unsent,
+                                           "_gristsys_ActionHistory.id, actionNum, actionHash, body",
+                                           maxActions,
+                                           true);
+    result.reverse();  // Implementation note: this could be optimized away when `maxActions`
+                       // is not specified, by simply asking _fetchParts for ascending order.
+    return result;
+  }
+
   /** Check if we need to update the next shared actionNum */
   private _noteSharedAction(actionNum: number): void {
     if (actionNum >= this._sharedActionNum) {

app/server/lib/ActiveDoc.ts

@@ -46,7 +46,7 @@ import * as log from 'app/server/lib/log';
 import {shortDesc} from 'app/server/lib/shortDesc';
 import {fetchURL, FileUploadInfo, globalUploadSet, UploadInfo} from 'app/server/lib/uploads';
 
-import {ActionHistory, asActionGroup} from './ActionHistory';
+import {ActionHistory} from './ActionHistory';
 import {ActionHistoryImpl} from './ActionHistoryImpl';
 import {ActiveDocImport} from './ActiveDocImport';
 import {DocClients} from './DocClients';
@@ -174,6 +174,8 @@ export class ActiveDoc extends EventEmitter {
     return this._muted;
   }
 
+  // Note that this method is only used in tests, and should be avoided in production (see note
+  // in ActionHistory about getRecentActions).
   public getRecentActionsDirect(maxActions?: number): Promise<LocalActionBundle[]> {
     return this._actionHistory.getRecentActions(maxActions);
   }
@@ -199,8 +201,8 @@ export class ActiveDoc extends EventEmitter {
    * action summaries are computed and included.
    */
   public async getRecentActions(docSession: OptDocSession, summarize: boolean): Promise<ActionGroup[]> {
-    const actions = await this._actionHistory.getRecentActions(MAX_RECENT_ACTIONS);
-    const groups = actions.map(act => asActionGroup(this._actionHistory, act, {client: docSession.client, summarize}));
+    const groups = await this._actionHistory.getRecentActionGroups(MAX_RECENT_ACTIONS,
+      {client: docSession.client, summarize});
     return groups.filter(actionGroup => this._granularAccess.allowActionGroup(docSession, actionGroup));
   }