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] Remove wait_for_worker_monitor functionality #14257

Closed

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Mar 9, 2017

@carbonin @jrafanie Here is a WIP of killing wait_for_worker_monitor. I'd like to try this out on an appliance with you both.

@Fryguy
Copy link
Member Author

Fryguy commented Mar 22, 2017

@carbonin @jrafanie I was thinking about this some more and we may need to keep it. For example, vmware refresh workers depend on the broker to start before starting. I believe the mechanism they use is the wait_for_worker_monitor functionality.

@miq-bot
Copy link
Member

miq-bot commented May 22, 2017

This pull request is not mergeable. Please rebase and repush.

@Fryguy
Copy link
Member Author

Fryguy commented Jul 25, 2017

Talked to @carbonin on this one, and we've come to realize that the wait_for_worker_monitor functionality has become essentially a wait barrier for dependent workers that is implemented via waiting for MiqServer to transition to started. In particular, when refresh workers need to wait for the broker, but there may be others upon more needed research.

Here is a chart depicting how it all works

    MiqServer      Generic      Broker      Refresh
|
|   starting
                  starting     starting    starting
T    check           𝗫            𝗫           𝗫
I                 started      starting    started-but-waiting-for-server-to-be-started
M    check           ✅            𝗫           ✅
E                 started      started     started-but-waiting-for-server-to-be-started
     check           ✅            ✅           ✅
|   started
|                 started      started     started
V

@Fryguy Fryguy force-pushed the remove_wait_for_worker_monitor branch from befcfde to e2cdcab Compare July 25, 2017 18:00
@miq-bot
Copy link
Member

miq-bot commented Jul 25, 2017

Checked commit Fryguy@e2cdcab with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 0 offenses detected
Everything looks fine. 👍

Fryguy added a commit to Fryguy/manageiq that referenced this pull request Jul 25, 2017
A majority of workers do not need to wait for the server to start, so
changing the default simplifies the code and also identifies which
workers actually need to wait.  In researching this, we found that the
only reason that workers are waiting is to actually wait for the vmware
broker to start and provide its connection pool.  Follow up PRs will
change this from wait_for_worker_monitor to "wait for broker", and we
can further make this more specific to vmware only classes.

See also
ManageIQ#14257 (comment)
for a description of how the wait_for_worker_monitor manifests as a
"wait for broker" scenario.
@miq-bot
Copy link
Member

miq-bot commented Jul 25, 2017

This pull request is not mergeable. Please rebase and repush.

agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Aug 21, 2017
A majority of workers do not need to wait for the server to start, so
changing the default simplifies the code and also identifies which
workers actually need to wait.  In researching this, we found that the
only reason that workers are waiting is to actually wait for the vmware
broker to start and provide its connection pool.  Follow up PRs will
change this from wait_for_worker_monitor to "wait for broker", and we
can further make this more specific to vmware only classes.

See also
ManageIQ/manageiq#14257 (comment)
for a description of how the wait_for_worker_monitor manifests as a
"wait for broker" scenario.

(transferred from ManageIQ/manageiq@9e99ba0)
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Aug 23, 2017
A majority of workers do not need to wait for the server to start, so
changing the default simplifies the code and also identifies which
workers actually need to wait.  In researching this, we found that the
only reason that workers are waiting is to actually wait for the vmware
broker to start and provide its connection pool.  Follow up PRs will
change this from wait_for_worker_monitor to "wait for broker", and we
can further make this more specific to vmware only classes.

See also
ManageIQ/manageiq#14257 (comment)
for a description of how the wait_for_worker_monitor manifests as a
"wait for broker" scenario.

(transferred from ManageIQ/manageiq@9e99ba0)
@carbonin
Copy link
Member

Fairly sure we can close this in favor of #15654

@carbonin carbonin closed this Sep 27, 2017
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Dec 19, 2017
A majority of workers do not need to wait for the server to start, so
changing the default simplifies the code and also identifies which
workers actually need to wait.  In researching this, we found that the
only reason that workers are waiting is to actually wait for the vmware
broker to start and provide its connection pool.  Follow up PRs will
change this from wait_for_worker_monitor to "wait for broker", and we
can further make this more specific to vmware only classes.

See also
ManageIQ/manageiq#14257 (comment)
for a description of how the wait_for_worker_monitor manifests as a
"wait for broker" scenario.

(transferred from ManageIQ/manageiq@9e99ba0)
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Mar 23, 2018
A majority of workers do not need to wait for the server to start, so
changing the default simplifies the code and also identifies which
workers actually need to wait.  In researching this, we found that the
only reason that workers are waiting is to actually wait for the vmware
broker to start and provide its connection pool.  Follow up PRs will
change this from wait_for_worker_monitor to "wait for broker", and we
can further make this more specific to vmware only classes.

See also
ManageIQ/manageiq#14257 (comment)
for a description of how the wait_for_worker_monitor manifests as a
"wait for broker" scenario.

(transferred from ManageIQ/manageiq@9e99ba0)
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.

3 participants