Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-09
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
22:28:21 mriedem several hours ago
22:28:31 mriedem and it sounds like you guys are talking about it all over again to come back to the same conclusion?
22:28:55 sean-k-mooney no there was a vlaid case to return 1 before
22:29:20 mriedem "(11:25:34 AM) dansmith: what if 2 means "I didn't do anything but there were exceptions", 1 means "I did things, maybe there were some exceptions too", 0 means "I didn't do anything, but no errors""
22:29:30 imacdonn mriedem: the sticking point now is the meaning of '1'
22:29:33 sean-k-mooney we would have return one if we set a max count and there were more then max count migrations
22:30:27 imacdonn mriedem: In the current implementation, if it runs through all migrations (in batches), the final return code is 0, even though work was done
22:30:57 sean-k-mooney mriedem: so if there were 100 migrations to run and we set --max-count=90 before and all 90 ran sucessfully we would have retruned 1 to indicate there are more to run
22:31:02 imacdonn in my interpretation of the discussion this morning, it should be '1' if work was done, even if there is no work remaining to do
22:32:14 sean-k-mooney mriedem: 0 used to mean i ran all migrtions sucessfuly since 0 is sucess on bash
22:33:20 mriedem sorry but i'm past the point of having attention to think about this today
22:33:56 sean-k-mooney mriedem: no worries am ill leave a comment in the patch with what i understand the current logic is and you and dan can check when ye have had some rest
22:34:18 sean-k-mooney imacdonn: are you ok with waiting for them to look at this tomorrow?
22:34:44 imacdonn sean-k-mooney: sure
23:22:49 openstackgerrit sean mooney proposed openstack/nova master: add get_pci_requests_from_vifs to request.py https://review.openstack.org/609166
23:56:48 openstackgerrit Chris Friesen proposed openstack/nova-specs master: Add support for emulated virtual TPM https://review.openstack.org/571111

Earlier   Later