Skip to content
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

Make systemd service state less slow #29008

Merged
merged 5 commits into from
Nov 19, 2015
Merged

Conversation

k00mi
Copy link
Contributor

@k00mi k00mi commented Nov 18, 2015

As has been noted in #10710, execution of a systemd service state is quite slow. The suggested solution of using systemctl is-enabled only helps for the enable operation. Instead, these commits address the expensive function _untracked_custom_unit_found(), which is used in several other places.

Previously our highstate runs took about 45s to several minutes due to a bug in systemd/logind which leaks unit files. With these patches we are down to about 36s in all cases.

A stat is cheap and most units don't reside in /etc/systemd so the
expensive operation of gathering all known units and iterating over them
can be avoided.
@cachedout
Copy link
Contributor

@k00mi Thanks for this. I am concerned that this may regress the fix that was made here: #11921

Thoughts?

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Nov 18, 2015
@cachedout
Copy link
Contributor

@k00mi Additionally, it looks like this is causing a test to fail so if we were to merge this, we would need to have that fixed. Could you please take a look? Thanks.

Lukas Braun added 2 commits November 18, 2015 22:38
Instead of gathering and iterating over all known units we can just ask
systemd about a given unit to check if it's available.
@k00mi
Copy link
Contributor Author

k00mi commented Nov 18, 2015

@cachedout Indeed parametrized units were not handled correctly. I think I fixed it now by asking systemctl list-unit-files if the unit was not found in list-units and added a comment explaining why both are necessary. systemd-sysv-generator takes care of legacy units in /etc/init.d, so they appear as regular units in list-units.

The CentOS VMs don't even bootstrap properly and those tests that actually reach the systemd module fail with KeyError: 'cmd.run_stdout'. I have zero clue why this happens. Other functions in this module use that function, as did the previous version and I can't reproduce said error on any of our systems.

Looking at the definition of test_available(), I think the issue is with the test itself. It mocks get_all() (which would later call cmd.run_stdout), but instead of calling that I use cmd.run_stdout directly. I don't know enough about your test system to fix this, can someone more familiar take a look?

@cachedout
Copy link
Contributor

@k00mi That all sounds great. Thank you!

@jfindlay Any chance you could jump in here and help with the test errors?

@jfindlay
Copy link
Contributor

@k00mi, I have updated the test on your branch, k00mi#1

systemd module: new unit test for available
cachedout pushed a commit that referenced this pull request Nov 19, 2015
Make systemd service state less slow
@cachedout cachedout merged commit 30d0e7a into saltstack:develop Nov 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants