Skip to content

Make ConcurrencyContext exit cleanup best-effort#22619

Merged
desertaxle merged 4 commits into
mainfrom
devin1/oss-8099-lease-release-best-effort
Jul 24, 2026
Merged

Make ConcurrencyContext exit cleanup best-effort#22619
desertaxle merged 4 commits into
mainfrom
devin1/oss-8099-lease-release-best-effort

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

closes #22615

ConcurrencyContext.__exit__ released its fallback cleanup_lease_ids in a bare loop, so a single transient failure stranded every remaining lease (holding slots until TTL expiry) and skipped super().__exit__(). This PR guards each release individually, logs a warning on failure, and always runs the context teardown. The same fix is applied to the v1 context's cleanup_slots loop, which had the identical pattern.

for lease_id in self.cleanup_lease_ids:
    try:
        client.release_concurrency_slots_with_lease(lease_id=lease_id)
    except Exception:
        logger.warning("Failed to release concurrency lease %s during cleanup", lease_id, exc_info=True)

The outer get_client(...) block is also guarded so client construction/teardown errors don't bypass super().__exit__().

Tests

tests/concurrency/test_context.py and tests/concurrency/v1/test_context.py each get a test where the first cleanup release raises: the remaining IDs are still attempted and the context var is reset.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
  • If this pull request adds or changes functionality, it includes tests or explains why tests are not needed.
  • If this pull request changes user-facing behavior, it updates documentation or explains why documentation is not needed. (internal cleanup-path behavior only)

Link to Devin session: https://app.devin.ai/sessions/b7ce20e35d99461fba5c18d77790ca3f
Requested by: @desertaxle

Co-authored-by: Alexander Streed <alex.s@prefect.io>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions github-actions Bot added the bug Something isn't working label Jul 24, 2026
Co-authored-by: Alexander Streed <alex.s@prefect.io>
Co-Authored-By: alex.s <ajstreed1@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c49479c130

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/prefect/concurrency/context.py
Co-authored-by: Alexander Streed <alex.s@prefect.io>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
Comment thread tests/concurrency/v1/test_context.py Outdated
@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 8 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing devin1/oss-8099-lease-release-best-effort (ef504b2) with main (a917cb7)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

Co-authored-by: Alexander Streed <alex.s@prefect.io>
Co-Authored-By: alex.s <ajstreed1@gmail.com>
@desertaxle
desertaxle merged commit d2d873b into main Jul 24, 2026
163 of 167 checks passed
@desertaxle
desertaxle deleted the devin1/oss-8099-lease-release-best-effort branch July 24, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One failed lease release in ConcurrencyContext.__exit__ strands all remaining leases

1 participant