Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-27
19:45:16 mriedem same for multiattach volumes and device tags
19:45:21 jaypipes that's scheduling though. :)
19:45:31 jaypipes but yes.
19:46:02 mriedem my point was just trusted certs are not a flavor/image thing
19:46:11 mriedem you can't build aggregates around them
19:47:13 jaypipes ack
19:48:41 mriedem efried: aspiers: ok documented https://review.openstack.org/#/c/538498/19/nova/virt/driver.py@1034
19:49:23 efried mriedem: nice, race condition, correctly assumed "furiously"
19:52:33 melwitt mriedem, dansmith: thanks for the comments on the InstanceMapping.user_id patch. I just replied to mriedem. dansmith, build request doesn't have a user_id field, and request spec has had the user_id field only since rocky. do you think that's ok to rely on? I had been thinking we have to account for request specs older than rocky, but maybe that doesn't make sense because we're trying to account for old instance mappings that are in
19:52:33 melwitt the middle of being scheduled (and something older than rocky should be still in the middle of being scheduled)
19:52:57 melwitt *shouldn't
19:53:22 dansmith melwitt: right, it seems only worse to rely on something that has only existed since stein, but I thought it was on reqspec from well before that
19:53:23 dansmith either way,
19:53:37 melwitt it was added to reqspec in rocky
19:53:38 dansmith *all* IMs are missing it now, only some reqspecs are
19:53:40 dansmith okay
19:53:57 melwitt https://github.com/openstack/nova/commit/6e49019fae80586c4bbb8a7281600cf6140c176a
19:54:03 dansmith just seems like a joined query would be plenty fast, and then we're not just duplicating this again and again
19:54:10 dansmith once per instance per db should be enough :D
19:54:30 dansmith that's two years ago.. is that rocky?
19:54:54 melwitt yeah, I thought that would be older but I notice the included in version shows 18.0.0
19:55:13 dansmith should be queens right?
19:55:25 dansmith er, actually
19:55:28 dansmith maybe before that?
19:55:30 melwitt must have taken a long time to merge. queens is 17.0.0
19:55:34 dansmith queens was early '18
19:55:50 melwitt it merged may 2018
19:55:59 melwitt https://review.openstack.org/565340
19:56:06 dansmith ohh, I see, the 2017 date is the first git date I guess
19:56:11 melwitt yeah
19:56:27 dansmith well, anyway, same argument I think
19:56:44 melwitt I'm totally cool with not caring about request specs older than rocky if I got that reassurance
19:57:10 dansmith regardless, you could drop the first two patches and just change how you query for it whenever you do that
19:58:08 dansmith we could fix up old reqspecs on server GET for that matter
19:58:33 dansmith change our IM query to be IM joined with reqspec.user_id, and if null, set it to the instance.user_id we get from the cell before returning to the user
19:58:44 dansmith that would be pretty low overhead I think,
19:59:18 dansmith or just tell people if you want this to be accurate, make sure your migrations are done, just like would have to be the case if we added a new column to IM
20:00:09 melwitt migration for what, request spec? the addition of user_id didn't come with a migration
20:02:32 melwitt dansmith ^
20:02:55 dansmith melwitt: but you'd need one for your column, right? and I'm not sure why we don't have one for the reqspec one
20:03:06 melwitt oh, you mean me add one
20:03:10 melwitt yeah, ok
20:03:37 melwitt man, all that work I did to do all the various user_id populations
20:03:44 dansmith I'm saying just add one migration, for the field we have, which would be enough for all cases, instead of adding it to another place that has to also get patched up
20:04:02 melwitt I think I'm following now. thanks
20:04:24 melwitt back to the salt mines
20:06:12 dansmith melwitt: why do we not already have a migration for user_id on reqspec?
20:06:22 dansmith did that just get missed maybe?
20:06:42 melwitt dansmith: I dunno. probably wasn't considered important for already existing records
20:06:54 dansmith any idea why?
20:07:56 melwitt no, that's my guess
20:08:14 melwitt mriedem: do you happen to know why we didn't do nova-manage data migration for RequestSpec.user_id? https://review.openstack.org/565340
20:08:33 dansmith seems really weird to me
20:08:46 melwitt aye
20:09:04 dansmith I think because it's for "out of tree filters" and thus wasn't really done completely
20:09:35 dansmith just another reason not to further proliferate "I need this duplicate info over here so I'll add it again" kinda stuff
20:10:08 melwitt yeah. I was thinking in the box
20:11:38 mriedem melwitt: was never added - as dansmith said, it was added for an out of tree filter (mgagne) that relied on the user_id and therefore a full-fledged data migration wasn't added for existing request specs because we hack them prior to scheduling during moves
20:11:48 mriedem but any persistence of that request_spec.user_id update is purely accidental
20:12:53 dansmith yeah, that's unfortunate
20:12:57 melwitt request spec, what's persisted? what isn't? it's a surprise!
20:12:57 mriedem https://github.com/openstack/nova/commit/6e49019fae80586c4bbb8a7281600cf6140c176a is queens btw
20:13:26 dansmith melwitt: right, let's migrate to make it consistent and remove this point of ambiguity for the future :)
20:13:32 mriedem oh shit i guess that was rocky
20:13:34 mriedem i thought it was older
20:13:47 melwitt YEAH told ya
20:14:03 melwitt it was started way before rocky though
20:14:20 melwitt maybe not way before, but in queens
20:17:21 mriedem we don't have a db migration for requestspec.user_id b/c it's a json blob
20:17:34 mriedem oh having said that...
20:17:41 melwitt wait what? json blob
20:17:42 mriedem you can't join on a blob
20:17:50 dansmith wait it's not a column?
20:18:00 mriedem nope
20:18:01 mriedem ha
20:18:05 mriedem man, all that work I did to do all the various user_id populations
20:18:07 mriedem oops
20:18:12 mriedem spec = Column(MediumText(), nullable=False)
20:18:17 mriedem everything in request spec is json blob
20:18:23 mriedem so you can't filter on a join
20:18:24 mriedem wahwah
20:18:39 dansmith isn't project_id a column on reqspec?
20:18:55 mriedem hell no
20:19:14 mriedem https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api_models.py#L164
20:20:00 dansmith ugh, okay, nevermind then
20:20:13 mriedem salt mines averted
20:20:28 dansmith so, if we keep that,
20:20:42 dansmith I'm not sure why we're patching it up in all these places, vs. just on create and doing a migration like normal
20:20:58 melwitt I was thinking to be opportunistic but maybe I went overboard
20:21:21 dansmith it's just a lot harder to reason about correctness
20:21:29 melwitt we do need one for a soft-deleted instance restore at least. but maybe none of the others
20:21:36 melwitt oh wait, no
20:21:38 mriedem i don't think we need the conductor ones
20:21:42 mriedem in https://review.openstack.org/#/c/638574/3
20:21:46 melwitt since I added soft-deleted instances to the migration
20:21:48 mriedem we want the one on GET /servers/{server_id}
20:22:01 dansmith if we do it opportunitstically, just do it on load, not in 100 places, IMHO
20:22:03 dansmith mriedem: right
20:22:25 mriedem i can't speak for the soft deleted restore thing
20:22:37 melwitt ok, thanks. I pre-emptive optimizationed myself :(

Earlier   Later