Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-22
21:30:01 mriedem aneurysm? definitely.
21:30:31 melwitt there's a note in the code saying that if it's a reschedule, it's already been set to a cell on the first schedule attempt https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L717-L718
21:30:57 melwitt so how can the first schedule attempt in a cells v2 env ever be calling build_instances (and not schedule_and_build_instances)
21:30:58 mriedem melwitt: right, so that if is False
21:31:26 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L721 is the first time through for cells v1
21:31:34 mriedem after picking a host, need to update the instance mapping with the cell of the selectd host
21:31:44 mriedem else you are rescheduling https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L738
21:32:04 melwitt I see that, but we need to update instance mapping for cells v1?
21:32:32 mriedem what we need to do is delete cells v1
21:32:38 melwitt yes we do
21:33:01 mriedem for cells v1 i assume the instance mapping is updated for the same reason as v2 - to know where to pull the instance information on a GET
21:33:17 mriedem because if it's not in the mapping, we'd pull from the top level API DB i geuss which has the synced info
21:33:23 melwitt yeah, because code path is the same? yeah, guh
21:33:42 mriedem https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2449
21:33:49 mriedem we don't look at the mapping in that case
21:34:02 mriedem so idk
21:34:05 melwitt ok. just going through this realizing I'm not going to have a "populate instance mapping user_id" opportunity during a reschedule, at least not from any already existing instance mapping update
21:34:08 mriedem maybe it's for the v1 -> v2 transition
21:34:33 mriedem why would you need to update the user_id during a reschedule?
21:34:35 melwitt yeah, must be
21:34:59 mriedem we only reschedule (from compute) in 2 cases, server create and resize
21:35:02 melwitt I don't have to, but I had been thinking use all the opportunities for setting user_id if it's not set
21:35:10 mriedem in either of those cases, you could have already updated the instance mapping at the top
21:35:52 mriedem i.e. https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L167
21:36:02 melwitt yeah, once the new host is picked and saved, that should be where. I guess I missed that mapping update
21:36:31 mriedem the host shouldn't have anything to do with the instance mapping...
21:36:38 mriedem you're just looking for places that InstanceMapping.save() happens yes?
21:37:13 mriedem hell you could do it right here and hit all move operations https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L75
21:37:19 melwitt oh wait no, during a reschedule we wouldn't update the mapping bc that's just for the cell, not the host. gah I am just confusing myself
21:37:20 melwitt yes
21:38:00 melwitt yeah, I was piggy backing on any existing save. there won't be a new save for a reschedule, is what I realized
21:38:31 mriedem nor any move as far as i know, unless you add one to ^
21:38:31 melwitt which I could just punt on, but that code in build_instances made me wonder what it was for
21:39:47 melwitt yeah
21:40:32 melwitt initially I thought, since I saw "populate_instance_mapping" in build_instances, that there was a save() to piggy back. but looking closer, there isn't
21:40:46 melwitt not a big deal
22:21:18 melwitt I wonder if I could use request specs to populate user_id instead of looking in cells at all? hmmm
22:22:42 sean-k-mooney melwitt: https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L65-L66
22:22:51 sean-k-mooney the user and project ides are stored in it
22:23:10 melwitt yeah, I was noticing that in compute_api.get()
22:24:22 melwitt I'm a bit wary if there's any way it couldn't be relied on though. being that request specs are only removed at archive_deleted_rows time, I wonder if it's possible that old, unarchived things could have a null user_id
22:24:52 melwitt maybe best not to chance it
22:25:43 sean-k-mooney i honest taught the scduler depending on this to look up the users limets
22:26:21 melwitt it looks at build requests, in addition to instances
22:26:59 melwitt which are ephemeral (only live until scheduling completes)
22:27:25 melwitt request specs are not looked at for any quota limit checking
22:28:00 sean-k-mooney hum ok
22:29:03 sean-k-mooney are you concured that the user may be delete and we some how have a non archived instance belonving to that user and try to look it up and get an error
22:29:21 sean-k-mooney or are you concured that the request spec will have a null user id
22:30:23 melwitt null user_id, or worse, no user_id field. I didn't look at the history of when that field was added
22:30:50 melwitt I guess "no user_id field" isn't a thing, it would be null in that case
22:31:32 sean-k-mooney melwitt: https://github.com/openstack/nova/commit/6e49019fae80586c4bbb8a7281600cf6140c176a
22:31:46 melwitt request spec has a wild west reputation, so I'm just not sure if I could rely on its user_id field. being that I didn't dig into it yet
22:32:18 melwitt ah, wow. relatively new
22:32:37 sean-k-mooney well its been there since queens?
22:32:52 melwitt and no data migration, so would be null for already existing request specs when that code landed
22:33:19 melwitt yeah, I just expected it would be older than that
22:33:30 melwitt ok, nevermind my request spec flighty idea
22:34:35 sean-k-mooney well looking at the change it look like there are check in the conductor that depend on both the user id and porject id being set
22:34:56 mriedem fried_rice: well, i got something working for the functional regression test at least
22:35:00 melwitt hah, ok
22:35:05 mriedem of course it's rife with TODOs and FIXMEs
22:35:09 fried_rice neat.
22:35:17 sean-k-mooney i wasnt following hte context of what you were working on but it look like it will alway be none none
22:36:52 sean-k-mooney melwitt: you could also get the user_id out of the instance if you have that https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L122-L123
22:40:04 melwitt sean-k-mooney: I'm adding user_id to InstanceMapping and need a reliable way to populate it (data migration of already existing records). and instance was the assumed way. and it occurred to me (from looking at the down cell stuff) that, request spec has user_id too, could I use that? the answer I now know is, no
22:40:26 melwitt I already wrote everything to use instance user_id, so this means no change for me
22:40:50 sean-k-mooney ah ok
22:44:00 sean-k-mooney melwitt: you could be checky and look at the allocation for the instacne and get the user id out of the consumers table in the api db
22:44:44 sean-k-mooney the consumer and allocation should have been there since pike
22:51:29 sean-k-mooney melwitt: :) oh look like you added the consumer table in pike https://github.com/openstack/nova/commit/b9eed6a2862d0d72c77706b49d05ea588d2d81f3
23:04:08 melwitt yeah, I dunno
23:06:00 melwitt that might not work bc I need to migrate soft-deleted instances (the soft-delete API) too, since they could restored theoretically at any point in the future. and I don't think those would have allocations, as they are supposed to be gone from the user perspective and not consuming resources
23:07:29 openstackgerrit Merged openstack/nova stable/pike: Not set instance to ERROR if set_admin_password failed https://review.openstack.org/608180
23:07:31 melwitt besides that, it seems most reliable to use instances. we had bugs in the past with orphaned allocations in placement and deployments out there might still have some
23:07:36 openstackgerrit Merged openstack/nova stable/pike: Add functional regression test for bug 1806064 https://review.openstack.org/623935
23:07:37 openstack bug 1806064 in OpenStack Compute (nova) pike "Volume remains in attaching/reserved status, if the instance is deleted after TooManyInstances exception in nova-conductor" [Medium,In progress] https://launchpad.net/bugs/1806064 - Assigned to Matt Riedemann (mriedem)
23:07:37 sean-k-mooney ya i guess you have to blance the risks. if the cell is donw then using ht user form the instance will fail
23:08:24 sean-k-mooney melwitt: ya the instace is proably the safest whcih is why i said using the consumer recored associated with the instance allocation was a little cheaky
23:09:15 melwitt oh, cheaky? you spelled it "checky" earlier and I didn't get it :)
23:09:37 sean-k-mooney oh ha ya
23:10:24 sean-k-mooney well it shoudl be cheeky if i use the irish/uk spelling
23:10:52 sean-k-mooney anyway i better go. have a good weekend o/
23:11:04 melwitt have a good weekend o/
23:21:28 openstackgerrit Merged openstack/nova stable/pike: Create BDMs/tags in cell with instance when over-quota https://review.openstack.org/623937
23:22:40 mriedem hmm, why do we persist RequestSpec.ignore_hosts
23:22:46 mriedem dansmith: ^ seems dangerous yeah?
23:22:55 mriedem just like why we don't persist retry and requested_destination
23:23:41 mriedem i.e. resize with allow_resize_to_same_host=false sets ignore_hosts=[instance.host], we save that, then resize again with allow_resize_to_same_host=False will always filter out the same host
23:24:02 mriedem er allow_resize_to_same_host=True the 2nd run
23:24:28 mriedem i know request spec is the thing everyone wants to think about at 3:30 on a friday
23:24:32 sean-k-mooney do we clear it on resize confirm/revert
23:24:55 sean-k-mooney i dont recal if we can retry a resize but perhapse that is why
23:25:09 sean-k-mooney that is just a guess
23:25:18 mriedem well, on the 2nd resize we would overwrite RequestSpec.ignore_hosts again
23:26:08 mriedem looks like the only other move operation that really looks at it is evacuate, and you can't evacuate to the same host anyway so ..
23:26:24 mriedem evacuate ignores instance.host as well
23:26:35 mriedem so i guess this is why it's not a problem
23:27:27 mriedem live migrate also uses it while retrying different hosts in conductor, but doesn't persist those changes (until someone comes along and adds code to the live migration task that saves the request spec for some reason)
23:29:51 sean-k-mooney well it doesnt sound like there is an obvious reason to save it to the db so maybe put up a patch to and see what breaks but form the sounds of it might not be needed and might be a source of a future bug

Earlier   Later