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 ruby not runner for run single worker #15825

Merged

Conversation

jrafanie
Copy link
Member

This is another version of #15821

Basically:

  • drop ui_dependencies from the manageiq_default gemset
  • use this gemset for all non-ui workers
  • add ui_dependencies for ui_workers and non worker rails processes (runner, console, etc.)
  • Change Procfile example to use ruby not bundle exec rails runner

This is a way to dial back the aggressive "minimal" gemset selection for best benefits with much less risk.

We can continue to try to find solutions to the problems described here and try to whittle away/decouple the dependencies.

The concern is any worker could try to constantize a constant in a gemset such as below. We need a better story for how to deal with that.

[----] I, [2017-08-16T14:15:02.432484 #39682:3fe16d83fa0c]  INFO -- : MIQ(MiqQueue.get) Message id: [1000000001483], MiqServer id: [1000000000001], Zone: [default], Role: [], Server: [], Ident: [generic], Target id: [], Instance id: [], Task id: [], Command: [MiqEvent.raise_evm_event], Timeout: [600], Priority: [100], State: [dequeue], Deliver On: [], Data: [], Args: [["ManageIQ::Providers::Vmware::InfraManager", 1000000000004], "ems_auth_unreachable", {}], Dequeued in: [392.403107] seconds
[----] E, [2017-08-16T14:15:02.952992 #39682:3fe16d83fa0c] ERROR -- : [NameError]: uninitialized constant ManageIQ::Providers::Vmware  Method:[block in method_missing]
[----] E, [2017-08-16T14:15:02.953031 #39682:3fe16d83fa0c] ERROR -- : /Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/inflector/methods.rb:270:in `const_get'
/Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/inflector/methods.rb:270:in `block in constantize'
/Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/inflector/methods.rb:266:in `each'
/Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/inflector/methods.rb:266:in `inject'
/Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/inflector/methods.rb:266:in `constantize'
/Users/joerafaniello/.gem/ruby/2.3.4/gems/activesupport-5.0.5/lib/active_support/core_ext/string/inflections.rb:66:in `constantize'
/Users/joerafaniello/Code/manageiq/app/models/miq_event.rb:30:in `raise_evm_event'
/Users/joerafaniello/Code/manageiq/app/models/miq_queue.rb:387:in `block in deliver'
/Users/joerafaniello/.rubies/ruby-2.3.4/lib/ruby/2.3.0/timeout.rb:91:in `block in timeout'
/Users/joerafaniello/.rubies/ruby-2.3.4/lib/ruby/2.3.0/timeout.rb:33:in `block in catch'
/Users/joerafaniello/.rubies/ruby-2.3.4/lib/ruby/2.3.0/timeout.rb:33:in `catch'
/Users/joerafaniello/.rubies/ruby-2.3.4/lib/ruby/2.3.0/timeout.rb:33:in `catch'
/Users/joerafaniello/.rubies/ruby-2.3.4/lib/ruby/2.3.0/timeout.rb:106:in `timeout'
/Users/joerafaniello/Code/manageiq/app/models/miq_queue.rb:386:in `deliver'

It is the biggest bundler group in terms of loading time, memory, code,
and it's not needed by all non ui worker processes.

Note, we need to set ENV["BUNDLER_GROUPS"] by default in application.rb
so all non worker rails processes (console, runner, etc.) will include
all gemsets by default.
It's the easiest gemset to exclude by default and provides the most
benefits to non-ui workers.
This allows us to take advantage of bundler_groups.
@miq-bot
Copy link
Member

miq-bot commented Aug 16, 2017

Checked commits jrafanie/manageiq@c57b0b7~...cb63b4b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

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

tl; dr; 👍

I like this approach, and much prefer it to my "fix the two things that were bothering me at the time" PR, #15821

While I this will bloat what we have a bit, it won't be so bad because we autoload everything (for better or worse). I think the stability benefits we gain from this and actually launching things in foreman the way I had intended (and doing this before this becomes a heavily used standard for developers) far out way the loss memory locally for now.

That said, we should get a plan in place to start tackling striping this down, but that can be done later.

@jrafanie jrafanie changed the title [WIP] Use ruby not runner for run single worker Use ruby not runner for run single worker Aug 16, 2017
@jrafanie jrafanie added core/workers and removed wip labels Aug 16, 2017
@NickLaMuro
Copy link
Member

@carbonin @gtanzillo When you have time, I think me and Joe have agreed that this is the path we would like to start on for a reset on getting the bundler groups working properly, and having this in place is needed for that.

To avoid a 📖 , I will leave it at that, but let me know if there is anything that is confusing about this approach.

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

Copy link
Member

@gtanzillo gtanzillo 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 👍

@gtanzillo gtanzillo added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 17, 2017
@gtanzillo gtanzillo merged commit dc082ae into ManageIQ:master Aug 17, 2017
@jrafanie jrafanie deleted the use_ruby_not_runner_for_run_single_worker branch August 29, 2017 18:30
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.

None yet

5 participants