Since check_max_open_files has no bearing on wether we accept the minion or not, there is no reason to list the directory on every auth call. With this mechanism it is a "fixed cost" since it will run at loop_interval instead of on each auth call
Currently servers that handle many thousands of minions spend measurable time
doing only stat(2) calls.
In strace it looks like::
0.000093 stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=118, ...}) = 0
0.000236 open("/etc/salt/pki/master/minions", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 154
0.011412 stat("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", {st_mode=S_IFREG|0644, st_size=800, ...}) = 0
0.000102 stat("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", {st_mode=S_IFREG|0644, st_size=800, ...}) = 0
...many thousands lines...
0.000064 stat("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", {st_mode=S_IFREG|0644, st_size=800, ...}) = 0
0.000062 stat("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", {st_mode=S_IFREG|0644, st_size=796, ...}) = 0
0.000485 stat("/export/apps/salt/log/master", {st_mode=S_IFREG|0644, st_size=37769598988, ...}) = 0
0.000065 stat("/export/apps/salt/log/master", {st_mode=S_IFREG|0644, st_size=37769598988, ...}) = 0
0.000184 stat("/etc/salt/pki/master/minions_rejected/hostXXXX.linkedin.com", 0x7fff28209f40) = -1 ENOENT (No such file or directory)
0.000071 stat("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", {st_mode=S_IFREG|0644, st_size=800, ...}) = 0
0.000074 open("/etc/salt/pki/master/minions/hostXXXX.linkedin.com", O_RDONLY) = 154
Happens that on each _auth() call salt is counting files in pki/master/minions
and checks whenever those are regular files by applying os.path.isfile()
function to each which considerably slows things down.
This patch removes isfile() call that effectively makes check_max_open_files()
check less precise (but since it's already subject to external race conditions
like creation/removal of files by another process it should be OK) in exchange
for making it much faster.
PS. Note that calling salt.utils.verify.check_max_open_files() on each _auth is
not really efficient, it probably should be done periodically in a background
thread.
Sponsored by: LinkedIn
Signed-off-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
@s0undt3ch FYI. If you think this is a bad change, I'd be happy to
discuss it with you, but 99% of salt users don't use that handler (as
far as I know) and it's just garbage in the debug log.