Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-27
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 mriedem https://github.com/openstack/nova/commit/6e49019fae80586c4bbb8a7281600cf6140c176a is queens btw
20:12:57 melwitt request spec, what's persisted? what isn't? it's a surprise!
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 mriedem it should be idempotent, save for the ones that create a marker
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: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
21:08:28 melwitt ok, I see. that's helpful
21:08:32 mriedem i.e. you can't upgrade to U if you haven't done your homework
21:08:43 melwitt right
21:08:54 mriedem of course i'd like dansmith to check my work her
21:08:55 mriedem *her
21:08:57 mriedem di
21:08:59 mriedem here
21:09:23 mriedem also,
21:09:36 dansmith I refuse to touch your her
21:09:38 mriedem the GET /servers/{server_id} should catch any that miss that very tight winow
21:09:41 mriedem *window
21:09:54 mriedem yeah?
21:09:54 mriedem b/c the client is going to be polling the API for the server to be ACTIVE
21:10:20 melwitt yeah, I guess, another thought is if I re-worked the online_data_migrations to go the other direction, from instance mappings => cells (which was the original proposal)
21:10:32 dansmith I definitely think the less-complicated and more usual migration and update-on-read approach is good

Earlier   Later