-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: add --fdlimit flag #20168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add --fdlimit flag #20168
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,8 +201,8 @@ impl LaunchContext { | |
| } | ||
|
|
||
| /// Convenience function to [`Self::configure_globals`] | ||
| pub fn with_configured_globals(self, reserved_cpu_cores: usize) -> Self { | ||
| self.configure_globals(reserved_cpu_cores); | ||
| pub fn with_configured_globals(self, reserved_cpu_cores: usize, fdlimit: u64) -> Self { | ||
| self.configure_globals(reserved_cpu_cores, fdlimit); | ||
| self | ||
| } | ||
|
|
||
|
|
@@ -212,15 +212,51 @@ impl LaunchContext { | |
| /// - Configuring the global rayon thread pool with available parallelism. Honoring | ||
| /// engine.reserved-cpu-cores to reserve given number of cores for O while using at least 1 | ||
| /// core for the rayon thread pool | ||
| pub fn configure_globals(&self, reserved_cpu_cores: usize) { | ||
| pub fn configure_globals(&self, reserved_cpu_cores: usize, fdlimit: u64) { | ||
| // Raise the fd limit of the process. | ||
| // Does not do anything on windows. | ||
| match fdlimit::raise_fd_limit() { | ||
| Ok(fdlimit::Outcome::LimitRaised { from, to }) => { | ||
| debug!(from, to, "Raised file descriptor limit"); | ||
| if fdlimit == 0 { | ||
| // Use system default (raise to maximum) | ||
| match fdlimit::raise_fd_limit() { | ||
|
Comment on lines
+218
to
+220
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will alter current behaviour and will likely be super annoying
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0 for old behavior may be okay. |
||
| Ok(fdlimit::Outcome::LimitRaised { from, to }) => { | ||
| debug!(from, to, "Raised file descriptor limit"); | ||
| } | ||
| Ok(fdlimit::Outcome::Unsupported) => {} | ||
| Err(err) => warn!(%err, "Failed to raise file descriptor limit"), | ||
| } | ||
| } else { | ||
| // Set to specific limit | ||
| #[cfg(unix)] | ||
| { | ||
| let result = unsafe { | ||
| let mut rlimit = libc::rlimit { rlim_cur: 0, rlim_max: 0 }; | ||
| if libc::getrlimit(libc::RLIMIT_NOFILE, &raw mut rlimit) == 0 { | ||
| let old_limit = rlimit.rlim_cur; | ||
| rlimit.rlim_cur = fdlimit.min(rlimit.rlim_max) as libc::rlim_t; | ||
| if libc::setrlimit(libc::RLIMIT_NOFILE, &raw const rlimit) == 0 { | ||
| if rlimit.rlim_cur != old_limit { | ||
| debug!( | ||
| from = old_limit, | ||
| to = rlimit.rlim_cur, | ||
| "Set file descriptor limit" | ||
| ); | ||
| } | ||
| Ok(()) | ||
| } else { | ||
| Err(std::io::Error::last_os_error()) | ||
| } | ||
| } else { | ||
| Err(std::io::Error::last_os_error()) | ||
| } | ||
| }; | ||
| if let Err(err) = result { | ||
| warn!(%err, limit = fdlimit, "Failed to set file descriptor limit"); | ||
| } | ||
| } | ||
| #[cfg(not(unix))] | ||
| { | ||
| warn!("File descriptor limit setting is not supported on this platform"); | ||
| } | ||
| Ok(fdlimit::Outcome::Unsupported) => {} | ||
| Err(err) => warn!(%err, "Failed to raise file descriptor limit"), | ||
| } | ||
|
|
||
| // Reserving the given number of CPU cores for the rest of OS. | ||
|
|
@@ -261,8 +297,8 @@ impl<T> LaunchContextWith<T> { | |
| /// | ||
| /// - Raising the file descriptor limit | ||
| /// - Configuring the global rayon thread pool | ||
| pub fn configure_globals(&self, reserved_cpu_cores: u64) { | ||
| self.inner.configure_globals(reserved_cpu_cores.try_into().unwrap()); | ||
| pub fn configure_globals(&self, reserved_cpu_cores: u64, fdlimit: u64) { | ||
| self.inner.configure_globals(reserved_cpu_cores.try_into().unwrap(), fdlimit); | ||
| } | ||
|
|
||
| /// Returns the data directory. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we really need this
imo always raising is appropriate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if the system allows setting a higher file descriptor limit, the actual availability of resources such as memory and CPU can impact the stability and performance of the program. Setting the file descriptor limit too high can lead to resource exhaustion, which may cause the system or application to become unstable. This is because every open file descriptor consumes system resources, and if the number of file descriptors is increased significantly, it can strain the system's capacity to manage them effectively. Therefore, it's important to consider the overall resource usage and system capabilities when adjusting file descriptor limits.