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

apt: speedup installing packages with '>=' or '<=' and reduced memory… #50270

Conversation

Projects
None yet
3 participants
@terrible-broom
Copy link
Contributor

commented Oct 29, 2018

… consumption

What does this PR do?

Speedup salt work and reduce memory comsumption in aptpkg.py when installing packages with operators '>=' and '<='.

What issues does this PR fix or reference?

Previous Behavior

For each pkg.installed with '>=' or '<=' salt saved output of "apt-cache dump" to variable and then read it. But this output could be huge. For example, on my system it took 47 Gb, but I didn't have 47 Gb of memory, so salt couldn't work.

New Behavior

Salt just saves output of "apt-cache show " if pkg is given, or full output: "apt-cache show .". It is much shorter than "apt-cache dump" (380 Mb on the same system) but contains the same useful information.

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@salt-jenkins salt-jenkins requested a review from saltstack/team-suse Oct 29, 2018

cmd = ['apt-cache', 'show' ] + [arg for arg in args]
else:
# get information about all available packages
cmd = ['apt-cache', 'show', '.']

This comment has been minimized.

Copy link
@isbm

isbm Oct 29, 2018

Contributor
cmd = ['apt-cache', 'show' ] + [arg for arg in args or ['.']]

But I don't like you use apt-cache show . here. It is super-slow, apparently, and doesn't speeds this up. If you use apt-cache dump, it will get all packages ~100Mb data about 2-3 seconds. The other one will do really long. The only reason I see to use apt-cache show is when you want to see specific packages, so no "skip thing" logic you've deleted before. I would then use both commands: "dump" for just everything and "show" only for specific packages:

cmd = ['apt-cache']
if args:
    cmd.expand(['show'] + [arg for arg in args])
else:
    cmd.append('dump')

This comment has been minimized.

Copy link
@isbm

isbm Oct 29, 2018

Contributor

Here is the proof: I don't know how this is ever good, when the entire core of the CPU is 100% full for another couple of minutes.

$ time apt-cache show . > /dev/null

real    2m24.267s
user    0m32.576s
sys     1m49.792s

And other story with just dumping it:

$ time apt-cache dump > /dev/null                                                           

real    0m1.182s
user    0m0.852s
sys     0m0.324s

This is just a no-go.

@@ -1671,29 +1678,19 @@ def list_repo_pkgs(*args, **kwargs): # pylint: disable=unused-import
skip_pkg = False
new_pkg = re.compile('^Package: (.+)')
for line in salt.utils.itertools.split(out['stdout'], '\n'):
if len(line.strip()) == 0:
continue

This comment has been minimized.

Copy link
@isbm

isbm Oct 29, 2018

Contributor
if not line.strip():
    continue
@terrible-broom

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2018

@isbm Thanks a lot for your feedback! I think you are right.
Pushed new commit.

@rallytime rallytime merged commit 57fe6cd into saltstack:2018.3 Oct 30, 2018

1 of 9 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/pr/lint The lint job has failed
Details
jenkins/pr/py2-centos-7 running py2-centos-7...
Details
jenkins/pr/py2-ubuntu-1604 running py2-ubuntu-1604...
Details
jenkins/pr/py2-windows-2016 running py2-windows-2016...
Details
jenkins/pr/py3-centos-7 running py3-centos-7...
Details
jenkins/pr/py3-ubuntu-1604 running py3-ubuntu-1604...
Details
jenkins/pr/py3-windows-2016 running py3-windows-2016...
Details
WIP Ready for review
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.