aws-samples / aws-cdk-examples

Example projects using the AWS CDK
Apache License 2.0
5.12k stars 2.15k forks source link

Example doesn't underline the importance of a good timeout set up. #690

Open pmithrandir opened 2 years ago

pmithrandir commented 2 years ago

Describe the bug

In the s3_sns_sqs_lambda_chain example, I found a practice that is not valid, and might lead in some configuration to an infinite loop.

The visibility timeout is set to 30 seconds, which is fine... only if the lambda timeout is 5 sec.

According to that doc, there should be a factor 6 between the 2 values. https://docs.aws.amazon.com/lambda/latest/dg/with-sqs.html

If the lambda takes more that 30 seconds to run(which is unlikely in the example I agree) it will release the message again, triggering another lambda, etc...

I actually encounter that bug on production recently leading a 14 000$ overbilling.

Expected Behavior

The example should describe the relation between the 2 values. Maybe set a lambda time out constant and use that constant x6 calculation in the visibility_timeout parameter

Current Behavior

An infinite loop might be put in place.

Reproduction Steps

Add a wait 800 second in the lambda, and you'll reproduce the issue.

Possible Solution

apply recommendation of https://docs.aws.amazon.com/lambda/latest/dg/with-sqs.html link the 2 timeout.

Additional Information/Context

No response

CDK CLI Version

any

Framework Version

No response

Node.js Version

any

OS

any

Language

Python

Language Version

No response

pmithrandir commented 2 years ago

@aabragan I beleive this one might interest you.

Regards, Pierre

NGL321 commented 2 years ago

Hmm, this is a good callout, but I think it is more of an enhancement to the example than a bug (since in the given example, it is unrealistic to see problematic behavior spawning due to the set timeout). I am going to make it a feature-request for now, but if you think my classification is wrong, please just comment here again.

pmithrandir commented 2 years ago

Hello,

You are right it's more an enhancement than the bug report. I created the PR. https://github.com/aws-samples/aws-cdk-examples/pull/691

Regards, Pierre