From 8da89b0a3d26ba5a53f776cf5092cb088f7a871e Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Sat, 17 Aug 2024 22:03:11 -0400 Subject: [PATCH] (core) Improve behavior to update current time, to allow inactive docs to shut down, and reduce spurious errors. Summary: - When sandbox is down, report failing UpdateCurrentTime calls as warnings instead of errors. - When applying system actions (such as updating current time), don't treat them as user activity for the purpose of keeping the doc open. Test Plan: Added a test case for the fixed behavior. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D4324 --- app/server/lib/ActiveDoc.ts | 33 ++++++++++++++++++++++++++++----- app/server/lib/ISandbox.ts | 1 + app/server/lib/NSandbox.ts | 4 ++++ app/server/lib/NullSandbox.ts | 2 ++ 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 14df8946..5a0e5648 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -303,7 +303,7 @@ export class ActiveDoc extends EventEmitter { ), // Update the time in formulas every hour. new Interval( - () => this._applyUserActions(makeExceptionalDocSession('system'), [['UpdateCurrentTime']]), + () => this._updateCurrentTime(), Deps.UPDATE_CURRENT_TIME_DELAY, {onError: (e) => this._log.error(null, 'failed to update current time', e)}, ), @@ -1696,7 +1696,7 @@ export class ActiveDoc extends EventEmitter { const timeDeleted = changes.map(r => r.used ? null : now); const action: BulkUpdateRecord = ["BulkUpdateRecord", "_grist_Attachments", rowIds, {timeDeleted}]; // Don't use applyUserActions which may block the update action in delete-only mode - await this._applyUserActions(makeExceptionalDocSession('system'), [action]); + await this._applyUserActionsAsSystem([action]); return true; } @@ -2011,6 +2011,16 @@ export class ActiveDoc extends EventEmitter { return [tableNames, onDemandNames]; } + /** + * Applies an array of user actions initiated by Grist itself, using a DocSession with "system" + * access rights. These bypass access rules. + * + * They also do not count as "user activity" for the purpose of keeping the document open. + */ + protected async _applyUserActionsAsSystem(actions: UserAction[]): Promise { + return this._applyUserActions(makeExceptionalDocSession('system'), actions, {}); + } + /** * Applies an array of user actions to the sandbox and broadcasts the results to doc's clients. * @@ -2028,14 +2038,12 @@ export class ActiveDoc extends EventEmitter { * isModification: true if document was changed by one or more actions. * } */ - @ActiveDoc.keepDocOpen protected async _applyUserActions(docSession: OptDocSession, actions: UserAction[], options: ApplyUAExtendedOptions = {}): Promise { const client = docSession.client; this._log.debug(docSession, "_applyUserActions(%s, %s)%s", client, shortDesc(actions), options.parseStrings ? ' (will parse)' : ''); - this._inactivityTimer.ping(); // The doc is in active use; ping it to stay open longer. if (options.parseStrings) { actions = actions.map(ua => parseUserAction(ua, this.docData!)); @@ -2159,6 +2167,7 @@ export class ActiveDoc extends EventEmitter { this._log.debug(docSession, "shutdown complete"); } + @ActiveDoc.keepDocOpen private async _applyUserActionsWithExtendedOptions(docSession: OptDocSession, actions: UserAction[], options?: ApplyUAExtendedOptions): Promise { assert(Array.isArray(actions), "`actions` parameter should be an array."); @@ -2222,6 +2231,20 @@ export class ActiveDoc extends EventEmitter { }; } + /** + * Update the time in formulas; this is called via Interval every hour. + */ + private async _updateCurrentTime() { + const dataEngine = await this._getEngine(); + if (dataEngine.isProcessDown()) { + // Don't attempt to update time if data engine is down, as this can't help, and leads to + // spurious errors. Instead, report as a warning, more clearly and concisely. + this._log.warn(null, 'failed to update current time: data engine is down'); + return; + } + return this._applyUserActionsAsSystem([['UpdateCurrentTime']]); + } + /** * Applies all metrics from `usage` to the current document usage state. * @@ -2462,7 +2485,7 @@ export class ActiveDoc extends EventEmitter { // 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._applyUserActionsAsSystem([['Calculate']]); await this._reportDataEngineMemory(); } diff --git a/app/server/lib/ISandbox.ts b/app/server/lib/ISandbox.ts index 89d09fd8..87553055 100644 --- a/app/server/lib/ISandbox.ts +++ b/app/server/lib/ISandbox.ts @@ -27,6 +27,7 @@ export interface ISandbox { pyCall(funcName: string, ...varArgs: unknown[]): Promise; reportMemoryUsage(): Promise; getFlavor(): string; + isProcessDown(): boolean; } export interface ISandboxCreator { diff --git a/app/server/lib/NSandbox.ts b/app/server/lib/NSandbox.ts index 86a1ded6..5e47a22b 100644 --- a/app/server/lib/NSandbox.ts +++ b/app/server/lib/NSandbox.ts @@ -230,6 +230,10 @@ export class NSandbox implements ISandbox { log.rawDebug('Sandbox memory', {memory, ...this._logMeta}); } + public isProcessDown() { + return this._isReadClosed || this._isWriteClosed; + } + public getFlavor() { return this._logMeta.flavor; } diff --git a/app/server/lib/NullSandbox.ts b/app/server/lib/NullSandbox.ts index 96045972..bed7cc69 100644 --- a/app/server/lib/NullSandbox.ts +++ b/app/server/lib/NullSandbox.ts @@ -22,4 +22,6 @@ export class NullSandbox implements ISandbox { public getFlavor() { return 'null'; } + + public isProcessDown() { return true; } }