fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete #18536

corymhall · 2022-01-19T19:07:52Z

There are upcoming changes
that will rollout Lambda states to all Lambda Functions. Prior to
this update (current functionality) when you made an
updateFunctionCode request the function was immediately available for
both invocation and future updates. Once this change is rolled out this
will no longer be the case. With Lambda states, when you make an update
to a Lambda Function, it will not be available for future updates until
the LastUpdateStatus returns Successful.

This PR introduces a custom waiter that will wait for the update to
complete before proceeding. The waiter will wait until the
State=Active and the LastUpdateStatus=Successful.

The State controls whether or not the function can be invoked, and the
LastUpdateStatus controls whether the function can be updated. Based
on this, I am not considering a deployment complete until both are
successful. To see a more in depth analysis of the different values see #18386.

In my testing I found that the time it took for a function to go from
LastUpdateStatus=InProgress to LastUpdateStatus=Successful was:

~1 second for a zip Function not in a VPC
~25 seconds for a container Function or a Function in a VPC
~2 minutes to deploy a VPC function (best proxy for StateReasonCode=Restoring)

There are a couple of built in waiters that could have been used for
this, namely
functionUpdated.
This waiter uses getFunctionConfiguration which has a quota of 15
requests/second. In addition the waiter polls every 5 seconds and this
cannot be configured. Because hotswapping is sensitive to any latency
that is introduced, I created a custom waiter that uses getFunction.
getFunction has a quota of 100 requests/second and the custom waiter
can be configured to poll every 1 second or every 5 seconds depending on
what type of function is being updated.

fixes #18386

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…mplete There are [upcoming changes](https://aws.amazon.com/blogs/compute/coming-soon-expansion-of-aws-lambda-states-to-all-functions/) that will rollout Lambda states to all Lambda Functions. Prior to this update (current functionality) when you made an `updateFunctionCode` request the function was immediately available for both invocation and future updates. Once this change is rolled out this will no longer be the case. With Lambda states, when you make an update to a Lambda Function, it will not be available for future updates until the `LastUpdateStatus` returns `Successful`. This PR introduces a custom waiter that will wait for the update to complete before proceeding. The waiter will wait until the `State=Active` and the `LastUpdateStatus=Successful`. The `State` controls whether or not the function can be invoked, and the `LastUpdateStatus` controls whether the function can be updated. Based on this, I am not considering a deployment complete until both are successful. To see a more in depth analysis of the different values see #18386. In my testing I found that the time it took for a function to go from `LastUpdateStatus=InProgress` to `LastUpdateStatus=Successful` was: - ~1 second for a zip Function not in a VPC - ~25 seconds for a container Function or a Function in a VPC - ~2 minutes to deploy a VPC function (best proxy for StateReasonCode=Restoring) There are a couple of built in waiters that could have been used for this, namely [functionUpdated](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Lambda.html#functionUpdated-waiter). This waiter uses `getFunctionConfiguration` which has a quota of 15 requests/second. In addition the waiter polls every 5 seconds and this cannot be configured. Because hotswapping is sensitive to any latency that is introduced, I created a custom waiter that uses `getFunction`. `getFunction` has a quota of 100 requests/second and the custom waiter can be configured to poll every 1 second or every 5 seconds depending on what type of function is being updated. fixes #18386

gitpod-io · 2022-01-19T19:07:55Z

skinny85

Looks great @corymhall! A few minor notes.

Do you see any degradation in performance of hotswapping for the "simple" case (Lambda not in a VPC with code from a file Asset, not a Docker image) compared to what it was before these changes?

skinny85 · 2022-01-20T01:14:48Z

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

+    let delay = 1; // in seconds
+    // if the function is deployed in a VPC or if it is a container image function
+    // then the update will take much longer and we can wait longer between checks
+    if (currentFunctionConfiguration.VpcConfig || currentFunctionConfiguration.PackageType === 'Image') {
+      delay = 5;
+    }


Let's just do it in one expression - I just hate using let when a const would do:

Suggested change

let delay = 1; // in seconds

// if the function is deployed in a VPC or if it is a container image function

// then the update will take much longer and we can wait longer between checks

if (currentFunctionConfiguration.VpcConfig || currentFunctionConfiguration.PackageType === 'Image') {

delay = 5;

}

const delaySeconds = currentFunctionConfiguration.VpcConfig ||

currentFunctionConfiguration.PackageType === 'Image'

// if the function is deployed in a VPC or if it is a container image function

// then the update will take much longer and we can wait longer between checks

? 5

// otherwise, the update will be quick, so a 1-second delay is fine

: 1;

Updated. I also discovered that Lambdas not in a VPC can return a VPC config like

VpcConfig: { VpcId: '', SubnetIds: [], SecurityGroupIds: [] }

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

skinny85 · 2022-01-20T01:19:43Z

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

+        {
+          state: 'retry',
+          matcher: 'path',
+          argument: 'Configuration.LastUpdateStatus',
+          expected: 'InProgress',
+        },


Do we actually need this? I thought 'retry' was the default if the response was neither 'success' nor 'failure'?

I guess we don't. I was following the logic here, but I actually think it is more clear without specifying retry so I went ahead and changed it.

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts

corymhall · 2022-01-20T16:10:09Z

Do you see any degradation in performance of hotswapping for the "simple" case (Lambda not in a VPC with code from a file Asset, not a Docker image) compared to what it was before these changes?

In my testing it doesn't add any time if Lambda states are disabled. If Lambda states are enabled then it adds about 1 second.

Before: Deployment time: 0.78s
After: Deployment time: 1.78s

skinny85

Looks amazing @corymhall! A few more nitpicks from me, and we can merge this in!

packages/aws-cdk/test/api/hotswap/lambda-versions-aliases-hotswap-deployments.test.ts

packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts

skinny85 · 2022-01-20T19:51:11Z

Do you see any degradation in performance of hotswapping for the "simple" case (Lambda not in a VPC with code from a file Asset, not a Docker image) compared to what it was before these changes?

In my testing it doesn't add any time if Lambda states are disabled. If Lambda states are enabled then it adds about 1 second.

Before: Deployment time: 0.78s
After: Deployment time: 1.78s

That's great. One second is great compared to the "naive" waiter that adds 5 seconds.

Awesome work!

skinny85

👍

mergify · 2022-01-20T21:54:42Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

skinny85

Since this failed to build because of the merge back, I've snuck in a few last-minute optional comments if you want to include them 🙂.

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts

skinny85 · 2022-01-20T23:27:58Z

packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts

@@ -83,8 +83,24 @@ export class HotswapMockSdkProvider {
    });
  }

