Repository navigation
fix(idempotency): retry Redis inconsistent state errors - #8736
leandrodamascena merged 2 commits into
Conversation
powertools-for-aws-oss-automation
Bot
added
the
size/M
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
|
Thanks for the PR and for filling in the verification section, that helps. Heads up that the review may take a few days: this goes into 3.41.0 and I review every PR against a real service before approving, one at a time. No need to rebase or ping in the meantime, I'll come back here. |
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @coughlmao! I agree with the approach, the retry logic was already there, it just never saw this error. I tried to reproduce the race against a real Valkey and couldn't, it's too tight a window, so your test is what covers it. I did run the normal flow against the real server with your change and everything still works. Goes out in 3.41.0, no need to rebase, you'll be in the release notes.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8736 +/- ##
========================================
Coverage 97.17% 97.17%
========================================
Files 323 323
Lines 16588 16591 +3
Branches 1502 1502
========================================
+ Hits 16119 16122 +3
Misses 328 328
Partials 141 141 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: closes #8727
Summary
Fix a race condition in the Redis idempotency persistence layer where an existing record expires between a failed
SET NXoperation and the subsequentGET.Previously, this caused an
IdempotencyItemNotFoundErrorto escape and be wrapped as anIdempotencyPersistenceLayerError, failing the Lambda invocation instead of retrying.Changes
IdempotencyItemNotFoundErrorwhen retrieving a record after a failed RedisSET NX.IdempotencyInconsistentStateErrorto signal the recoverable inconsistent state.User experience
Before: If an existing Redis idempotency record expired between
SET NXandGET, the invocation failed with a persistence-layer error.After: The Redis persistence layer raises
IdempotencyInconsistentStateError, allowing the existing retry mechanism to retry the operation. The invocation succeeds when the subsequent attempt acquires the lock successfully.Verification
Tests and checks run locally:
uv run --locked pytest tests/functional/idempotency/_redis/test_redis_layer.py -k inconsistent_state -v— 3 passed.uv run --locked pytest tests/functional/idempotency/_redis/test_redis_layer.py -q— 33 passed.uv run --locked ruff check aws_lambda_powertools/utilities/idempotency/base.py aws_lambda_powertools/utilities/idempotency/persistence/redis.py tests/functional/idempotency/_redis/test_redis_layer.py— passed.uv run --locked mypy aws_lambda_powertools/utilities/idempotency/base.py aws_lambda_powertools/utilities/idempotency/persistence/redis.py— passed.git diff --check— passed.The regression scenario was verified using a mocked Redis client. Real Redis service validation has not been performed.
This contribution was developed with AI assistance. I reviewed the changes and personally ran the tests and checks listed above.
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.