Skip to content

Reduce nonsence value for capacity.skipcounting.hours#3961

Closed
andrijapanicsb wants to merge 1 commit intomasterfrom
lower-capacity-skipcounting-hours-setting
Closed

Reduce nonsence value for capacity.skipcounting.hours#3961
andrijapanicsb wants to merge 1 commit intomasterfrom
lower-capacity-skipcounting-hours-setting

Conversation

@andrijapanicsb
Copy link
Copy Markdown
Contributor

@andrijapanicsb andrijapanicsb commented Mar 11, 2020

When doing rapid host maintenance one after the other, and when previously stopping a huge number of VMs on a single host (i.e. all VMs on that host are stopped) - no new VMs can be started on this empty host since those VMs' capacities are reserved for 3600sec - so when doing maintenance of another host, VMs can't be migrated to this empty host.

Not sure who and why has this setting default value to such high value (1h) (doesn't make sense)
Not to mention the name of the setting is wrong, should be "capacity.skipcounting.second"

Another PR to rename this setting is most welcome @nvazquez @DaanHoogland @weizhouapache

@nvazquez nvazquez mentioned this pull request Mar 11, 2020
5 tasks
@weizhouapache
Copy link
Copy Markdown
Member

@andrijapanicsb is is better to remove this configuration from database and add new config via Configkey ?

@andrijapanicsb
Copy link
Copy Markdown
Contributor Author

@weizhouapache if it doesn't exists in config key form, I guess that's OK? How about #3960

@weizhouapache
Copy link
Copy Markdown
Member

@weizhouapache if it doesn't exists in config key form, I guess that's OK? How about #3960

@andrijapanicsb I think these are different.
max.retries was added in 4.13->4.14, since 4.14 is not released yet, we do not need extra sql change if we rename it.
capacity.skipcounting.hours was added quite long ago (before 2013), so extra sql change is needed if we rename it. we have two options (1) rename it in sql (2) delete it in sql and add new ConfigKey.

@andrijapanicsb
Copy link
Copy Markdown
Contributor Author

sorry @weizhouapache my bad (read something wrong...) - so whatever works for you - if the config key-based addition of this one is not there, yes please (don't expect me to know how to add config key :) - so feel free to edit my PR please?

@andrijapanicsb
Copy link
Copy Markdown
Contributor Author

@weizhouapache I was expecting a PR from @nvazquez to rename this setting to xxx..seconds (not hours) - that's what I was mentioning (linked to wrong PR)... so we can put everything in a single PR?

@DaanHoogland
Copy link
Copy Markdown
Contributor

👎 no config key should be added in this way

@andrijapanicsb
Copy link
Copy Markdown
Contributor Author

Closed due to not being able to accomplish the full goal - as discussed in #3942

@andrijapanicsb andrijapanicsb deleted the lower-capacity-skipcounting-hours-setting branch April 3, 2020 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants