feat(idempotency): handle lambda timeout scenarios for INPROGRESS records#1387
Merged
heitorlessa merged 27 commits intoaws-powertools:developfrom Jul 29, 2022
Merged
Conversation
boring-cyborg
bot
added
documentation
pull-request-size
bot
added
the
size/XL
rubenfonseca
force-pushed
the
feat/expire-inprogress
branch
from
de71882 to
a4f8ce7
Compare
rubenfonseca
requested review from
am29d and
heitorlessa
and removed request for
a team and
am29d
Codecov Report
@@ Coverage Diff @@
## develop #1387 +/- ##
========================================
Coverage 99.88% 99.89%
========================================
Files 119 119
Lines 5429 5456 +27
Branches 620 624 +4
========================================
+ Hits 5423 5450 +27
Misses 2 2
Partials 4 4
Continue to review full report at Codecov.
|
pull-request-size
bot
added
size/L
heitorlessa
suggested changes
Jul 29, 2022
Contributor
heitorlessa
left a comment
heitorlessa
left a comment
There was a problem hiding this comment.
early comments before I resume reading it after lunch. At a quick glance, the pieces that would benefit the most now are:
- 1/ Comments on the formula you're using since I saw Lambda context giving us milliseconds, then epoch (sensible for comparison), then type coercion to string (?) - sounds like we could reuse the milliseconds instead of wall clock time. Also,
datetime.datetime.now()uses timezone offset too, instead of a safe non-backward timestamp like time.monotonic - 2/ I think the Update Expression would benefit to having the new one as a separate variable, then use f-string to combine them and make it more readable (which conditional is for which use case)
heitorlessa
reviewed
Jul 29, 2022
heitorlessa
reviewed
Jul 29, 2022
heitorlessa
reviewed
Jul 29, 2022
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
heitorlessa
reviewed
Jul 29, 2022
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
heitorlessa
reviewed
Jul 29, 2022
jeromevdl
reviewed
Jul 29, 2022
aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py
Outdated
Show resolved
Hide resolved
…d split diag section Signed-off-by: heitorlessa <lessa@amazon.co.uk>
Contributor
|
I've pushed an updated doc with the following changes:
|
…to feat/expire-inprogress * rubenfonseca/feat/expire-inprogress: chore(idempotency): no need to update expire on update chore(documentation): addressed comments chore(idempotency): simplified strings chore(idempotency): address comments chore(docs): add documentation to method chore(idempotency): added tests for handle_for_status
Contributor
|
Shortening the wording on sequence diagram to fit in the final SVG New (pushing shortly)
|
heitorlessa
approved these changes
Jul 29, 2022
heitorlessa
changed the title
heitorlessa
added a commit
that referenced
this pull request
Jul 29, 2022
…tools-python into develop * 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: feat(idempotency): handle lambda timeout scenarios for INPROGRESS records (#1387) chore(deps): bump jsii from 1.57.0 to 1.63.1 (#1390) chore(deps): bump constructs from 10.1.1 to 10.1.59 (#1396) chore(deps-dev): bump flake8-isort from 4.1.1 to 4.1.2.post0 (#1384) docs(examples): enforce and fix all mypy errors (#1393) chore(ci): drop 3.6 from workflows (#1395)
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: #1038
Summary
Expire in-progress invocations caused by Lambda time outs.
Changes
During an invocation, we try to figure out how much time the Lambda invocation has before it expires, and save it on the idempotency record together will all the other fields.
When a new invocation with the same key arrives, we check against this timestamp to see if there's any
INPROGRESSinvocation past the timeout timestamp. If there is, we allow the invocation to be retried.We also moved the squence diagrams of the idempotency module in documentation to mermaid.js.
Caveats
@idempotentdecorator, because right now it's the only way to access the Lambda context to get the remaining time left. I've added aregister_lambda_contextto theIdempotencyConfigclass as a workaroundUser experience
Before: if a Lambda invocation times out, the invocation will get stuck until the idempotency record expires (default = 1 hour).
After: if a Lambda invocation times out, the invocation will be tried again on the next retry.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.
View rendered docs/utilities/idempotency.md