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

[WIP] Load linux_admin once when needed #327

Conversation

@NickLaMuro
Copy link
Member

commented Jan 15, 2018

Due to some concerns about loading linux_admin multiple times when calling MiqSystem.num_cpus causing a memory leak, this creates a helper method that no-ops after it has been required once.

This is currently a [WIP] pull request as we verify further that calling require multiple times is leaking memory in the manageiq application, but putting this out there as a proposal for a quick fix. Fixing require to not leak is far more ideal, but will take time.

Concerns

Unsure if this will break autoloading in some way, though I doubt too many are trying to do a reload with in development linux_admin, or calling code where this would be executed and reloaded in development. Regardless, something to consider when reviewing.

@miq-bot miq-bot added the wip label Jan 15, 2018
Copy link
Member

left a comment

fun stuff


def self.require_linux_admin
return true if @linux_admin_required
require_linux_admin

This comment has been minimized.

Copy link
@kbrock

kbrock Jan 15, 2018

Member

think you mean

require "linux_admin"

This comment has been minimized.

Copy link
@NickLaMuro

NickLaMuro Jan 15, 2018

Author Member

Derp. Thanks for the catch. Will fix.

@NickLaMuro NickLaMuro force-pushed the NickLaMuro:surgical-fix-for-possible-leak-in-num-cpu branch from a6d9d01 to 3a7d4d1 Jan 15, 2018
@miq-bot

This comment has been minimized.

Copy link
Member

commented Jan 15, 2018

Checked commit NickLaMuro@3a7d4d1 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐️

Due to some concerns about loading `linux_admin` multiple times when
calling `MiqSystem.num_cpus` causing a memory leak, this creates a
helper method that no-ops after it has been required once.
@NickLaMuro

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2018

going to close this as ManageIQ/manageiq#16837 is more all encompassing of a fix for the entire project. Will re-open if it turns out that this is a more preferred/surgical/safer solution (doubt it, though).

@NickLaMuro NickLaMuro closed this Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.