-
Notifications
You must be signed in to change notification settings - Fork 125
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 new resource_pools column for cpu_cores_* #708
Add new resource_pools column for cpu_cores_* #708
Conversation
spec/migrations/20231018141450_add_cpu_cores_columns_to_resource_pools_spec.rb
Outdated
Show resolved
Hide resolved
effa691
to
d9a1a0c
Compare
Are there changes to the refresh classes so these share columns will get null going forward? specifically: UPDATE: ok, the updated code just ignores the columns. think that is good. |
add_column :resource_pools, :cpu_cores_limit, :float | ||
|
||
say_with_time('Removing data non-applicable data from IbmCloud::PowerVirtualServer resource pools') do | ||
ResourcePools.in_my_region.where(:type => %w[ManageIQ::Providers::IbmCloud::PowerVirtualServers::CloudManager::ResourcePool]).update_all( |
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 works fine. Thanks for doing this in SQL and not ruby.
I may be misreading, but it looks like you could have merged these 2 clauses into a single query (:type => %w[...IbmCloud... ...IbmPowerHmc...]
(with a space as a delimiter)
But this looks good to me and since the query is quick, merging them will not really buy us much.
db/migrate/20231018141450_add_cpu_cores_columns_to_resource_pools.rb
Outdated
Show resolved
Hide resolved
db/migrate/20231018141450_add_cpu_cores_columns_to_resource_pools.rb
Outdated
Show resolved
Hide resolved
From Pull Request: ManageIQ/manageiq-schema#708
@jaywcarman I fixed the description but can you update the commit message to fix the typos? ( |
d9a1a0c
to
e0029c3
Compare
db/migrate/20231018141450_add_cpu_cores_columns_to_resource_pools.rb
Outdated
Show resolved
Hide resolved
db/migrate/20231018141450_add_cpu_cores_columns_to_resource_pools.rb
Outdated
Show resolved
Hide resolved
spec/migrations/20231018141450_add_cpu_cores_columns_to_resource_pools_spec.rb
Outdated
Show resolved
Hide resolved
The resource_pools table schema follows VMware resource pools. Because of this the 'cpu_reserve', 'cpu_limit', and 'cpu_shares' columns are all integers (representing units of Hz). When IbmCloud::PowerVirtualServers and IbmPowerHmc providers used resource_pools to store information about Shared Processor Pools they stored CPU core shares in these 'cpu_*' columns (even though they are not in Hz). Since there is a unit mis-match the best way to store both in the same table is to create new columns 'cpu_core_*' (note the data type is float since fractional core counts are allowed).
e0029c3
to
3bc329a
Compare
Thanks for the lightnign quick reviews @agrare :) I ran specs locally before pushing this time. |
@jaywcarman I think we discussed this on a call earlier but the cpu_(shares,limit,reserve) are in MHz not in |
Host CPU clock speed is not available through PowerVS. For HMC I don't see it in available in our current SDK schemas but it seems like something we might be able to get. Even if we can collect clock speed, it would be messy because the Power hosts are able to run in modes with variable clock speeds (based on workload and cores available). |
Proposing this instead of #675
Fixes ManageIQ/manageiq-providers-ibm_power_hmc#108
Related to:
The resource_pools table schema follows VMware resource pools. Because of this the 'cpu_reserve', 'cpu_limit', and 'cpu_shares' columns are all integers (representing units of Hz).
When IbmCloud::PowerVirtualServers and IbmPowerHmc providers used resource_pools to store information about Shared Processor Pools they stored CPU core shares in these 'cpu_' columns (even though they are not in Hz). Since there is a unit mis-match the best way to store both in the same table is to create new columns 'cpu_core_' (note the data type is float since fractional core counts are allowed).