From 215711238f54e61a7516e4d14c63bfec9db85789 Mon Sep 17 00:00:00 2001 From: Tom Marsh Date: Tue, 31 Mar 2020 18:11:04 +0100 Subject: [PATCH 1/5] Reuse Python handler processes for performance. --- .../python-runner/PythonRunner.js | 125 +++++++++--------- .../handler-runner/python-runner/invoke.py | 43 +++--- 2 files changed, 88 insertions(+), 80 deletions(-) diff --git a/src/lambda/handler-runner/python-runner/PythonRunner.js b/src/lambda/handler-runner/python-runner/PythonRunner.js index bb9eb3534..8b1c9e2e8 100644 --- a/src/lambda/handler-runner/python-runner/PythonRunner.js +++ b/src/lambda/handler-runner/python-runner/PythonRunner.js @@ -1,6 +1,7 @@ import { EOL, platform } from 'os' import { delimiter, join, relative, resolve } from 'path' -import execa from 'execa' +import { spawn } from 'child_process' +import extend from 'extend' const { parse, stringify } = JSON const { cwd } = process @@ -18,7 +19,36 @@ export default class PythonRunner { this.#env = env this.#handlerName = handlerName this.#handlerPath = handlerPath - this.#runtime = runtime + this.#runtime = platform() === 'win32' ? 'python.exe' : runtime + + if (process.env.VIRTUAL_ENV) { + const runtimeDir = platform() === 'win32' ? 'Scripts' : 'bin' + process.env.PATH = [ + join(process.env.VIRTUAL_ENV, runtimeDir), + delimiter, + process.env.PATH, + ].join('') + } + + const [pythonExecutable] = this.#runtime.split('.') + + this.handlerProcess = spawn( + pythonExecutable, + [ + '-u', + resolve(__dirname, 'invoke.py'), + relative(cwd(), this.#handlerPath), + this.#handlerName, + ], + { + env: extend(process.env, this.#env), + shell: true + }, + ) + + process.on('exit', () => { + this.handlerProcess.kill(); + }) } // no-op @@ -57,68 +87,43 @@ export default class PythonRunner { // invokeLocalPython, loosely based on: // https://github.com/serverless/serverless/blob/v1.50.0/lib/plugins/aws/invokeLocal/index.js#L410 - // invoke.py, copy/pasted entirely as is: + // invoke.py, based on: // https://github.com/serverless/serverless/blob/v1.50.0/lib/plugins/aws/invokeLocal/invoke.py async run(event, context) { - const runtime = platform() === 'win32' ? 'python.exe' : this.#runtime - - const input = stringify({ - context, - event, - }) - - if (process.env.VIRTUAL_ENV) { - const runtimeDir = platform() === 'win32' ? 'Scripts' : 'bin' - process.env.PATH = [ - join(process.env.VIRTUAL_ENV, runtimeDir), - delimiter, - process.env.PATH, - ].join('') - } - - const [pythonExecutable] = runtime.split('.') - - const python = execa( - pythonExecutable, - [ - '-u', - resolve(__dirname, 'invoke.py'), - relative(cwd(), this.#handlerPath), - this.#handlerName, - ], - { - env: this.#env, - input, - // shell: true, - }, - ) - - let result - - try { - result = await python - } catch (err) { - // TODO - console.log(err) - - throw err - } - - const { stderr, stdout } = result + return new Promise((accept, reject) => { + + const input = stringify({ + context, + event, + }) + + const onData = (data) => { + this.handlerProcess.stdout.removeListener('data', onData) + this.handlerProcess.stderr.removeListener('data', onErr) + + try { + return accept(this._parsePayload(data.toString())) + } catch (err) { + // TODO + console.log('No JSON') + + // TODO return or re-throw? + return reject(err) + } + } - if (stderr) { - // TODO - console.log(stderr) - } + const onErr = (data) => { + // TODO + console.log(data.toString()) + } - try { - return this._parsePayload(stdout) - } catch (err) { - // TODO - console.log('No JSON') + this.handlerProcess.stdout.on('data', onData) + this.handlerProcess.stderr.on('data', onErr) - // TODO return or re-throw? - return err - } + process.nextTick(() => { + this.handlerProcess.stdin.write(input); + this.handlerProcess.stdin.write('\n') + }) + }) } } diff --git a/src/lambda/handler-runner/python-runner/invoke.py b/src/lambda/handler-runner/python-runner/invoke.py index b8c78c6da..77819c230 100644 --- a/src/lambda/handler-runner/python-runner/invoke.py +++ b/src/lambda/handler-runner/python-runner/invoke.py @@ -75,23 +75,26 @@ def log(self): module = import_module(args.handler_path.replace('/', '.')) handler = getattr(module, args.handler_name) - input = json.load(sys.stdin) - if sys.platform != 'win32': - try: - if sys.platform != 'darwin': - subprocess.check_call('tty', stdout=subprocess.PIPE, stderr=subprocess.PIPE) - except (OSError, subprocess.CalledProcessError): - pass - else: - sys.stdin = open('/dev/tty') - - context = FakeLambdaContext(**input.get('context', {})) - result = handler(input['event'], context) - - data = { - # just an identifier to distinguish between - # interesting data (result) and stdout/print - '__offline_payload__': result - } - - sys.stdout.write(json.dumps(data)) + while True: + input = json.loads(sys.stdin.readline()) + + if sys.platform != 'win32': + try: + if sys.platform != 'darwin': + subprocess.check_call('tty', stdout=subprocess.PIPE, stderr=subprocess.PIPE) + except (OSError, subprocess.CalledProcessError): + pass + else: + sys.stdin = open('/dev/tty') + + context = FakeLambdaContext(**input.get('context', {})) + result = handler(input['event'], context) + + data = { + # just an identifier to distinguish between + # interesting data (result) and stdout/print + '__offline_payload__': result + } + + sys.stdout.write(json.dumps(data)) + sys.stdout.write('\n') From 64ad8376930b3598f52eb1cda69014c0572fe494 Mon Sep 17 00:00:00 2001 From: Tom Marsh Date: Thu, 2 Apr 2020 09:09:32 +0100 Subject: [PATCH 2/5] Fix linter issues. --- package-lock.json | 5 ++--- package.json | 1 + .../python-runner/PythonRunner.js | 17 ++++++++--------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index e0424888f..66de2c9c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "serverless-offline", - "version": "6.1.1", + "version": "6.1.3", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -4877,8 +4877,7 @@ "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", - "integrity": "sha512-fjquC59cD7CyW6urNXK0FBufkZcoiGG80wTuPujX590cB5Ttln20E2UB4S/WARVqhXffZl2LNgS+gQdPIIim/g==", - "dev": true + "integrity": "sha512-fjquC59cD7CyW6urNXK0FBufkZcoiGG80wTuPujX590cB5Ttln20E2UB4S/WARVqhXffZl2LNgS+gQdPIIim/g==" }, "extend-shallow": { "version": "3.0.2", diff --git a/package.json b/package.json index a678be020..df997952b 100644 --- a/package.json +++ b/package.json @@ -150,6 +150,7 @@ "chalk": "^3.0.0", "cuid": "^2.1.8", "execa": "^4.0.0", + "extend": "^3.0.2", "fs-extra": "^8.1.0", "js-string-escape": "^1.0.1", "jsonpath-plus": "^3.0.0", diff --git a/src/lambda/handler-runner/python-runner/PythonRunner.js b/src/lambda/handler-runner/python-runner/PythonRunner.js index 8b1c9e2e8..39f329d1c 100644 --- a/src/lambda/handler-runner/python-runner/PythonRunner.js +++ b/src/lambda/handler-runner/python-runner/PythonRunner.js @@ -42,12 +42,12 @@ export default class PythonRunner { ], { env: extend(process.env, this.#env), - shell: true + shell: true, }, ) process.on('exit', () => { - this.handlerProcess.kill(); + this.handlerProcess.kill() }) } @@ -91,12 +91,16 @@ export default class PythonRunner { // https://github.com/serverless/serverless/blob/v1.50.0/lib/plugins/aws/invokeLocal/invoke.py async run(event, context) { return new Promise((accept, reject) => { - const input = stringify({ context, event, }) + const onErr = (data) => { + // TODO + console.log(data.toString()) + } + const onData = (data) => { this.handlerProcess.stdout.removeListener('data', onData) this.handlerProcess.stderr.removeListener('data', onErr) @@ -112,16 +116,11 @@ export default class PythonRunner { } } - const onErr = (data) => { - // TODO - console.log(data.toString()) - } - this.handlerProcess.stdout.on('data', onData) this.handlerProcess.stderr.on('data', onErr) process.nextTick(() => { - this.handlerProcess.stdin.write(input); + this.handlerProcess.stdin.write(input) this.handlerProcess.stdin.write('\n') }) }) From c531811b9ff64556cbdeca6abdf1d2beec8c5045 Mon Sep 17 00:00:00 2001 From: Tom Marsh Date: Thu, 2 Apr 2020 16:06:22 +0100 Subject: [PATCH 3/5] Fix handling of handler process streams. --- .../python-runner/PythonRunner.js | 24 ++++++++++-------- .../handler-runner/python-runner/invoke.py | 25 +++++++++++-------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/lambda/handler-runner/python-runner/PythonRunner.js b/src/lambda/handler-runner/python-runner/PythonRunner.js index 39f329d1c..693e252ad 100644 --- a/src/lambda/handler-runner/python-runner/PythonRunner.js +++ b/src/lambda/handler-runner/python-runner/PythonRunner.js @@ -2,6 +2,7 @@ import { EOL, platform } from 'os' import { delimiter, join, relative, resolve } from 'path' import { spawn } from 'child_process' import extend from 'extend' +import readline from 'readline' const { parse, stringify } = JSON const { cwd } = process @@ -46,6 +47,10 @@ export default class PythonRunner { }, ) + this.handlerProcess.stdout.readline = readline.createInterface({ + input: this.handlerProcess.stdout, + }) + process.on('exit', () => { this.handlerProcess.kill() }) @@ -101,22 +106,21 @@ export default class PythonRunner { console.log(data.toString()) } - const onData = (data) => { - this.handlerProcess.stdout.removeListener('data', onData) - this.handlerProcess.stderr.removeListener('data', onErr) - + const onLine = (line) => { try { - return accept(this._parsePayload(data.toString())) + const parsed = this._parsePayload(line.toString()) + if (parsed) { + this.handlerProcess.stdout.readline.removeListener('line', onLine) + this.handlerProcess.stderr.removeListener('data', onErr) + return accept(parsed) + } + return null } catch (err) { - // TODO - console.log('No JSON') - - // TODO return or re-throw? return reject(err) } } - this.handlerProcess.stdout.on('data', onData) + this.handlerProcess.stdout.readline.on('line', onLine) this.handlerProcess.stderr.on('data', onErr) process.nextTick(() => { diff --git a/src/lambda/handler-runner/python-runner/invoke.py b/src/lambda/handler-runner/python-runner/invoke.py index 77819c230..e1ae9a7b5 100644 --- a/src/lambda/handler-runner/python-runner/invoke.py +++ b/src/lambda/handler-runner/python-runner/invoke.py @@ -75,17 +75,22 @@ def log(self): module = import_module(args.handler_path.replace('/', '.')) handler = getattr(module, args.handler_name) + # Keep a reference to the original stdin so that we can continue to receive + # input from the parent process. + stdin = sys.stdin + + if sys.platform != 'win32': + try: + if sys.platform != 'darwin': + subprocess.check_call('tty', stdout=subprocess.PIPE, stderr=subprocess.PIPE) + except (OSError, subprocess.CalledProcessError): + pass + else: + # Replace stdin with a TTY to enable pdb usage. + sys.stdin = open('/dev/tty') + while True: - input = json.loads(sys.stdin.readline()) - - if sys.platform != 'win32': - try: - if sys.platform != 'darwin': - subprocess.check_call('tty', stdout=subprocess.PIPE, stderr=subprocess.PIPE) - except (OSError, subprocess.CalledProcessError): - pass - else: - sys.stdin = open('/dev/tty') + input = json.loads(stdin.readline()) context = FakeLambdaContext(**input.get('context', {})) result = handler(input['event'], context) From 49e5121869aff40fe74f5a6b51f78f71b625b36c Mon Sep 17 00:00:00 2001 From: Tom Marsh Date: Wed, 8 Apr 2020 12:11:17 +0100 Subject: [PATCH 4/5] Properly mop up Python handler runners. --- src/lambda/handler-runner/python-runner/PythonRunner.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/lambda/handler-runner/python-runner/PythonRunner.js b/src/lambda/handler-runner/python-runner/PythonRunner.js index 693e252ad..f44583252 100644 --- a/src/lambda/handler-runner/python-runner/PythonRunner.js +++ b/src/lambda/handler-runner/python-runner/PythonRunner.js @@ -50,15 +50,12 @@ export default class PythonRunner { this.handlerProcess.stdout.readline = readline.createInterface({ input: this.handlerProcess.stdout, }) - - process.on('exit', () => { - this.handlerProcess.kill() - }) } - // no-op // () => void - cleanup() {} + cleanup() { + this.handlerProcess.kill() + } _parsePayload(value) { let payload From 2551156cda7eacdcb160b8d6115e5db4c4df2b46 Mon Sep 17 00:00:00 2001 From: Tom Marsh Date: Wed, 29 Apr 2020 09:26:12 +0100 Subject: [PATCH 5/5] Add command line option for function cleanup idle time. --- src/config/commandOptions.js | 3 +++ src/config/defaultOptions.js | 1 + src/lambda/LambdaFunctionPool.js | 9 ++++++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/config/commandOptions.js b/src/config/commandOptions.js index c54f27436..b93c0076e 100644 --- a/src/config/commandOptions.js +++ b/src/config/commandOptions.js @@ -72,4 +72,7 @@ export default { useDocker: { usage: 'Uses docker for node/python/ruby', }, + functionCleanupIdleTimeSeconds: { + usage: 'Number of seconds until an idle function is eligible for cleanup', + }, } diff --git a/src/config/defaultOptions.js b/src/config/defaultOptions.js index 0b0b051ea..20cc05b73 100644 --- a/src/config/defaultOptions.js +++ b/src/config/defaultOptions.js @@ -22,4 +22,5 @@ export default { useWorkerThreads: false, websocketPort: 3001, useDocker: false, + functionCleanupIdleTimeSeconds: 60, } diff --git a/src/lambda/LambdaFunctionPool.js b/src/lambda/LambdaFunctionPool.js index b78675784..b91037ec5 100644 --- a/src/lambda/LambdaFunctionPool.js +++ b/src/lambda/LambdaFunctionPool.js @@ -24,8 +24,11 @@ export default class LambdaFunctionPool { const { idleTimeInMinutes, status } = lambdaFunction // console.log(idleTimeInMinutes, status) - // 45 // TODO config, or maybe option? - if (status === 'IDLE' && idleTimeInMinutes >= 1) { + if ( + status === 'IDLE' && + idleTimeInMinutes >= + this.#options.functionCleanupIdleTimeSeconds / 60 + ) { // console.log(`removed Lambda Function ${lambdaFunction.functionName}`) lambdaFunction.cleanup() lambdaFunctions.delete(lambdaFunction) @@ -35,7 +38,7 @@ export default class LambdaFunctionPool { // schedule new timer this._startCleanTimer() - }, 10000) // TODO: config, or maybe option? + }, (this.#options.functionCleanupIdleTimeSeconds * 1000) / 2) } _cleanupPool() {