Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-09
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
#openstack-nova - 2018-10-10
00:09:29 openstackgerrit Merged openstack/nova master: api-ref: Replace non UUID string with UUID https://review.openstack.org/608854
00:22:15 openstackgerrit Brin Zhang proposed openstack/nova master: Add compute version 36 to support ``volume_type`` https://review.openstack.org/579360
00:34:16 openstackgerrit iain MacDonnell proposed openstack/nova master: Handle online_data_migrations exceptions https://review.openstack.org/608091
01:17:29 openstackgerrit Sam Morrison proposed openstack/nova master: Allow ability for non admin users to list all flavors. https://review.openstack.org/608474
02:30:06 openstackgerrit Jack Ding proposed openstack/nova master: Add I/O Semaphore to limit concurrent disk ops https://review.openstack.org/609180
02:33:21 openstackgerrit huanhongda proposed openstack/nova master: api-ref: add two tables in the note of DELETE /os-services https://review.openstack.org/609186
03:03:19 openstackgerrit Sam Morrison proposed openstack/nova master: Allow ability for non admin users to list all flavors. https://review.openstack.org/608474
03:26:53 openstackgerrit Brin Zhang proposed openstack/nova master: Add compute API version for when a ``volume_type`` is requested https://review.openstack.org/605573
04:04:45 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (7) https://review.openstack.org/571992
04:05:05 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (8) https://review.openstack.org/571993
04:05:31 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in virt/test_block_device.py https://review.openstack.org/566153
04:27:35 openstackgerrit Sundar Nadathur proposed openstack/nova-specs master: Nova Cyborg interaction specification. https://review.openstack.org/603955
05:55:47 gmann alex_xu hi, will you be there for API office hour ?
05:56:23 alex_xu gmann: yea
05:56:31 gmann cool,
06:01:53 gmann let's start
06:02:03 gmann #startmeeting nova api
06:02:04 openstack Meeting started Wed Oct 10 06:02:03 2018 UTC and is due to finish in 60 minutes. The chair is gmann. Information about MeetBot at http://wiki.debian.org/MeetBot.
06:02:05 openstack Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
06:02:07 openstack The meeting name has been set to 'nova_api'
06:02:38 gmann #link https://wiki.openstack.org/wiki/Meetings/NovaAPI#Agenda_for_next_Office_hours
06:02:41 gmann agenda ^^
06:03:11 gmann i have not got the much time to review for couple of weeks.
06:03:27 gmann #topic API Subteam Tracking
06:03:34 gmann #linkhttps://etherpad.openstack.org/p/stein-nova-subteam-tracking
06:03:51 gmann i added the approved/under review BP related to API on this extherpad
06:04:01 gmann l63
06:04:36 alex_xu currently we have an api bp in the runway
06:04:53 gmann yeah
06:05:35 gmann #link https://etherpad.openstack.org/p/nova-runways-stein
06:06:30 alex_xu yea, the volume type in boot
06:07:09 alex_xu how much we left for extension merge?
06:07:29 gmann alex_xu: link https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/api-extensions-merge-stein+status:open
06:07:34 gmann 3-4 patch i will say.
06:07:40 gmann i need to rebase them though
06:07:43 alex_xu cool

Earlier   Later