Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to power-tune only cold starts #206

Merged
merged 40 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
efd6ad0
Another implementation of onlyColdStarts
mriccia May 10, 2023
89a255b
Working implementation
mriccia May 11, 2023
711e37d
Addressing PR feedback
mriccia May 19, 2023
4720586
add permission required for `functionActiveV2`
mriccia May 24, 2023
f70677d
add comments and tidy up
mriccia May 24, 2023
0445f32
tidy up
mriccia May 24, 2023
4d8f554
Fix tests and add more tests
mriccia May 24, 2023
f6d7ad1
Modify the Terraform template to match the changes in the SAM template
mriccia May 24, 2023
32f9533
preserve env vars when optimising
mriccia May 24, 2023
c0dd5ff
remove redundant logic
mriccia May 24, 2023
dd14172
Merge branch 'master' into onlycoldstarts
alexcasalboni Mar 27, 2024
c0192ec
Merge remote-tracking branch 'origin/master' into onlycoldstarts
mriccia May 8, 2024
5a2e515
reintroduce changes that were removed in the merge
mriccia May 8, 2024
93802fd
PR feedback
mriccia May 8, 2024
aa85838
PR feedback
mriccia May 8, 2024
1656b7e
PR feedback
mriccia May 8, 2024
79bb3b8
fix tests
mriccia May 8, 2024
3c4b5d6
fix cleaner.js
mriccia May 9, 2024
69edf22
remove singleton for client to avoid breaking existing use cases
mriccia May 15, 2024
6b9c85d
change logic to use Description instead of Env Vars
mriccia May 15, 2024
331de58
PR Feedback
mriccia May 16, 2024
aad097c
PR Feedback
mriccia May 16, 2024
2a547a6
Fix unit tests and linting
mriccia May 16, 2024
4b58451
remove unnecessary permission
mriccia May 16, 2024
9a9b243
remove unnecessary permission
mriccia May 16, 2024
1979fb6
fix node version in the Terraform template
mriccia May 16, 2024
446fff2
remove error catch from initializer state
alexcasalboni May 20, 2024
e8b7d17
Uupdate analyzer and executor to include init stats and new state mac…
alexcasalboni May 20, 2024
71aa0cb
update tests and coverage
alexcasalboni May 20, 2024
40c221b
linting
alexcasalboni May 20, 2024
6f001da
Improve publisher coverage & linting
alexcasalboni May 20, 2024
940ecd8
Update documentation for onlyColdStarts
alexcasalboni May 20, 2024
000cd74
PR feedback
mriccia May 23, 2024
797f89c
Remove getAlias permission from the Initializer
mriccia May 23, 2024
355ea6f
PR Feedback
mriccia May 23, 2024
18a8a65
Modify logic for computing duration and cost
mriccia May 24, 2024
aba4d63
Remove tests for deleted code
mriccia May 24, 2024
103786c
Add few TODOs as reminders
mriccia May 24, 2024
38f5c96
Add tests for extractDuration
mriccia May 27, 2024
d837b53
Add more tests
mriccia May 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions lambda/cleaner.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ const utils = require('./utils');
*/
module.exports.handler = async(event, context) => {

const {lambdaARN, powerValues} = event;
const {
lambdaARN,
aliases,
} = extractDataFromInput(event);

validateInput(lambdaARN, powerValues); // may throw
validateInput(lambdaARN, aliases); // may throw

const ops = powerValues.map(async(value) => {
const alias = 'RAM' + value;
await cleanup(lambdaARN, alias); // may throw
const ops = aliases.map(async(alias) => {
await cleanup(lambdaARN, alias);
});

// run everything in parallel and wait until completed
Expand All @@ -22,12 +24,19 @@ module.exports.handler = async(event, context) => {
return 'OK';
};

const validateInput = (lambdaARN, powerValues) => {
const extractDataFromInput = (event) => {
return {
lambdaARN: event.lambdaARN,
aliases: event.powerValues.aliases,
};
};

const validateInput = (lambdaARN, aliases) => {
if (!lambdaARN) {
throw new Error('Missing or empty lambdaARN');
}
if (!powerValues || !powerValues.length) {
throw new Error('Missing or empty power values');
if (!aliases || !aliases.length) {
throw new Error('Missing or empty alias values');
}
};

Expand Down
33 changes: 25 additions & 8 deletions lambda/executor.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports.handler = async(event, context) => {
preProcessorARN,
postProcessorARN,
discardTopBottom,
onlyColdStarts,
sleepBetweenRunsMs,
} = await extractDataFromInput(event);

Expand All @@ -40,8 +41,9 @@ module.exports.handler = async(event, context) => {
const lambdaAlias = 'RAM' + value;
let results;

// fetch architecture from $LATEST
const {architecture, isPending} = await utils.getLambdaConfig(lambdaARN, lambdaAlias);
let aliasToInvoke = utils.buildAliasString(lambdaAlias, onlyColdStarts, 0);
const {architecture, isPending} = await utils.getLambdaConfig(lambdaARN, aliasToInvoke);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at what the Initializer is doing (very similarly), I would consider merging utils.getLambdaPower and utils.getLambdaConfig since they are both using GetFunctionConfigurationCommand and they're simply retrieving different fields from the result.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking for this PR - just a reminder for myself in the future :)


console.log(`Detected architecture type: ${architecture}, isPending: ${isPending}`);

// pre-generate an array of N payloads
Expand All @@ -54,11 +56,12 @@ module.exports.handler = async(event, context) => {
payloads: payloads,
preARN: preProcessorARN,
postARN: postProcessorARN,
onlyColdStarts: onlyColdStarts,
sleepBetweenRunsMs: sleepBetweenRunsMs,
};

// wait if the function/alias state is Pending
if (isPending) {
if (isPending && !onlyColdStarts) {
await utils.waitForAliasActive(lambdaARN, lambdaAlias);
console.log('Alias active');
}
Expand Down Expand Up @@ -101,7 +104,11 @@ const extractDiscardTopBottomValue = (event) => {
// extract discardTopBottom used to trim values from average duration
let discardTopBottom = event.discardTopBottom;
if (typeof discardTopBottom === 'undefined') {
discardTopBottom = 0.2;
if (event.onlyColdStarts){
discardTopBottom = 0;
} else {
discardTopBottom = 0.2;
}
}
// discardTopBottom must be between 0 and 0.4
return Math.min(Math.max(discardTopBottom, 0.0), 0.4);
Expand Down Expand Up @@ -132,15 +139,21 @@ const extractDataFromInput = async(event) => {
preProcessorARN: input.preProcessorARN,
postProcessorARN: input.postProcessorARN,
discardTopBottom: discardTopBottom,
onlyColdStarts: !!input.onlyColdStarts,
sleepBetweenRunsMs: sleepBetweenRunsMs,
};
};

const runInParallel = async({num, lambdaARN, lambdaAlias, payloads, preARN, postARN}) => {
const runInParallel = async({num, lambdaARN, lambdaAlias, payloads, preARN, postARN, onlyColdStarts}) => {
const results = [];
// run all invocations in parallel ...
const invocations = utils.range(num).map(async(_, i) => {
const {invocationResults, actualPayload} = await utils.invokeLambdaWithProcessors(lambdaARN, lambdaAlias, payloads[i], preARN, postARN);
let aliasToInvoke = utils.buildAliasString(lambdaAlias, onlyColdStarts, i);
if (onlyColdStarts){
await utils.waitForAliasActive(lambdaARN, aliasToInvoke);
console.log(`${aliasToInvoke} is active`);
}
const {invocationResults, actualPayload} = await utils.invokeLambdaWithProcessors(lambdaARN, aliasToInvoke, payloads[i], preARN, postARN);
// invocation errors return 200 and contain FunctionError and Payload
if (invocationResults.FunctionError) {
throw new Error(`Invocation error (running in parallel): ${invocationResults.Payload} with payload ${JSON.stringify(actualPayload)}`);
Expand All @@ -152,11 +165,15 @@ const runInParallel = async({num, lambdaARN, lambdaAlias, payloads, preARN, post
return results;
};

const runInSeries = async({num, lambdaARN, lambdaAlias, payloads, preARN, postARN, sleepBetweenRunsMs}) => {
const runInSeries = async({num, lambdaARN, lambdaAlias, payloads, preARN, postARN, onlyColdStarts, sleepBetweenRunsMs }) => {
const results = [];
for (let i = 0; i < num; i++) {
let aliasToInvoke = utils.buildAliasString(lambdaAlias, onlyColdStarts, i);
// run invocations in series
const {invocationResults, actualPayload} = await utils.invokeLambdaWithProcessors(lambdaARN, lambdaAlias, payloads[i], preARN, postARN);
if (onlyColdStarts){
await utils.waitForAliasActive(lambdaARN, aliasToInvoke);
}
const {invocationResults, actualPayload} = await utils.invokeLambdaWithProcessors(lambdaARN, aliasToInvoke, payloads[i], preARN, postARN);
// invocation errors return 200 and contain FunctionError and Payload
if (invocationResults.FunctionError) {
throw new Error(`Invocation error (running in series): ${invocationResults.Payload} with payload ${JSON.stringify(actualPayload)}`);
Expand Down
50 changes: 42 additions & 8 deletions lambda/initializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,58 @@ const defaultPowerValues = process.env.defaultPowerValues.split(',');
*/
module.exports.handler = async(event, context) => {

const {lambdaARN, num} = event;
const powerValues = extractPowerValues(event);
const {
lambdaARN,
num,
powerValues,
onlyColdStarts,
} = extractDataFromInput(event);

validateInput(lambdaARN, num); // may throw

// fetch initial $LATEST value so we can reset it later
const initialPower = await utils.getLambdaPower(lambdaARN);
const {power} = await utils.getLambdaPower(lambdaARN);

let lambdaFunctionsToSet = [];

// reminder: configuration updates must run sequentially
// (otherwise you get a ResourceConflictException)
for (let value of powerValues){
const alias = 'RAM' + value;
await utils.createPowerConfiguration(lambdaARN, value, alias);
for (let powerValue of powerValues){
const baseAlias = 'RAM' + powerValue;
if (!onlyColdStarts){
lambdaFunctionsToSet.push({powerValue: powerValue, alias: baseAlias});
mriccia marked this conversation as resolved.
Show resolved Hide resolved
} else {
for (let n of utils.range(num)){
let alias = utils.buildAliasString(baseAlias, onlyColdStarts, n);
// here we inject a custom env variable to force the creation of a new version
mriccia marked this conversation as resolved.
Show resolved Hide resolved
// even if the power is the same, which will force a cold start
lambdaFunctionsToSet.push({powerValue: powerValue, alias: alias});
}
}
}
// Publish another version to revert the Lambda Function to its original configuration
lambdaFunctionsToSet.push({powerValue: power});

const returnObj = {
initConfigurations: lambdaFunctionsToSet,
iterator: {
index: 0,
count: lambdaFunctionsToSet.length,
continue: true,
},
powerValues: powerValues,
};
return returnObj;
};

await utils.setLambdaPower(lambdaARN, initialPower);

return powerValues;
const extractDataFromInput = (event) => {
return {
lambdaARN: event.lambdaARN,
num: parseInt(event.num, 10),
powerValues: extractPowerValues(event),
onlyColdStarts: !!event.onlyColdStarts,
};
};

const extractPowerValues = (event) => {
Expand Down
3 changes: 2 additions & 1 deletion lambda/optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ module.exports.handler = async(event, context) => {
await utils.setLambdaPower(lambdaARN, optimalValue);
} else {
// create/update alias
await utils.createPowerConfiguration(lambdaARN, optimalValue, autoOptimizeAlias);
const {envVars} = await utils.getLambdaPower(lambdaARN);
await utils.createPowerConfiguration(lambdaARN, optimalValue, autoOptimizeAlias, envVars);
}

return 'OK';
Expand Down
52 changes: 52 additions & 0 deletions lambda/publisher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';

const utils = require('./utils');


module.exports.handler = async(event, context) => {
const {iterator, aliases, currConfig, lambdaARN} = validateInputs(event);
const {envVars} = await utils.getLambdaPower(lambdaARN);
// Alias may not exist when we are reverting the Lambda function to its original configuration
if (typeof currConfig.alias !== 'undefined'){
envVars.LambdaPowerTuningForceColdStart = currConfig.alias;
mriccia marked this conversation as resolved.
Show resolved Hide resolved
} else {
delete envVars.LambdaPowerTuningForceColdStart;
}

// publish version & assign alias (if present)
await utils.createPowerConfiguration(lambdaARN, currConfig.powerValue, currConfig.alias, envVars);
if (typeof currConfig.alias !== 'undefined') {
// keep track of all aliases
aliases.push(currConfig.alias);
}

// update iterator
iterator.index++;
iterator.continue = (iterator.index < iterator.count);
if (!iterator.continue) {
delete event.powerValues.initConfigurations;
}
event.powerValues.aliases = aliases;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems confusing here. event.powerValues is just a list of integer values, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, figured this is because of the result path of the Initializer function. As commented below, I would rename that result path to make this easier to understand. event.powerValues used to just contain the list of power values to stream down to the rest of the state machine, but now it contains a lot more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, instead of modifying event.powerValues.aliases I would just keep a copy of the entire event.powerValues (extracted with validateInputs) and work on that. And then just return it at the end of the function invocation. Not a critical change, but it'd be easier to understand and debug.

return event.powerValues;
};
function validateInputs(event) {
if (!event.lambdaARN) {
throw new Error('Missing or empty lambdaARN');
}
const lambdaARN = event.lambdaARN;
if (!(event.powerValues && event.powerValues.iterator && event.powerValues.initConfigurations)){
throw new Error('Invalid input');
mriccia marked this conversation as resolved.
Show resolved Hide resolved
}
const iterator = event.powerValues.iterator;
if (!(iterator.index >= 0 && iterator.index < iterator.count)){
throw new Error('Invalid iterator input');
mriccia marked this conversation as resolved.
Show resolved Hide resolved
}
const initConfigurations = event.powerValues.initConfigurations;
const aliases = event.powerValues.aliases || [];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be empty the first time Publisher is invoked, right?

const currIdx = iterator.index;
const currConfig = initConfigurations[currIdx];
if (!(currConfig && currConfig.powerValue)){
throw new Error('Invalid configuration');
mriccia marked this conversation as resolved.
Show resolved Hide resolved
}
return {iterator, aliases, currConfig, lambdaARN};
}
46 changes: 32 additions & 14 deletions lambda/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ module.exports.lambdaBaseCost = (region, architecture) => {
return this.baseCostForRegion(priceMap, region);
};

module.exports.buildAliasString = (baseAlias, onlyColdStarts, index) => {
mriccia marked this conversation as resolved.
Show resolved Hide resolved
let alias = baseAlias;
if (onlyColdStarts) {
alias += `-${index}`;
}
return alias;
};

module.exports.allPowerValues = () => {
const increment = 64;
const powerValues = [];
Expand Down Expand Up @@ -69,28 +77,27 @@ module.exports.verifyAliasExistance = async(lambdaARN, alias) => {
/**
* Update power, publish new version, and create/update alias.
*/
module.exports.createPowerConfiguration = async(lambdaARN, value, alias) => {
module.exports.createPowerConfiguration = async(lambdaARN, value, alias, envVars) => {
try {
await utils.setLambdaPower(lambdaARN, value);
await utils.setLambdaPower(lambdaARN, value, envVars);

// wait for functoin update to complete
// wait for function update to complete
await utils.waitForFunctionUpdate(lambdaARN);

const {Version} = await utils.publishLambdaVersion(lambdaARN);
if (typeof alias === 'undefined'){
mriccia marked this conversation as resolved.
Show resolved Hide resolved
console.log('No alias defined');
return;
}
const aliasExists = await utils.verifyAliasExistance(lambdaARN, alias);
if (aliasExists) {
await utils.updateLambdaAlias(lambdaARN, alias, Version);
} else {
await utils.createLambdaAlias(lambdaARN, alias, Version);
}
} catch (error) {
if (error.message && error.message.includes('Alias already exists')) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be a real case, when something goes wrong with the Cleaner and there are still existing aliases from a previous power-tuning execution. In that case, we don't want the execution to explode (that's why the message said "Ok, even if ....").

Am I missing something or this could still happen?

// shouldn't happen, but nothing we can do in that case
console.log('OK, even if: ', error);
} else {
console.log('error during config creation for value ' + value);
throw error; // a real error :)
}
console.log('error during config creation for value ' + value);
throw error; // a real error :)
}
};

Expand Down Expand Up @@ -126,7 +133,7 @@ module.exports.waitForAliasActive = async(lambdaARN, alias) => {
},
};
const lambda = utils.lambdaClientFromARN(lambdaARN);
return lambda.waitFor('functionActive', params).promise();
return lambda.waitFor('functionActiveV2', params).promise();
};

/**
Expand All @@ -140,7 +147,11 @@ module.exports.getLambdaPower = async(lambdaARN) => {
};
const lambda = utils.lambdaClientFromARN(lambdaARN);
const config = await lambda.getFunctionConfiguration(params).promise();
return config.MemorySize;
return {
power: config.MemorySize,
// we need to fetch env vars only to add a new one and force a cold start
envVars: (config.Environment || {}).Variables || {},
};
};

/**
Expand Down Expand Up @@ -175,11 +186,12 @@ module.exports.getLambdaConfig = async(lambdaARN, alias) => {
/**
* Update a given Lambda Function's memory size (always $LATEST version).
*/
module.exports.setLambdaPower = (lambdaARN, value) => {
module.exports.setLambdaPower = (lambdaARN, value, envVars) => {
console.log('Setting power to ', value);
const params = {
FunctionName: lambdaARN,
MemorySize: parseInt(value, 10),
Environment: {Variables: envVars},
};
const lambda = utils.lambdaClientFromARN(lambdaARN);
return lambda.updateFunctionConfiguration(params).promise();
Expand Down Expand Up @@ -543,9 +555,15 @@ module.exports.regionFromARN = (arn) => {
return arn.split(':')[3];
};

let client;
module.exports.lambdaClientFromARN = (lambdaARN) => {
const region = this.regionFromARN(lambdaARN);
return new AWS.Lambda({region});
// create a client only once
if (typeof client === 'undefined'){
// set Max Retries to 20, increase the retry delay to 500
client = new AWS.Lambda({region: region, maxRetries: 20, retryDelayOptions: {base: 500}});
mriccia marked this conversation as resolved.
Show resolved Hide resolved
}
return client;
};

/**
Expand Down
Loading