search

serverless / examples

Serverless Examples – A collection of boilerplates and examples of serverless architectures built with the Serverless Framework on AWS Lambda, Microsoft Azure, Google Cloud Functions, and more.
https://www.serverless.com/examples/
Other
11.45k stars 4.47k forks source link

Ensure the function in aws-node-express-dynamodb-api example to be idempotent #658

Closed Nsupyq closed 3 years ago

Nsupyq commented 3 years ago

This PR tries to fix issue #657. It replaces the non-idempotent API dynamoDbClient.put with the idempotent API dynamoDbClient.transactWrite to make the function of writing username in index.js idempotent. Invoking dynamoDbClient.transactWrite needs a unique identifier called ClientRequestToken, which is used by DynamoDB to determine whether the incoming write operation is a retry. Thus this PR adds a function parameter called token to be ClientRequestToken.

mnapoli commented 3 years ago

Hi! Looking at #657 and the PR, I wonder whether this is worth fixing? The example is really simple and small (as it's supposed to be). By adding in transactions, it makes it much more complex (e.g. now to play the example we need to send a token, which is not really clear how to get that token).

Is the example really supposed to be perfect at a high scale?

pgrzesik commented 3 years ago

I share the same sentiment as @mnapoli here - looking at the change, it now seems more cryptic to users that are just starting out and that's the goal of these type of templates. Do you strongly feel that making the example more complex is worth it here?

Nsupyq commented 3 years ago

Hi! I have simplified the idempotence check mechanism. Now we do not need to send a token. The requestId provided by AWS Lambda can be used as the ClientRequestToken.

mnapoli commented 3 years ago

Right, thank you.

To be honest, I have reservations. First, the code change that you introduced uses the AWS request ID as a token: are we 100% sure that request ID is the same in retries? (I would doubt that)

Second, now that we have the diff, and after re-reading #657, it seems to me this change isn't needed at all:

Assuming two concurrent functions want to change the user name with the same userId. When one of them fails after executing dynamodb_client.put, AWS Lambda will retry it

The example is an express app: this is an HTTP (API Gateway) event, which is synchronous. Lambda will not retry the invocation.

So I think there is no issue to fix here?

Nsupyq commented 3 years ago

For your first question, AWS Lambda will use the same parameters to retry a function. The awsRequestId is part of the functional parameters and it is the same in retries.

For the second question, although this is an HTTP (API Gateway) event, the client can still invoke the function asynchronously via AWS CLI or some other ways. Then the function will still be retried.

Finally, idempotence is a new requirement for developers of serverless applications and is important for fault tolerance. I think it would be better to teach people about this thing at the beginning.

mnapoli commented 3 years ago

For the second question, although this is an HTTP (API Gateway) event, the client can still invoke the function asynchronously via AWS CLI or some other ways.

Yes, but that's not what the example is about. This is a starter example, it doesn't have to account for the example to be misused in scenarios with high load.

mattwelke commented 3 years ago

@Nsupyq raised an issue in my examples repository (https://github.com/mattwelke/serverless-managed-dbs-examples) too related to this. I agree, these examples are meant to be starters only. The idea is that beginners get up and running as quick as possible with their functions. And then they can explore more complex examples over time when they need complex things.

But I also wanted to say, I love that you're out there contributing to repos @Nsupyq. Have you considered creating your own third-party example, like my repo, showing more advanced concepts like idempotent functions?

Nsupyq commented 3 years ago

Thank you @mattwelke @mnapoli :) I will create my own examples to introduce what is idempotency and how to guarantee that. I will propose new PRs when I finish them.