-  public stubLambda(stubs: SyncHandlerSubsetOf<AWS.Lambda>) {
-    this.mockSdkProvider.stubLambda(stubs);
+  public stubLambda(stubs: SyncHandlerSubsetOf<AWS.Lambda>, additionalProperties: { [key: string]: any } = {}) {


Can you specify the return type here explicitly?

What do you think of adding some type-safety here? Right now, using this method, you have to know a lot about the structure of the things you are passing as the second argument. Maybe we can re-use some of the SDK types here?

Can you specify the return type here explicitly?

Done.

2. What do you think of adding some type-safety here? Right now, using this method, you have to know a lot about the structure of the things you are passing as the second argument. Maybe we can re-use some of the SDK types here?

Added an additional input property for the AWS.Service methods.

skinny85 · 2022-01-20T23:31:18Z

packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts

+    });
+  }
+
+  public getLambdaApiWaiters() {


Please specify the return type of this method (you might want to introduce some anonymous types here, to help consumers of this method).

Added return type of { [key: string]: any }

mergify · 2022-01-21T16:11:45Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-01-21T17:36:35Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 0918edc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-01-21T17:36:54Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

* origin/master: (31 commits) feat(iotevents): add DetectorModel L2 Construct (aws#18049) feat(ecs): add `BaseService.fromServiceArnWithCluster()` for use in CodePipeline (aws#18530) docs(s3): remove vestigial documentation (aws#18604) chore(cli): LogMonitor test fails randomly due to Date.now() (aws#18601) chore(codedeploy): migrate tests to use the Assertions module (aws#18585) docs(stepfunctions): fix typo (aws#18583) chore(eks-legacy): migrate tests to `assertions` (aws#18596) fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete (aws#18536) fix(apigatewayv2): websocket api: allow all methods in grant manage connections (aws#18544) chore(dynamodb): migrate tests to assertions (aws#18560) fix(aws-apigateway): cross region authorizer ref (aws#18444) feat(lambda-nodejs): Allow setting mainFields for esbuild (aws#18569) docs(cfnspec): update CloudFormation documentation (aws#18587) feat(assertions): support for conditions (aws#18577) fix(ecs-patterns): Fix Network Load Balancer Port assignments in ECS Patterns (aws#18157) chore(region-info): ap-southeast-3 (Jakarta) ELBV2_ACCOUNT (aws#18300) fix(pipelines): CodeBuild projects are hard to tell apart (aws#18492) fix(ecs): only works in 'aws' partition (aws#18496) chore(app-delivery): migrate unit tests to Assertions (aws#18574) chore: migrate kaizen3031593's modules to assertions (aws#18205) ...

…mplete (aws#18536) There are [upcoming changes](https://aws.amazon.com/blogs/compute/coming-soon-expansion-of-aws-lambda-states-to-all-functions/) that will rollout Lambda states to all Lambda Functions. Prior to this update (current functionality) when you made an `updateFunctionCode` request the function was immediately available for both invocation and future updates. Once this change is rolled out this will no longer be the case. With Lambda states, when you make an update to a Lambda Function, it will not be available for future updates until the `LastUpdateStatus` returns `Successful`. This PR introduces a custom waiter that will wait for the update to complete before proceeding. The waiter will wait until the `State=Active` and the `LastUpdateStatus=Successful`. The `State` controls whether or not the function can be invoked, and the `LastUpdateStatus` controls whether the function can be updated. Based on this, I am not considering a deployment complete until both are successful. To see a more in depth analysis of the different values see aws#18386. In my testing I found that the time it took for a function to go from `LastUpdateStatus=InProgress` to `LastUpdateStatus=Successful` was: - ~1 second for a zip Function not in a VPC - ~25 seconds for a container Function or a Function in a VPC - ~2 minutes to deploy a VPC function (best proxy for StateReasonCode=Restoring) There are a couple of built in waiters that could have been used for this, namely [functionUpdated](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Lambda.html#functionUpdated-waiter). This waiter uses `getFunctionConfiguration` which has a quota of 15 requests/second. In addition the waiter polls every 5 seconds and this cannot be configured. Because hotswapping is sensitive to any latency that is introduced, I created a custom waiter that uses `getFunction`. `getFunction` has a quota of 100 requests/second and the custom waiter can be configured to poll every 1 second or every 5 seconds depending on what type of function is being updated. fixes aws#18386 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

corymhall added 2 commits January 19, 2022 17:50

updating tests

140a4e9

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 19, 2022

github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jan 19, 2022

github-actions bot assigned rix0rrr Jan 19, 2022

corymhall requested a review from skinny85 January 19, 2022 19:09

skinny85 suggested changes Jan 20, 2022

View reviewed changes

skinny85 assigned skinny85 and unassigned rix0rrr Jan 20, 2022

updating based on review

9f9e720

corymhall requested a review from skinny85 January 20, 2022 16:10

skinny85 reviewed Jan 20, 2022

View reviewed changes

updates based on review

4e6f78e

corymhall requested a review from skinny85 January 20, 2022 20:18

skinny85 approved these changes Jan 20, 2022

View reviewed changes

Merge branch 'master' into corymhall/hotswap-lambda-wait

0b58f07

skinny85 approved these changes Jan 20, 2022

View reviewed changes

corymhall added the pr/do-not-merge This PR should not be merged at this time. label Jan 21, 2022

corymhall added 2 commits January 21, 2022 08:39

Merge branch 'master' into corymhall/hotswap-lambda-wait

4333dfe

updates

757eb20

corymhall removed the pr/do-not-merge This PR should not be merged at this time. label Jan 21, 2022

Merge branch 'master' into corymhall/hotswap-lambda-wait

0918edc

mergify bot merged commit 0e08eeb into aws:master Jan 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete #18536

fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete #18536

corymhall commented Jan 19, 2022

gitpod-io bot commented Jan 19, 2022

skinny85 left a comment •

edited

Loading

skinny85 Jan 20, 2022

corymhall Jan 20, 2022

skinny85 Jan 20, 2022

corymhall Jan 20, 2022

corymhall commented Jan 20, 2022 •

edited by skinny85

Loading

skinny85 left a comment

skinny85 commented Jan 20, 2022

skinny85 left a comment

mergify bot commented Jan 20, 2022

skinny85 left a comment

skinny85 Jan 20, 2022

corymhall Jan 21, 2022

skinny85 Jan 20, 2022

corymhall Jan 21, 2022

mergify bot commented Jan 21, 2022

aws-cdk-automation commented Jan 21, 2022

mergify bot commented Jan 21, 2022

fix(cli): hotswap should wait for lambda's updateFunctionCode to complete #18536

fix(cli): hotswap should wait for lambda's updateFunctionCode to complete #18536

Conversation

corymhall commented Jan 19, 2022

gitpod-io bot commented Jan 19, 2022

skinny85 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corymhall commented Jan 20, 2022 • edited by skinny85 Loading

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 commented Jan 20, 2022

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 20, 2022

skinny85 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Jan 21, 2022

aws-cdk-automation commented Jan 21, 2022

AWS CodeBuild CI Report

mergify bot commented Jan 21, 2022

fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete #18536

fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete #18536

skinny85 left a comment •

edited

Loading

corymhall commented Jan 20, 2022 •

edited by skinny85

Loading