Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-27
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 :(
20:23:12 openstackgerrit Eric Fried proposed openstack/nova master: Better handle live migration abort https://review.openstack.org/635440
20:23:15 mriedem if someone is building servers between the time the scheduler/conductor is new and the api is old, and they miss that race, the data migration can handle it
20:23:24 melwitt no, I think we don't need the soft deleted restore anymore because I included soft deleted in the nova-manage migration. since they could be restored theoretically any time in the future
20:24:40 mriedem so handle create and GET and that's good enough
20:24:44 mriedem data migration takes care of the rest
20:24:56 melwitt ok, I'll clean that up. thanks
20:30:19 melwitt that was an emotional roller coaster
20:31:24 openstackgerrit Merged openstack/nova master: Fix the api sample docs for microversion 2.68 https://review.openstack.org/639707
20:45:26 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Privatize SchedulerReportClient HTTP wrappers https://review.openstack.org/639818
20:45:39 efried melwitt: FYI as discussed earlier ^
21:00:05 melwitt efried: ack
21:03:46 mriedem melwitt: ok i've gone a few more patches up in your series
21:03:51 mriedem to the point of the wip and stopped there
21:05:15 melwitt mriedem: been reading through the comments. thanks. indeed I was trying to account for a scenario like, "what if they run nova-manage in the middle of the upgrade but not at the end, how to catch the stragglers". but I realize I might be suffering from not knowing something like, nova-manage is always run once at the end of the upgrade too
21:05:54 mriedem you run it until there are no more to process
21:05:58 mriedem and then when we drop the compat code,
21:06:01 mriedem we put in a blocker migration
21:06:42 mriedem there is no defined time that operators have to run online_data_migrations, they can run it whenever
21:06:58 melwitt well, if it's run in the middle of the upgrade and run at the exact wrong time, it would come back as "all done" because of the not-yet-scheduled old instance mappings, if there are any
21:06:58 mriedem it should be idempotent, save for the ones that create a marker
21:07:28 melwitt yeah, that's what I had thought, no defined time or way to run online_data_migrations
21:07:43 mriedem but then your counting quota code is going to catch that and do the legacy counting method right?
21:07:58 openstackgerrit Michael Still proposed openstack/nova master: Cleanup no longer required filters and add a release note. https://review.openstack.org/639826
21:08:01 mriedem and presumably log something if they haven't opted out of counting from placement b/c of edge or something?
21:08:08 melwitt yes
21:08:16 mriedem so i don't think we need to be super paranoid about it
21:08:24 mriedem and we'll add a blocker schema migration before dropping the compat code

Earlier   Later