Skip to content

Commit a884aed

Browse files
author
Remi Hakim
authored
[core][subprocess output] Do not set close_fds=True (#2950)
This was introduced by 24cf7823c254925ab061baf6aff71 2678a4c793f But it’s not needed anymore and as a result the collector is looping over thousand of file descriptors, trying to close them (most of the time it cannot) and use a lot of CPU to do so. This commit change that and revert to the default `close_fds=False` Also I’m removing a bunch of options that are not used anywhere.
1 parent 4ecd5ef commit a884aed

File tree

2 files changed

+7
-14
lines changed

2 files changed

+7
-14
lines changed

checks.d/varnish.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ def check(self, instance):
107107

108108
def _get_version_info(self, varnishstat_path):
109109
# Get the varnish version from varnishstat
110-
output, error, _ = get_subprocess_output([varnishstat_path, "-V"], self.log)
110+
output, error, _ = get_subprocess_output([varnishstat_path, "-V"], self.log,
111+
raise_on_empty_output=False)
111112

112113
# Assumptions regarding varnish's version
113114
use_xml = True

utils/subprocess_output.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,13 @@
99
import subprocess
1010
import tempfile
1111

12-
# project
13-
from utils.platform import Platform
14-
1512
log = logging.getLogger(__name__)
1613

1714
class SubprocessOutputEmptyError(Exception):
1815
pass
1916

2017
# FIXME: python 2.7 has a far better way to do this
21-
def get_subprocess_output(command, log, shell=False, stdin=None, output_expected=True):
18+
def get_subprocess_output(command, log, raise_on_empty_output=True):
2219
"""
2320
Run the given subprocess command and return it's output. Raise an Exception
2421
if an error occurs.
@@ -28,23 +25,18 @@ def get_subprocess_output(command, log, shell=False, stdin=None, output_expected
2825
# docs warn that the data read is buffered in memory. They suggest not to
2926
# use subprocess.PIPE if the data size is large or unlimited.
3027
with nested(tempfile.TemporaryFile(), tempfile.TemporaryFile()) as (stdout_f, stderr_f):
31-
proc = subprocess.Popen(command,
32-
close_fds=not Platform.is_windows(), # only set to True when on Unix, for WIN compatibility
33-
shell=shell,
34-
stdin=stdin,
35-
stdout=stdout_f,
36-
stderr=stderr_f)
28+
29+
proc = subprocess.Popen(command, stdout=stdout_f, stderr=stderr_f)
3730
proc.wait()
3831
stderr_f.seek(0)
3932
err = stderr_f.read()
4033
if err:
41-
log.debug("Error while running {0} : {1}".format(" ".join(command),
42-
err))
34+
log.debug("Error while running {0} : {1}".format(" ".join(command), err))
4335

4436
stdout_f.seek(0)
4537
output = stdout_f.read()
4638

47-
if output_expected and output is None:
39+
if not output and raise_on_empty_output:
4840
raise SubprocessOutputEmptyError("get_subprocess_output expected output but had none.")
4941

5042
return (output, err, proc.returncode)

0 commit comments

Comments
 (0)