Skip to content

[ML] Improve performance of closing files before spawning #2424

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

Conversation

droberts195
Copy link
Contributor

Before spawning new processes from the controller we close all open file descriptors except for stdin, stdout and stderr.

Previously this was done by checking every possible file descriptor to see if it was open, but this is very expensive if the file descriptor limit is high. During a lookback a large number of normalize processes get started, and this could lead to significant CPU usage by the controller process.

This change makes the file closure code learn the highest open file descriptor each time it is used, and work on the basis that no more than 10 files will be opened in between calls to it. This significantly reduces controller CPU usage on machines that have high file descriptor limits and run a lot of normalize processes.

Before spawning new processes from the `controller` we
close all open file descriptors except for stdin, stdout
and stderr.

Previously this was done by checking every possible file
descriptor to see if it was open, but this is very expensive
if the file descriptor limit is high.  During a lookback a
large number of `normalize` processes get started, and this
could lead to significant CPU usage by the `controller`
process.

This change makes the file closure code learn the highest
open file descriptor each time it is used, and work on the
basis that no more than 10 files will be opened in between
calls to it. This significantly reduces `controller` CPU
usage on machines that have high file descriptor limits and
run a lot of `normalize` processes.
Copy link
Contributor

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 merged commit 698dd92 into elastic:main Dec 5, 2022
@droberts195 droberts195 deleted the more_efficient_file_closure_before_spawn branch December 5, 2022 09:51
droberts195 added a commit to droberts195/ml-cpp that referenced this pull request Dec 5, 2022
Before spawning new processes from the `controller` we
close all open file descriptors except for stdin, stdout
and stderr.

Previously this was done by checking every possible file
descriptor to see if it was open, but this is very expensive
if the file descriptor limit is high.  During a lookback a
large number of `normalize` processes get started, and this
could lead to significant CPU usage by the `controller`
process.

This change makes the file closure code learn the highest
open file descriptor each time it is used, and work on the
basis that no more than 10 files will be opened in between
calls to it. This significantly reduces `controller` CPU
usage on machines that have high file descriptor limits and
run a lot of `normalize` processes.

Backport of elastic#2424
droberts195 added a commit to droberts195/ml-cpp that referenced this pull request Dec 5, 2022
Before spawning new processes from the `controller` we
close all open file descriptors except for stdin, stdout
and stderr.

Previously this was done by checking every possible file
descriptor to see if it was open, but this is very expensive
if the file descriptor limit is high.  During a lookback a
large number of `normalize` processes get started, and this
could lead to significant CPU usage by the `controller`
process.

This change makes the file closure code learn the highest
open file descriptor each time it is used, and work on the
basis that no more than 10 files will be opened in between
calls to it. This significantly reduces `controller` CPU
usage on machines that have high file descriptor limits and
run a lot of `normalize` processes.

Backport of elastic#2424
droberts195 added a commit that referenced this pull request Dec 5, 2022
Before spawning new processes from the `controller` we
close all open file descriptors except for stdin, stdout
and stderr.

Previously this was done by checking every possible file
descriptor to see if it was open, but this is very expensive
if the file descriptor limit is high.  During a lookback a
large number of `normalize` processes get started, and this
could lead to significant CPU usage by the `controller`
process.

This change makes the file closure code learn the highest
open file descriptor each time it is used, and work on the
basis that no more than 10 files will be opened in between
calls to it. This significantly reduces `controller` CPU
usage on machines that have high file descriptor limits and
run a lot of `normalize` processes.

Backport of #2424
droberts195 added a commit that referenced this pull request Dec 8, 2022
Before spawning new processes from the `controller` we
close all open file descriptors except for stdin, stdout
and stderr.

Previously this was done by checking every possible file
descriptor to see if it was open, but this is very expensive
if the file descriptor limit is high.  During a lookback a
large number of `normalize` processes get started, and this
could lead to significant CPU usage by the `controller`
process.

This change makes the file closure code learn the highest
open file descriptor each time it is used, and work on the
basis that no more than 10 files will be opened in between
calls to it. This significantly reduces `controller` CPU
usage on machines that have high file descriptor limits and
run a lot of `normalize` processes.

Backport of #2424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants