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

Use USS (unique set size) instead of PSS for all the things #16570

Merged
merged 3 commits into from
Dec 15, 2017

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Nov 30, 2017

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1479356

Builds on top of #16569 [MERGED]

  • Change worker validation to check USS not PSS
  • Show USS instead of PSS in rake evm:status
  • Also log the server/worker unique set size (USS)

The first bullet point should alleviate the false positives where a large server process was causing new forked workers to exceed PSS memory thresholds very quickly. If the server process grows over time, that's a separate problem. We shouldn't flag and kill workers that inherited a large shared memory, only ones that grow beyond the thresholds on their own.

For prior versions such as euwe and fine, we can accomplish a similar result by storing the USS in the existing PSS column:

@jrafanie
Copy link
Member Author

@miq-bot add_label gaprindashvili/yes
@miq-bot add_label performance
@miq-bot add_label core/workers

@jrafanie jrafanie closed this Dec 1, 2017
@jrafanie jrafanie reopened this Dec 1, 2017
@jrafanie jrafanie changed the title [WIP] Use USS (unique set size) instead of PSS for all the things Use USS (unique set size) instead of PSS for all the things Dec 1, 2017
@jrafanie jrafanie removed the wip label Dec 1, 2017
@jrafanie
Copy link
Member Author

jrafanie commented Dec 1, 2017

@gtanzillo @Fryguy this is now ready for review with the prior PR merged cc @dmetzger57 @NickLaMuro

Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me ... any reason this can't go in?

@jrafanie does this address a specific bug or is this a general fix?

@jrafanie
Copy link
Member Author

Good point, let me put a BZ link @carbonin

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1479356

Why? USS is a more reliable mechanism for tracking workers with runaway
memory growth. PSS is great, until the server process that forks new
processes grows large. As each new worker is forked, it inherits a share
of the large amount of the parent process' memory and therefore starts
with a large PSS, possibly exceeding our limits before doing any work.
USS only measures a process' private memory and is a better indicator
when a process is responsible for allocating too much memory without
freeing it.
@miq-bot
Copy link
Member

miq-bot commented Dec 15, 2017

Checked commits jrafanie/manageiq@2d75877~...ef84a88 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🍪

@carbonin carbonin self-assigned this Dec 15, 2017
@carbonin carbonin added the bug label Dec 15, 2017
@carbonin carbonin added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 15, 2017
@carbonin carbonin merged commit eee0570 into ManageIQ:master Dec 15, 2017
simaishi pushed a commit that referenced this pull request Dec 18, 2017
Use USS (unique set size) instead of PSS for all the things
(cherry picked from commit eee0570)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1527093
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 0c79e86b72da95bde62b998f274d3080080f0f58
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Fri Dec 15 17:20:56 2017 -0500

    Merge pull request #16570 from jrafanie/USS_all_the_things
    
    Use USS (unique set size) instead of PSS for all the things
    (cherry picked from commit eee05705f47ca91d3785dc4caed3fd7188f5817c)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1527093

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants