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

Add save inventory thread for streaming refresh #233

Merged
merged 1 commit into from
Apr 24, 2018

Conversation

agrare
Copy link
Member

@agrare agrare commented Apr 20, 2018

This change adds a save inventory thread to help mitigate issues with lots of small updates overwhelm saving.

TODO: I'd like to concatenate the inventory collections which were queued so that e.g. hundreds of single changes are handled by a single refresh.

Depends:

@agrare agrare force-pushed the save_inventory_thread branch 2 times, most recently from fe4cb19 to 61c3536 Compare April 21, 2018 11:37
@agrare agrare mentioned this pull request Apr 21, 2018
3 tasks
@Ladas
Copy link
Contributor

Ladas commented Apr 23, 2018

@agrare dep merged, please rebase

we can talk later about the IC concat. To isolate the functionality, I would probably lean to Concatenate class, that would work with serialized ICs (Hash, parsed out of json). At this point it would be just merging of the hashes. What do you think?

@agrare
Copy link
Member Author

agrare commented Apr 23, 2018

@Ladas okay rebased

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

extracting the saver is nice. adds some complexity, but it gives us a long desired need of merging duplicate refresh requests.

The saver is good as long as it is intended to be single threaded. And adding a mutex around ensure_saver_thread would make it thread safe. (overkill for now - and would just slow us down in the single threaded case)

@miq-bot
Copy link
Member

miq-bot commented Apr 23, 2018

Checked commit agrare@808284b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 2 offenses detected

app/models/manageiq/providers/vmware/infra_manager/inventory/saver.rb


def dequeue
queue.deq(:non_block => true)
rescue ThreadError
Copy link
Contributor

Choose a reason for hiding this comment

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

probably some log here would be nice? we can do it in next PR

Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

Looks great

@Ladas Ladas merged commit b86a5e8 into ManageIQ:master Apr 24, 2018
@Ladas Ladas added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 24, 2018
@agrare agrare deleted the save_inventory_thread branch April 24, 2018 12:09
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
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.

4 participants