Skip to content

fix(idempotency): compute the Redis orphan lock timeout from the configured expiry - #8779

Open
CJstate wants to merge 1 commit into
aws-powertools:developfrom
CJstate:fix/8726-redis-lock-timeout
Open

CJstate wants to merge 1 commit into
aws-powertools:developfrom
CJstate:fix/8726-redis-lock-timeout

Conversation

@CJstate

@CJstate CJstate commented Nov 9, 2026 •

Copy link
Copy Markdown

Summary

The Redis/Cache orphan lock timeout was computed in CachePersistenceLayer.__init__, right after super().__init__() had set expires_after_seconds to its 3600-second default. min(10, 3600) therefore always produced 10, and the later configure() call — which applies the user's IdempotencyConfig — never changed it. A lock could outlive the record it protects when expires_after_seconds was below 10, leaving later calls unable to take over an orphaned record.

The timeout is now computed at the point of use in _acquire_lock, from self.expires_after_seconds after configuration. The 10-second cap is preserved for longer expiries.

Test

Added test_orphan_lock_ttl_follows_configured_expiry in tests/functional/idempotency/_redis/test_redis_layer.py. It drives the public idempotent decorator with a fake Redis client that records the ex argument of every set call, seeds an expired INPROGRESS record, and checks the ex used for the :lock key:

  • expires_after_seconds=2 -> lock ex=2 (was 10 before the fix)
  • expires_after_seconds=30 -> lock ex=10 (cap unchanged)

Verification run locally (terminal output):

$ uv run pytest tests/functional/idempotency/_redis/test_redis_layer.py -k orphan_lock_ttl -q
# before the fix
FAILED ... test_orphan_lock_ttl_follows_configured_expiry[2-2]
E       AssertionError: lock ex=10, want 2
=========== 1 failed, 1 passed, 31 deselected ============

# after the fix
================= 2 passed, 31 deselected =================
  • Full Redis functional test file: 33 passed.
  • ruff format aws_lambda_powertools tests examples --check: 1398 files already formatted; ruff check aws_lambda_powertools tests examples: All checks passed!.
  • mypy on the changed source: Success: no issues found in 1 source file.
  • All idempotency unit + functional tests: uv run pytest tests/functional/idempotency tests/unit/idempotency -q -> 210 passed.
  • The full unit + functional suite was also run. Two unrelated tests (auth_alpha/oauth2 timing and event_handler/_pydantic PurePath encoding) fail on this Windows/Python 3.14 setup; I confirmed both fail identically on an unmodified develop checkout, so they are pre-existing environment failures, not regressions from this change.

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.

closes #8726

…igured expiry

The lock timeout was cached in __init__ before configure() applied IdempotencyConfig, so min(10, 3600) always produced 10 seconds. Compute it in _acquire_lock from the configured expires_after_seconds instead.
@CJstate
CJstate requested a review from a team as a code owner November 9, 2026 03:32
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2026
@boring-cyborg boring-cyborg Bot added the tests label Nov 9, 2026
@boring-cyborg

boring-cyborg Bot commented Nov 9, 2026

Copy link
Copy Markdown

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

@blacksoxx blacksoxx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reads correct to me; I did not run the tests here.

On develop the timeout is frozen in __init__ right after super().__init__() sets the 3600 default, and configure() only runs later from the decorator, so min(10, 3600) was always 10. Computing it in _acquire_lock from self.expires_after_seconds fixes that and keeps the cap. Nothing else referenced _orphan_lock_timeout. The test goes through idempotent with a fake client and reads only the ex passed to set, which fits the public-API rule in AGENTS.md, and closes #8726 sits on its own line.

Validation: read the diff at ada9293 against base 111f93b, redis.py and base.py on develop. Not run: pytest, mypy.

@CJstate

CJstate commented Nov 9, 2026

Copy link
Copy Markdown
Author

Thanks for the confirmation! Let me know if there's anything else needed for this to be merged.

@CJstate

CJstate commented Nov 9, 2026

Copy link
Copy Markdown
Author

Hi @blacksoxx, I noticed the branch is currently behind develop. Would you prefer me to rebase and force-push to clean up the history, or will this be handled automatically during the squash merge? Thanks!

@blacksoxx

Copy link
Copy Markdown

I am a contributor here, not a maintainer, so the merge mechanics are theirs to decide. For what it is worth: the PR shows as mergeable, "behind" only means develop moved on, and PRs here land as single squash commits, so a rebase is only needed if a conflict appears or a maintainer asks for one.

@leandrodamascena leandrodamascena added this to the 3.41.0 milestone Nov 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Redis orphan lock timeout ignores expires_after_seconds and is always 10 seconds

3 participants