Skip to content

fix(install): limit preparer concurrency to prevent file handle exhaustion#17633

Draft
denyszhak wants to merge 3 commits intoastral-sh:mainfrom
denyszhak:fix/installer-concurrency
Draft

fix(install): limit preparer concurrency to prevent file handle exhaustion#17633
denyszhak wants to merge 3 commits intoastral-sh:mainfrom
denyszhak:fix/installer-concurrency

Conversation

@denyszhak
Copy link

@denyszhak denyszhak commented Jan 20, 2026

Summary

Resolves #17512

Fixes an issue where uv could exhaust the operating system's file descriptor limit when installing projects with a large number of local dependencies, causing a "Too many open files (os error 24)" crash.

Test Plan

  1. Built uv version that has an issue
  2. Issued ulimit -n 256 to set number of allowed descriptors to a low value
  3. mkdir -p /tmp/airflow-test
  4. git clone --depth 1 https://github.com/apache/airflow /tmp/airflow-test
  5. cd /tmp/airflow-test
  6. /Users/denyszhak/personal-repos/uv/target/debug/uv sync

Bug: Hit Too many open files (os error 24) at path "/Users/denyszhak/.cache/uv/sdists-v9/editable/136c133a3434a0fa/.tmpZtqKQt"

  1. Built uv version with the fix (source branch)
  2. /Users/denyszhak/personal-repos/uv/target/debug/uv sync

Fix: Successfully Resolved 922 packages in 10.82s

@denyszhak
Copy link
Author

denyszhak commented Jan 20, 2026

Not sure why test job is failing, seems unrelated to the change at first.

@konstin
Copy link
Member

konstin commented Jan 21, 2026

Yep the failure is unrelated: #17637

@konstin konstin added the bug Something isn't working label Jan 21, 2026
@konstin
Copy link
Member

konstin commented Jan 21, 2026

Interesting, I thought we were already using limits early but it seems we aren't.

CC @charliermarsh for the preparer code.

@konstin konstin requested a review from charliermarsh January 21, 2026 12:04
konstin
konstin reviewed Jan 21, 2026
View reviewed changes
@denyszhak denyszhak marked this pull request as draft February 23, 2026 00:18
konstin added a commit that referenced this pull request Feb 23, 2026
@konstin
Use global semaphores for concurrency limits
8b427a2 
Avoid problems such as #15307, follow-up to #18054. See also #17633, for which this should be helpful.
@konstin konstin mentioned this pull request Feb 23, 2026
Merged
konstin
konstin reviewed Feb 23, 2026
View reviewed changes
crates/uv-dispatch/src/lib.rs Outdated
sources: NoSources,
workspace_cache: WorkspaceCache,
concurrency: Concurrency,
source_distribution_limiter: Arc<Semaphore>,
Copy link
Member

@konstin konstin Feb 23, 2026

Choose a reason for hiding this comment

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

We should really handle this better globally: #18156

Copy link
Author

@denyszhak denyszhak Feb 23, 2026

Choose a reason for hiding this comment

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

thanks, closing this one

Copy link
Member

@konstin konstin Feb 23, 2026

Choose a reason for hiding this comment

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

It should be enough to rebase this on top of the other PR once it lands.

Copy link
Author

@denyszhak denyszhak Feb 23, 2026

Choose a reason for hiding this comment

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

Let me wait then, I thought you plan handling this thing there as well

Copy link
Member

@konstin konstin Feb 23, 2026

Choose a reason for hiding this comment

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

I haven't looked into this one. I believe we can handle them semi-independently, #18156 should only remove the amount of plumbing here, while we still need to figure out the correct approach to the file locks in this PR. It's also fine by me if we land this one first and I rebase #18156 on top of it.

Copy link
Author

@denyszhak denyszhak Feb 23, 2026

Choose a reason for hiding this comment

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

I would wait for your merged first so this one can be more local on the exact thing it addresses

Copy link
Member

@konstin konstin Feb 25, 2026

Choose a reason for hiding this comment

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

#18156 is merged now.

konstin added a commit that referenced this pull request Feb 23, 2026
@konstin
Use global semaphores for concurrency limits
599effa 
Avoid problems such as #15307, follow-up to #18054. See also #17633, for which this should be helpful.
konstin added a commit that referenced this pull request Feb 25, 2026
@konstin
Use global semaphores for concurrency limits
1369a7b 
Avoid problems such as #15307, follow-up to #18054. See also #17633, for which this should be helpful.
konstin added a commit that referenced this pull request Feb 25, 2026
@konstin
Use global semaphores for concurrency limits (#18156)
3697a2d 
Avoid problems such as #15307,
follow-up to #18054. See also
#17633, for which this should be
helpful.
@denyszhak denyszhak force-pushed the fix/installer-concurrency branch from 51390d1 to 2468a97 Compare March 2, 2026 01:00
@denyszhak denyszhak force-pushed the fix/installer-concurrency branch from 2468a97 to 949397f Compare March 2, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@konstin konstin konstin left review comments

@charliermarsh charliermarsh Awaiting requested review from charliermarsh

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate why uv holds so many file handles open

2 participants

@denyszhak @konstin