Earlier  
Posted Nick Remark
#openstack-nova - 2017-05-25
15:07:49 mriedem sdague: all api changes need a spec
15:07:53 mriedem thems the rules
15:09:17 mriedem smcginnis: so what kind of fun dual maintenance path is it going to be for os-brick in both python and go?
15:09:31 sdague mriedem: ok, let me throw together something quick
15:10:51 mriedem jaypipes: isn't this vestigial now? https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L800
15:11:49 mriedem added in https://github.com/openstack/nova/commit/8941b45bc58a833261bb09fc7eb8fb5c765e154a
15:12:18 mriedem wondering b/c of https://github.com/openstack/nova/commit/ea4754ba21ea08eb367d62bc46b5eba3777c119c
15:12:23 jaypipes mriedem: yeah. the comment can be removed.
15:20:49 smcginnis mriedem: Eh, it's inevitable.
15:24:28 mriedem hmm, i've got questions about compute nodes and services records
15:24:34 mriedem b/c something isn't adding up
15:25:44 mriedem deleting a nova-compute service record doesn't delete the compute_nodes record,
15:26:07 mriedem but if you restart the nova-compute process, it's going to create a new services table record for the same host/binary/topic
15:26:15 mriedem the compute_nodes table entry has a service_id record in it
15:26:33 mriedem the resource tracker isn't going to create a new compute_nodes record because one already exists for the given host and nodename
15:26:43 mriedem but, do we ever update the compute_nodes.service_id record anywhere?
15:27:46 openstackgerrit Sean Dague proposed openstack/nova-specs master: oslo.middleware request_id spec https://review.openstack.org/468066
15:27:52 sdague mriedem: ok, spec ^^^
15:28:17 mriedem ok
15:30:19 mriedem jaypipes: do we actually not tie the compute nodes record to the nova-compute service anywhere?
15:31:31 mriedem ah looks like it's not, by design
15:31:34 mriedem since it's all deprecated
15:35:53 mriedem oh snap
15:36:06 mriedem alex_xu: we do soft delete the compute node record when the service is deleted,
15:36:09 mriedem if they are tied together,
15:36:15 mriedem which happens for very old compute nodes records
15:36:52 mriedem https://github.com/openstack/nova/blob/65402e1aeb770d077620f2935a0cb305474aa7a1/nova/db/sqlalchemy/api.py#L461
15:37:15 jaypipes mriedem: we removed the direct linkage between the two on purpose
15:37:27 mriedem https://github.com/openstack/nova/blob/65402e1aeb770d077620f2935a0cb305474aa7a1/nova/db/sqlalchemy/api.py#L461
15:37:28 mriedem yeah
15:37:37 mriedem the db api tests are still doing the very old style linkage
15:37:47 mriedem so testing using those for a modern setup is really confusing
15:46:15 jaypipes edleafe: k, sorry about that. ready now.
15:46:16 jaypipes hangout?
15:51:20 mriedem sdague: -1 is specifically for expanding on this https://review.openstack.org/#/c/468066/1/specs/pike/approved/oslo-middleware-request-id.rst@48
15:51:23 mriedem otherwise +2
15:52:01 sdague mriedem: so, it's not sufficient that the patch already has that in it?
15:52:12 mriedem the spec is a doc
15:52:22 sdague sure
15:52:24 mriedem published as a doc, people reading those later shouldn't have to dig through lp to find the patch
15:52:55 edleafe jaypipes: cdent: so you suggested passing the instance UUIDs in the reqspec
15:53:20 edleafe That was my original approach, but dansmith preferred the RPC parameter approach, so I changed it
15:53:38 edleafe jaypipes: see https://review.openstack.org/#/c/463623/
15:53:45 jaypipes edleafe: right.
15:54:16 jaypipes edleafe: lemme walk through in my head whether the recent "continuing the automatic reschedule behaviour" thing affects my decision on that...
15:54:18 cdent edleafe: I don't really have a strong opinion. my natural inclination is parameters, but that's not been seasoned by years of nova-based RPC
15:55:07 edleafe jaypipes: cdent: my preference was reqspec (which is why I did it that way first), since it already had the pretty much useless 'instance_uuid' attribute
15:55:08 openstackgerrit Sean Dague proposed openstack/nova-specs master: oslo.middleware request_id spec https://review.openstack.org/468066
15:55:13 jaypipes dansmith: can you, in one sentence, explain your preference to change the RPC interface vs. add a field to the reqspec object?
15:55:21 sdague mriedem: done
15:55:35 jaypipes dansmith: I want to make sure I understand you completely.
15:56:00 dansmith jaypipes: we don't need to store the uuids in the database, right?
15:56:22 dansmith we've had this conversation a couple times
15:56:44 edleafe dansmith: but there is already the 'instance_uuid', which we don't store either
15:57:46 dansmith edleafe: um, what?
15:57:53 dansmith already instance_uuid in the reqspec?
15:58:00 edleafe yup
15:58:33 edleafe dansmith: https://review.openstack.org/#/c/463623/5/nova/objects/request_spec.py@77
15:58:38 dansmith what do you mean we don't store it
15:58:39 dansmith ?
15:58:49 dansmith ...right, I know it's in there
15:58:54 edleafe well, we don't *need* to store it
15:59:12 edleafe it's useless if num_instances > 1
15:59:15 dansmith okay not sure what that has to do with not adding another thing we don't need to store, which could be a hundred uuids :)
15:59:31 mriedem fuel on the fire makes it better right?
15:59:50 mriedem at what point do we add kitchen_sink to request spec?
16:00:21 edleafe I understand the reqspec to be the vehicle for passing the requested info to the scheduler
16:00:28 edleafe the scheduler needs this info
16:01:28 jaypipes dansmith: so the reason I was advocating passing in all the instance uuids in the reqspec was because I wanted to get rid of the retry stuff and do all of that in the scheduler (and thus we'd need all those UUIDs). But now that that is off the table, I'm not sure it makes sense any more.
16:01:50 dansmith aye
16:01:52 edleafe jaypipes: it definitely does
16:02:22 edleafe if num_instances > 1, how will the scheduler claim the resources if it doesn't know the consumer_id?
16:02:38 dansmith the uuid list is only something that makes sense for a single call to the scheduler, AFAIK, so it shouldn't be stuffed into the database, just something we pass like anything else, IMHO
16:03:19 jaypipes edleafe: a fair point.
16:03:52 jaypipes edleafe: if we moved the handling of num_instances > 1 to the conductor, though...
16:04:23 jaypipes edleafe: we'd need more robust concurrent update handling in the conductor, then.
16:04:24 edleafe jaypipes: sp the conductor would make num_instances calls to the scheduler, instead of 1?
16:04:32 jaypipes edleafe: I'm spitballing.
16:04:49 edleafe We'd also have to run the host queries num_instances times instead of once
16:05:05 jaypipes edleafe: right.
16:05:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Exclude deleted service records when calling hypervisor statistics https://review.openstack.org/467220
16:05:16 edleafe you're spitballing; I'm poking holes
16:05:19 edleafe :)
16:05:24 mriedem Kevin_Zheng: ^ i updated the commit message and re-wrote the test a bit
16:05:25 jaypipes edleafe: I'm thinking out loud.
16:05:29 mriedem with a bunch of comments to explain it
16:05:45 edleafe me too
16:05:58 edleafe I've thought about these a lot already, though :)
16:06:07 sdague mriedem: I'm confused about your comment on https://review.openstack.org/#/c/467995/ about implications for versioned objects
16:06:28 sdague maybe that's where we need dansmith poking
16:07:39 openstackgerrit Ed Leafe proposed openstack/nova master: Pass the number of hosts to return explicitly https://review.openstack.org/465767
16:07:40 openstackgerrit Ed Leafe proposed openstack/nova master: Remove selecting host based on host_subset_size https://review.openstack.org/465174
16:07:40 openstackgerrit Ed Leafe proposed openstack/nova master: Return multiple hosts from select_destinations https://review.openstack.org/465768
16:08:01 edleafe jaypipes: ^^ The series is ready for review. I can switch the passing of UUIDs if needed
16:08:08 jaypipes edleafe, dansmith: for retries, do we ever retry the entire list of instances (in a num_instances > 1 case) or do we just retry the launch of one of them?
16:08:36 edleafe jaypipes: if we retry the whole list, that would be very dumb
16:08:52 edleafe since some could have already been successful
16:09:00 jaypipes edleafe: well, it all depends on how "atomic" we view the num_instances > 1 case, right?
16:10:05 dansmith jaypipes: we've lost scope of the others by the time we're retrying any of them
16:10:11 dansmith jaypipes: and they might've scattered across cells

Earlier   Later