Set close_fds to False when spawning collector #367
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have noticed that on systems with a high value for SC_OPEN_MAX, tcollector
can cause spikes in CPU utilization when it runs a collector process.
The reason for this is that close_fds calls close() on every integer
between 3 and SC_OPEN_MAX before executing the child process, regardless
of if those file descriptors are currently open or not. On a
system with say a value of 1 million for SC_OPEN_MAX this is a pretty
expensive operation, even more so if that system is running something
like auditd/kaudit.
It seems that tcollector will typically spawn a short lived collector
that it will kill after a certain amount of time passes, so I don't
think the normal concerns of setting close_fds to False are applicable.
Some strace histogram output to demonstrate (note the error count):
A python bug report that discusses the same issue:
https://bugs.python.org/issue1663329
I am not clear on all of the potential downsides here so if anyone has
strong opinions on why it is a bad idea to disable close_fds here, I am
all ears.