Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-09
20:18:37 sean-k-mooney 0.0 also correct
20:20:09 sean-k-mooney mriedem: if the resouce provide exists and we get 0.0 from the db but placemetn has another vaule i would assuem we shoudl keep the placement value but not sure if that case can happen today
20:21:10 sean-k-mooney the compute node will just override placement with the value it gets from the resouce tracker today in update provider tree right?
20:25:04 mriedem the allocation ratio in placement can't be 0.0
20:25:09 mriedem it will literally shit itself
20:26:16 cfriesen I assume it
20:26:21 cfriesen it's a divide by zero thing?
20:26:30 sean-k-mooney mriedem: ok in that case the logic is simple. on upgrade if placement provider exits and db value is 0 set db value to placement value. if not placement provider exeits set db to intiall_* value and create provider as normal
20:26:31 cfriesen should placement check for that?
20:27:20 openstackgerrit iain MacDonnell proposed openstack/nova master: Handle online_data_migrations exceptions https://review.openstack.org/608091
20:27:36 sean-k-mooney cfriesen: its not actully a device by 0 but we multiply the available capasity by 0 and see if its larger then what we requested
20:28:05 sean-k-mooney cfriesen: so placement will not have a math error but you wont be able to get allocation against that resouce provider ever
20:28:12 cfriesen ah, thanks
20:50:31 efried mriedem: We talked about this in the sched meeting yesterday. jaypipes said he was about ready to abandon those two specs. We also discussed the possibility of generic inventory yaml leading to a solution.
20:51:21 mriedem efried: i know i was there and said i'd reach out to yikun to pick up the specs,
20:51:26 mriedem but he's confused about the direction, as am i
20:51:28 mriedem hence questions
20:51:43 mriedem i'm going through https://review.openstack.org/#/c/552105/ again now,
20:51:50 mriedem some of that is outdated given https://github.com/openstack/nova/commit/2588af87c862cfd02d860f6b860381e907b279ff
21:07:52 mriedem alright i've dumped comments in https://review.openstack.org/#/c/552105/ - i think i could probably update the spec at this point to cover the upgrade impact
21:08:11 mriedem i don't think we should leave the existing options defaulting to 0.0 like bauzas is asking for - that just prolongs the confusion of what those defaults mean
21:08:31 mriedem jaypipes: if you can skim my comments to see if they make sense i can take over updating the spec
21:20:49 sean-k-mooney mriedem: are you proposing defaulting them to None or 16.0/ what ever the real default is for that resource ?
21:21:08 mriedem what the spec says
21:21:21 mriedem change the *_allocation_ratio defaults from 0.0 to None
21:21:40 mriedem the initial_*_allocation_ratio defaults become what is in the ComputeNode object facade today
21:21:48 mriedem and we drop the facade
21:22:13 sean-k-mooney right that makes sense to me and inital_*_allocation_ratios will have per resouce type defaults correct
21:22:26 mriedem yes
21:22:53 sean-k-mooney ya that all sound sane to me. i have not read bauzas comment arguing for keeping 0.0
21:23:30 sean-k-mooney current 0.0 has a special meaning right? e.g. use schduler/conductor values not compute node os somthing like that
21:24:02 sean-k-mooney i assume that is what his comment was related too.
21:26:02 sean-k-mooney the nova-status check makes sense but im not sure you can check the config as part of it
21:27:15 mriedem if the config explicitly sets the *_allocation_ratios to 0.0 when we have changed the defaults to None, that means their config mgmt system is setting that on purpose and is likely busted
21:29:03 sean-k-mooney mriedem: ture i just was thinking for FFU or in general the nova status command cant check the config on each compute unless you ran it on each compute
21:30:33 sean-k-mooney that said if you use oslo configs ablity to auto generate configs does it generate the config with all the values commeted out or set to there default. just trying to think if there was a resonable reason why it might be set to 0.0
21:30:46 mriedem the allocation ratios aren't read on control plane services, so i think it's reasonable to assume if someone's config said 0.0 for those values when the defaults are None they are just setting the config globally and it's wrong
21:34:13 sean-k-mooney sorry i thnk i missed that bit where will these new values be read? scheduler/condctor or compute node
21:35:16 sean-k-mooney i had assuemed this was all config that was being read by the compute node?
21:38:19 mriedem the compute is what sets it
21:38:23 mriedem the scheduler will read it
21:38:26 mriedem from the compute node object
21:39:32 mriedem the only service that reads the config for these options is the compute service
21:41:35 openstackgerrit Adam Harwell proposed openstack/nova master: Add apply_cells to nova-manage https://review.openstack.org/568987
21:42:14 sean-k-mooney mriedem: oh ok i was under the impression if the sechduler recived a 0.0 from the compute node it would read its own config and use the allocation ratio it got. was that how it used to work or am i just imagining things.
21:43:42 sean-k-mooney mriedem: by in anycase i think the nova-status check is sufficent. if there is a 0.0 in the db for a value the operator should first update there config and then upgrate/run online migration whatever is needed
21:44:40 sean-k-mooney i.e. im agreeing with your suggestion thanks for explaining :)
21:48:07 imacdonn mriedem dansmith: Fixing the migrations thing made grenade go boom ... there actually was another latent bug that I stumbled on, which was causing the exit code to be zero even though work had been done - with that fixed, grenade is not doing the right thing (repeating until exit status 0)
21:48:45 sean-k-mooney imacdonn: what is the other bug?
21:49:39 mriedem jaypipes: yikun: i've also gone through https://review.openstack.org/#/c/544683/ and left comments; i'm not on board with all it's proposing, but i think some of that is outdated now per the ptg discussions
21:49:44 imacdonn sean-k-mooney: "ran" gets reset to zero for each iteration of the loop here: https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L718
21:49:55 mriedem i think the gist of ^ is that it's proposing to proxy aggregate allocation ratio metadata to placement, correct?
21:50:26 imacdonn sean-k-mooney: then it gets used later to determine if any migrations were ran/run at all here: https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L739
21:51:08 imacdonn sean-k-mooney: so if the last iteration of the loop didn't do anything (even though a previous iteration did), it'd not count
21:52:05 sean-k-mooney imacdonn im not sure that is incorrect. if the last iteration did nothing it means migrations was empty so we break out of the while
21:52:36 imacdonn sean-k-mooney: yes, but the decision about whether or not any work had been done needs to consider ALL iterations of the loop
21:53:14 sean-k-mooney does it? why?
21:53:34 imacdonn sean-k-mooney: because, IIUC, that's what exit code 1 means (some migrations did work)
21:55:26 imacdonn I interpret that as "if ran is not zero, return 1, otherwise return 0"
21:56:34 sean-k-mooney imacdonn: yes but im trying to think what does that logically mean
21:56:58 sean-k-mooney is return 0 been used to indicate sucess like in bash or does 1 indicate sucess
21:58:05 imacdonn sean-k-mooney: it's complicated :) (again, IIUC)... zero means that there is no migration work remaining to be done, 1 means "some migrations did work, and there may be some more work that still needs to be done:
21:58:38 imacdonn so you're supposed to keep running the command until it doesn't return 1
21:58:51 sean-k-mooney imacdonn: right in that cae you want ran to be 0 when all pending migrations have been processed so you want to reset it in the loop
21:59:49 imacdonn sean-k-mooney: no... you're supposed to re-run the command ... that's not what that loop is for
22:00:30 sean-k-mooney that is not how i read how it is currently written
22:01:13 sean-k-mooney from the current code it looks like its intent is to run all migration in batches up to max count and then exit when there are none left
22:02:09 imacdonn sean-k-mooney: yes, but there are scenarios where some of them will not work the first time (due to dependencies on others), so oit'
22:02:24 imacdonn it's necessary to iterate the whole command until you get a 0
22:02:32 imacdonn .... is what I've been led to understand
22:02:45 imacdonn see comments on https://review.openstack.org/#/c/608091/
22:04:21 sean-k-mooney this comment https://review.openstack.org/#/c/608091/1/nova/cmd/manage.py@753 or another one
22:04:56 imacdonn yes, those
22:05:01 sorrison mriedem I have another fun policy change https://review.openstack.org/#/c/608474/
22:06:44 sorrison mriedem: I've been trying to figure out the whole admin/user context thing with tests and requests. I think I'm missing something
22:06:47 sean-k-mooney so from mriedem comment we did not raise exceptions before so while we said we coudl exit with a 1 error code we did not
22:08:04 imacdonn sean-k-mooney: I'm still not completely clear on what the exit code should be ... I thought I understood that '1' means "some migrations did work"
22:11:01 sean-k-mooney i think for mriedem comment 1 means some migrations ran but there were no errors and he was suggesting 2 for some migrations ran and there were errros and 0 means all migration ran and no errors
22:11:48 imacdonn I'm not sure how to distinguish between "some ran" and "all ran"
22:12:40 sean-k-mooney imacdonn: if you specify make count before your change and you havde more migration pending then max count it would have exited with 1
22:13:19 sean-k-mooney so just change the retur on line 753 to 2 then be correct but we need a docs update to say what 2 means
22:14:32 imacdonn sean-k-mooney: the suggestion from today was to change it to return 2 *only if no migrations did work this time*
22:14:56 sean-k-mooney when --max-count is set unlimited is set to false so we break out of the loop on line 735 and ran is non 0 so " retrun ran and 1 or 0" returns 1
22:16:26 sean-k-mooney imacdonn: ok in that case on line 734 add if got_exceptions: return 2
22:17:53 sean-k-mooney infact you could do the exception check on line 725 if you really wanted to exit fast
22:18:09 imacdonn that would be bad
22:18:29 imacdonn because some other migration may be a dependency for the first one to not fail, and you'd never get to run the second one
22:19:13 sean-k-mooney you jsut said you want to exit if there are excpetions
22:19:27 imacdonn nope, pretty sure I didn't
22:19:49 imacdonn I said they want the final exit status to be 2, if there were exceptions AND no migrations did any work
22:20:10 sean-k-mooney oh "*only if no migrations did work this time*" i misread that
22:20:20 mriedem sorrison: comments inline
22:21:01 imacdonn I guess "did work" is a confusing term ... "took effect"? ......
22:21:16 sean-k-mooney mriedem: so we are gueessing about what you want return 2 to mean for https://review.openstack.org/#/c/608091/1/nova/cmd/manage.py@753
22:21:29 sorrison Thanks mriedem :-)
22:21:34 sean-k-mooney mriedem: since your hear can you clarify for imacdonn
22:23:33 sean-k-mooney mriedem: based on the current code return 1 could have been returned if we passed --max-count and we had more the max-count migration so i think retrun 1 means some migrations ran with no errors and there are more to run and return 2 should be mean there were errors when runign migrations you better check out what went wrong
22:26:59 imacdonn sean-k-mooney: per dansmith, we should only return 2 if there were exceptions *and* we've determined that no other migrations did work (i.e. modified rows)
22:28:14 sean-k-mooney imacdonn: i dont see that in his comment on the review. was that in the schduler meeting?
22:28:19 mriedem it was in channel

Earlier   Later