Delegate queue_name for refresh to parent manager instead of queue_name arrays #20345

agrare · 2020-07-09T00:38:55Z

Instead of changing the worker queue_name to allow for arrays which has been problematic we could simply delegate the queue_name for refresh to the parent manager if the child managers are targeted and leave the parent manager's RefreshWorker queue_name alone.

Depends on:

Update standalone network managers (Nuage and NSX-t) to not delegate to parent_manager (Don't delegate queue_name to parent_manager manageiq-providers-nuage#217, Don't delegate queue_name to parent_manager manageiq-providers-nsxt#10)

Dependent:

Data migration to clear worker records with json queue names (Delete any worker records that have a JSON queue_name manageiq-schema#496)

Fixes #20307

Fryguy · 2020-07-09T13:54:58Z

Are we still storing JSON in the column, then or will there be a data migration? Since it's using the parent's queue name, how do we report that in rake evm:status (maybe it doesn't matter)?

jrafanie · 2020-07-10T19:32:51Z

Since it's using the parent's queue name, how do we report that in rake evm:status (maybe it doesn't matter)?

Yeah, this would break evm:status since we use worker.queue_name...

EDIT: well, it could break it, depending on what queue_name would return.

agrare · 2020-07-11T16:51:01Z

Sorry might be missing something but why would it break evm:status? worker.queue_name would just be e.g. ems_1 like it used to be.

agrare · 2020-07-13T12:22:39Z

$ rake evm:status/manageiq/bundler.d/overrides.rb:4
** ManageIQ master, codename: Kasparov
Checking EVM status...
The version of PostgreSQL being connected to is incompatible with ManageIQ (10 required)
 Region | Zone    | Server | Status  |   PID |  SPID | Workers | Version | Started     | Heartbeat   | MB Usage | Roles
--------+---------+--------+---------+-------+-------+---------+---------+-------------+-------------+----------+----------------------------------------------------------------------------------------------------------------------------------------------------------
      0 | default | EVM*   | started | 75219 | 75499 |      13 | master  | 12:20:20UTC | 12:21:15UTC |      366 | automate:database_operations:database_owner:ems_inventory:ems_operations:event:remote_console:reporting:scheduler:smartstate:user_interface:web_services

* marks a master appliance

 Type                        | Status  |   PID | SPID  | Queue     | Started     | Heartbeat   | MB Usage
-----------------------------+---------+-------+-------+-----------+-------------+-------------+----------
 Amazon::Cloud::EventCatcher | started | 75902 | 77913 | ems_2     | 12:21:02UTC | 12:21:10UTC | 288/2048
 Amazon::Cloud::Refresh      | started | 76101 | 78032 | ems_2     | 12:21:06UTC | 12:21:12UTC | 323/2048
 EventHandler                | started | 75987 | 77659 | ems       | 12:21:03UTC | 12:21:04UTC | 139/500
 Generic                     | started | 76001 | 77677 | generic   | 12:21:03UTC | 12:21:05UTC | 138/500

jrafanie · 2020-07-13T13:51:20Z

Sorry might be missing something but why would it break evm:status? worker.queue_name would just be e.g. ems_1 like it used to be.

cool! Yeah, I changed my comment afterwards... it wouldn't definitely break it... if it returns a queue_name format that was previously supported, it would work

jrafanie · 2020-07-17T19:24:02Z

@agrare it says it depends on the other PRs, do they need to be merged first? If so, can you mark this WIP and ping me when they're merged?

agrare · 2020-07-17T19:30:56Z

Oh I think "not WIP" implies ready to review not ready to merge, I'll mark it WIP if that's how most people use it

kbrock

Looks good.

I think we may have this as a static method to get away from having to create an ems object for every queue.

But looking at the original code in ems_refresh.rb it looks like that wasn't the intent here.
I remember getting a big performance gain in MiqQueue.put by avoiding the queue lookup.

Maybe something to file away in our minds for future work

agrare · 2020-09-22T21:18:21Z

I'm going to un-WIP this because it is ready for review, just has an associated schema change.

@jrafanie please review

miq-bot · 2020-09-23T14:35:31Z

Checked commit agrare@387abe2 with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
8 files checked, 0 offenses detected
Everything looks fine. 🏆

jrafanie

LGTM

jrafanie · 2020-09-30T19:01:34Z

@Fryguy are you good with this PR and the associated migration (ManageIQ/manageiq-schema#496)?

ManageIQ/manageiq#20345 dropped combined child manager queue names

ems_id comes from ems_id_from_queue_name which as of [1] can only ever return an integer or nil from parse_ems_id. [1] ManageIQ#20345

agrare requested review from Fryguy, jrafanie and kbrock as code owners July 9, 2020 00:38

agrare added wip core/queue providers bug labels Jul 9, 2020

agrare force-pushed the drop_combined_queue_names branch 2 times, most recently from bff6497 to ea51528 Compare July 9, 2020 13:04

agrare force-pushed the drop_combined_queue_names branch from ea51528 to 2ddba64 Compare July 17, 2020 18:46

agrare changed the title ~~[WIP] Delegate queue_name for refresh to parent manager instead of queue_name arrays~~ Delegate queue_name for refresh to parent manager instead of queue_name arrays Jul 17, 2020

miq-bot removed the wip label Jul 17, 2020

agrare changed the title ~~Delegate queue_name for refresh to parent manager instead of queue_name arrays~~ [WIP] Delegate queue_name for refresh to parent manager instead of queue_name arrays Jul 17, 2020

miq-bot added the wip label Jul 17, 2020

miq-bot added the unmergeable label Aug 17, 2020

agrare force-pushed the drop_combined_queue_names branch from 2ddba64 to 99610f8 Compare August 21, 2020 14:58

miq-bot removed the unmergeable label Aug 21, 2020

kbrock approved these changes Aug 21, 2020

View reviewed changes

agrare changed the title ~~[WIP] Delegate queue_name for refresh to parent manager instead of queue_name arrays~~ Delegate queue_name for refresh to parent manager instead of queue_name arrays Sep 22, 2020

miq-bot added unmergeable and removed wip labels Sep 22, 2020

Instead of array of queue names delegate to parent_manager

387abe2

agrare force-pushed the drop_combined_queue_names branch from 99610f8 to 387abe2 Compare September 23, 2020 14:33

agrare removed the unmergeable label Sep 23, 2020

jrafanie approved these changes Sep 30, 2020

View reviewed changes

Fryguy merged commit be9bd91 into ManageIQ:master Oct 2, 2020

Fryguy self-assigned this Oct 2, 2020

agrare deleted the drop_combined_queue_names branch October 2, 2020 15:29

agrare added a commit to agrare/manageiq-providers-amazon that referenced this pull request Oct 3, 2020

Queue name for ems no longer an array

9973198

ManageIQ/manageiq#20345 dropped combined child manager queue names

agrare mentioned this pull request Oct 3, 2020

Queue name for ems no longer an array ManageIQ/manageiq-providers-amazon#656

Merged

agrare added a commit to agrare/manageiq-providers-azure that referenced this pull request Oct 3, 2020

Queue name for ems no longer an array

494fdcc

ManageIQ/manageiq#20345 dropped combined child manager queue names

agrare mentioned this pull request Oct 3, 2020

Queue name for ems no longer an array ManageIQ/manageiq-providers-azure#415

Merged

agrare added a commit to agrare/manageiq-providers-openstack that referenced this pull request Oct 3, 2020

Queue name for ems no longer an array

5767229

ManageIQ/manageiq#20345 dropped combined child manager queue names

agrare mentioned this pull request Oct 3, 2020

Queue name for ems no longer an array ManageIQ/manageiq-providers-openstack#642

Merged

agrare added a commit to agrare/manageiq-providers-openstack that referenced this pull request Oct 4, 2020

Queue name for ems no longer an array

ca4c190

ManageIQ/manageiq#20345 dropped combined child manager queue names

agrare mentioned this pull request Nov 12, 2020

Ensure that options[:ems_id] is an Array when passed as an ENV var #20809

Merged

agrare mentioned this pull request Apr 9, 2021

Simplify refresh of child managers for collapsed worker/manager providers #20301

Open

11 tasks

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 17, 2023

ems_id cannot be an array, simplifying this logic

bd73287

ems_id comes from ems_id_from_queue_name which as of [1] can only ever return an integer or nil from parse_ems_id. [1] ManageIQ#20345

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 17, 2023

ems_id cannot be an array, simplifying this logic

f0715d8

ems_id comes from ems_id_from_queue_name which as of [1] can only ever return an integer or nil from parse_ems_id. [1] ManageIQ#20345

jrafanie mentioned this pull request Mar 17, 2023

Don't leave trailing hyphen in deployment name #22419

Merged

agrare mentioned this pull request Mar 20, 2023

Delegate Provisioning refresh to Configuration ManageIQ/manageiq-providers-foreman#113

Merged

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 21, 2023

ems_id cannot be an array, simplifying this logic

29dd755

ems_id comes from ems_id_from_queue_name which as of [1] can only ever return an integer or nil from parse_ems_id. [1] ManageIQ#20345

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 29, 2023

ems_id cannot be an array, simplifying this logic

0686ffd

ems_id comes from ems_id_from_queue_name which as of [1] can only ever return an integer or nil from parse_ems_id. [1] ManageIQ#20345

agrare mentioned this pull request Jul 25, 2024

Worker EMS_ID can no longer be an array #23109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delegate queue_name for refresh to parent manager instead of queue_name arrays #20345

Delegate queue_name for refresh to parent manager instead of queue_name arrays #20345

agrare commented Jul 9, 2020 •

edited by Fryguy

Loading

Fryguy commented Jul 9, 2020

jrafanie commented Jul 10, 2020 •

edited

Loading

agrare commented Jul 11, 2020 •

edited

Loading

agrare commented Jul 13, 2020

jrafanie commented Jul 13, 2020

jrafanie commented Jul 17, 2020

agrare commented Jul 17, 2020 •

edited

Loading

kbrock left a comment

agrare commented Sep 22, 2020

miq-bot commented Sep 23, 2020

jrafanie left a comment

jrafanie commented Sep 30, 2020

Delegate queue_name for refresh to parent manager instead of queue_name arrays #20345

Delegate queue_name for refresh to parent manager instead of queue_name arrays #20345

Conversation

agrare commented Jul 9, 2020 • edited by Fryguy Loading

Fryguy commented Jul 9, 2020

jrafanie commented Jul 10, 2020 • edited Loading

agrare commented Jul 11, 2020 • edited Loading

agrare commented Jul 13, 2020

jrafanie commented Jul 13, 2020

jrafanie commented Jul 17, 2020

agrare commented Jul 17, 2020 • edited Loading

kbrock left a comment

Choose a reason for hiding this comment

agrare commented Sep 22, 2020

miq-bot commented Sep 23, 2020

jrafanie left a comment

Choose a reason for hiding this comment

jrafanie commented Sep 30, 2020

agrare commented Jul 9, 2020 •

edited by Fryguy

Loading

jrafanie commented Jul 10, 2020 •

edited

Loading

agrare commented Jul 11, 2020 •

edited

Loading

agrare commented Jul 17, 2020 •

edited

Loading