-
Notifications
You must be signed in to change notification settings - Fork 19
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
add refresh after creating manager #181
add refresh after creating manager #181
Conversation
Checked commit Autosde@91f2fda with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lauferin were you running with rails s + simulate_queue_worker
or the full appliance when you saw refresh taking forever?
When a refresh worker is started (which will happen on a real appliance or podified install) a refresh is queued right away before processing any work: https://github.com/ManageIQ/manageiq/blob/master/app/models/manageiq/providers/base_manager/refresh_worker/runner.rb#L14
This should be within a few seconds of adding the provider.
@@ -184,6 +184,7 @@ def connect(options = {}) | |||
host = options[:host] || address | |||
port = options[:port] || self.port | |||
self.class.raw_connect(username, password, host, port).login | |||
EmsRefresh.queue_refresh(ext_management_system) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a great place for this, connect is called in a lot of places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I paid attention to that, but I thought maybe for all those cases a refresh would fit well.
But if not, would it be preferable to do it 'after_save'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understand that after_save is not option (I tried and saw an infinite loop...). Where would you recommend it then? I don't see anywhere else in the file where this could fit.
It happens with rails and queue worker and also when not stopping the server at all. It doesn't take forever, but it can take up to one minute approximately. |
I tried again in both ways and I insist, it usually takes more or less a minute... I'll be happy to show you on live if you want :-) (have you checked?) |
@Lauferin so just so I understand, if you run on an appliance or with I tried running this locally with
It might take the first refresh more than a minute to finish but once the worker is started the first refresh should be queued and started in seconds. If the issue is that the first refresh takes a bit to complete, queueing another isn't going to do anything to help that. |
Until now, when adding a storage manager sometimes it took a long time until an automatic refresh happened, and this caused that the user wasn't able to add the type system of a new physical storage until a manual refresh.
So a refresh was added directly after connecting to the storage manager.