Skip to content

Commit

Permalink
util: convert aggregate from js to ts and tweak execute-code
Browse files Browse the repository at this point in the history
  • Loading branch information
haraldschilly committed Jul 12, 2024
1 parent 72b6ecf commit 4a7cdc0
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 20 deletions.
37 changes: 30 additions & 7 deletions src/packages/backend/execute-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
// Execute code in a subprocess.

import { callback } from "awaiting";
import { spawn } from "node:child_process";
import LRU from "lru-cache";
import {
ChildProcessWithoutNullStreams,
spawn,
SpawnOptionsWithoutStdio,
} from "node:child_process";
import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
Expand All @@ -27,6 +32,15 @@ import type {

const log = getLogger("execute-code");

const asyncCache = new LRU<string, ExecuteCodeOutput>({
max: 100,
ttl: 1000 * 60 * 60,
ttlAutopurge: true,
allowStale: true,
updateAgeOnGet: true,
updateAgeOnHas: true,
});

// Async/await interface to executing code.
export async function executeCode(
opts: ExecuteCodeOptions,
Expand All @@ -52,6 +66,13 @@ export const execute_code: ExecuteCodeFunctionWithCallback = aggregate(
async function executeCodeNoAggregate(
opts: ExecuteCodeOptions,
): Promise<ExecuteCodeOutput> {
if (typeof opts.async_get === "string") {
const s = asyncCache.get(opts.async_get);
if (s != null) {
return s;
}
}

if (opts.args == null) opts.args = [];
if (opts.timeout == null) opts.timeout = 10;
if (opts.ulimit_timeout == null) opts.ulimit_timeout = true;
Expand Down Expand Up @@ -138,7 +159,7 @@ function doSpawn(opts, cb) {
"seconds",
);
}
const spawnOptions = {
const spawnOptions: SpawnOptionsWithoutStdio = {
detached: true, // so we can kill the entire process group if it times out
cwd: opts.path,
...(opts.uid ? { uid: opts.uid } : undefined),
Expand All @@ -150,11 +171,11 @@ function doSpawn(opts, cb) {
},
};

let r,
ran_code = false;
let r: ChildProcessWithoutNullStreams;
let ran_code = false;
try {
r = spawn(opts.command, opts.args, spawnOptions);
if (r.stdout == null || r.stderr == null) {
if (r.stdout == null || r.stderr == null || r.pid == null) {
// The docs/examples at https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options
// suggest that r.stdout and r.stderr are always defined. However, this is
// definitely NOT the case in edge cases, as we have observed.
Expand Down Expand Up @@ -215,7 +236,7 @@ function doSpawn(opts, cb) {
});

r.on("exit", (code) => {
exit_code = code;
exit_code = code != null ? code : undefined;
finish();
});

Expand Down Expand Up @@ -318,7 +339,9 @@ function doSpawn(opts, cb) {
}
try {
killed = true;
process.kill(-r.pid, "SIGKILL"); // this should kill process group
if (r.pid != null) {
process.kill(-r.pid, "SIGKILL"); // this should kill process group
}
} catch (err) {
// Exceptions can happen, which left uncaught messes up calling code big time.
if (opts.verbose) {
Expand Down
19 changes: 13 additions & 6 deletions src/packages/next/lib/api/schema/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ export const ExecInputSchema = z
.object({
project_id: ProjectIdSchema,
compute_server_id: ComputeServerIdSchema.describe(
`If provided, the desired shell command will be run on the compute server whose id
`If provided, the desired shell command will be run on the compute server whose id
is specified in this field (if available).`,
).optional(),
filesystem: z
.boolean()
.optional()
.describe(
`If \`true\`, this shell command runs in the fileserver container on the compute
`If \`true\`, this shell command runs in the fileserver container on the compute
server; otherwise, it runs on the main compute container.`,
),
path: z
Expand Down Expand Up @@ -46,7 +46,7 @@ export const ExecInputSchema = z
.boolean()
.optional()
.describe(
`If \`true\`, this command runs in a \`bash\` shell. To do so, the provided shell
`If \`true\`, this command runs in a \`bash\` shell. To do so, the provided shell
command is written to a file and then executed via the \`bash\` command.`,
),
aggregate: z
Expand All @@ -57,16 +57,16 @@ export const ExecInputSchema = z
])
.optional()
.describe(
`If provided, this shell command is aggregated as in
`If provided, this shell command is aggregated as in
\`src/packages/backend/aggregate.js\`. This parameter allows one to specify
multiple callbacks to be executed against the output of the same command
multiple callbacks to be executed against the output of the same command
(given identical arguments) within a 60-second window.`,
),
err_on_exit: z
.boolean()
.optional()
.describe(
`When \`true\`, this call will throw an error whenever the provided shell command
`When \`true\`, this call will throw an error whenever the provided shell command
exits with a non-zero exit code.`,
),
env: z
Expand All @@ -75,6 +75,13 @@ export const ExecInputSchema = z
.describe(
"Environment variables to be passed to the shell command upon execution.",
),
async_exec: z.boolean().optional()
.describe(`If \`true\`, the execution happens asynchroneously.
This means it the API call does not block and returns an ID.
Later, use that ID in a call to \`async_get\` to eventually get the result`),
async_get: z.string().optional()
.describe(`For a given ID returned by \`async\`,
retun the status, or the result as if it is called synchroneously. Results are only cached temporarily!`),
})
.describe("Perform arbitrary shell commands in a compute server or project.");

Expand Down
4 changes: 4 additions & 0 deletions src/packages/next/pages/api/v2/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ async function get(req) {
aggregate,
err_on_exit,
env,
async_exec,
async_get,
} = getParams(req);

if (!(await isCollaborator({ account_id, project_id }))) {
Expand All @@ -61,6 +63,8 @@ async function get(req) {
aggregate,
err_on_exit,
env,
async_exec,
async_get,
},
});
// event and id don't make sense for http post api
Expand Down
1 change: 0 additions & 1 deletion src/packages/util/aggregate.d.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,17 @@ Where options is an object.
*/

const { copy_without, field_cmp } = require("./misc");
const json_stable = require("json-stable-stringify");

import { copy_without, field_cmp } from "./misc";

// To avoid using up too much memory, results are cached at most this long
// (so long as function is called periodically to clear the cache... if not,
// no point in clearing, since won't grow much.)
const DONE_CACHE_TIMEOUT_MS = 60000;

function clear_old(done) {
const now = new Date();
const now = Date.now();
for (let key in done) {
const s = done[key];
if (now - s.time >= DONE_CACHE_TIMEOUT_MS) {
Expand All @@ -93,7 +94,7 @@ function leq(a, b) {
return a <= b;
}

exports.aggregate = function (options, f) {
export function aggregate(options, f?: any) {
if (f == null) {
f = options;
options = undefined;
Expand All @@ -120,7 +121,7 @@ exports.aggregate = function (options, f) {

function aggregate_call_f(opts) {
// Key is a string that determines the inputs to f **that matter**.
const key = json_stable(copy_without(opts, omitted_fields));
const key: string = json_stable(copy_without(opts, omitted_fields));
// Check state
const current = state[key];
const recent = done[key];
Expand Down Expand Up @@ -190,4 +191,4 @@ exports.aggregate = function (options, f) {
aggregate_call_f(opts);
}
};
};
}
4 changes: 3 additions & 1 deletion src/packages/util/types/execute-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ export interface ExecuteCodeOptions {
env?: object; // if given, added to exec environment
aggregate?: string | number; // if given, aggregates multiple calls with same sequence number into one -- see @cocalc/util/aggregate; typically make this a timestamp for compiling code (e.g., latex).
verbose?: boolean; // default true -- impacts amount of logging
async_exec?: boolean; // default false -- if true, return an ID and execute it asynchroneously
async_get?: string; // if set, everything else is ignored and the status/output of the async call is returned
}

export interface ExecuteCodeOptionsWithCallback extends ExecuteCodeOptions {
cb?: (err: undefined | Error, output?: ExecuteCodeOutput) => void;
}

export type ExecuteCodeFunctionWithCallback = (
opts: ExecuteCodeOptionsWithCallback
opts: ExecuteCodeOptionsWithCallback,
) => void;

0 comments on commit 4a7cdc0

Please sign in to comment.