| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-27 | |||
| 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 | |
| 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 | b/c the client is going to be polling the API for the server to be ACTIVE | |
| 21:09:54 | mriedem | yeah? | |
| 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 | |
| 21:10:40 | melwitt | but looking at the queued_for_delete migration made me think maybe it would be too inefficient to go the other direction | |
| 21:11:12 | melwitt | the reason it can come back as "all done" is about I made it iterate cells and migrate that way | |
| 21:11:24 | mriedem | hmm | |
| 21:11:29 | melwitt | *when there are instance in-flight being scheduled | |
| 21:11:40 | mriedem | so if i'm cern and i've got 72 cells and running this in batches, | |
| 21:11:57 | mriedem | it's going to query instance mappings from the same cells every time until it find something new to process right? | |
| 21:12:27 | mriedem | e.g. we've migrated everything for cells 1-50 | |
| 21:12:35 | dansmith | are you talking about how to migrate the mappings in a migration? | |
| 21:12:39 | mriedem | to start processing cells after 50, we have to check 1-50 all over again | |
| 21:12:52 | mriedem | https://review.openstack.org/#/c/633351/14/nova/objects/instance_mapping.py@243 | |
| 21:13:01 | dansmith | you process the records by which mappings have a null user_id | |
| 21:13:14 | dansmith | you collate those by cell, and do them in batches against the cell | |
| 21:13:23 | dansmith | you don't re-process anything because you've set the value to non-null once it's done | |
| 21:13:41 | melwitt | yeah, I did it similar to queued_for_delete, it gets instance mappings that have instances in cells | |
| 21:13:41 | mriedem | i don't tihnk that's what this is doing | |
| 21:14:00 | dansmith | I'm describing what it *should* do | |
| 21:14:35 | melwitt | but it will miss instance mappings that don't yet have an instance in a cell (I think) | |
| 21:14:56 | dansmith | the current implementation will you mean | |
| 21:14:58 | melwitt | yeah | |
| 21:15:07 | dansmith | sure, but iterating by cell makes no sense anyway, IMHO | |
| 21:15:11 | mriedem | well it's based on populate_queued_for_delete which dansmith wrote so i hope it's correct :) | |
| 21:15:32 | melwitt | ok :( I will redo it | |
| 21:16:48 | mriedem | well i guess have dansmith look at https://review.openstack.org/#/c/633351/14/nova/objects/instance_mapping.py and make sure we're on the same page | |
| 21:17:57 | dansmith | I probably did the qfd one by cell because you had to get a list of instances not deleted per cell | |
| 21:18:14 | dansmith | or rather, if you don't find an instance deleted per cell, then you mark all of those mappings as not deleted | |
| 21:18:40 | dansmith | but you don't need to do that in this case.. you want to batch/collate by cell, but you can do that with a single query I think in the other case | |
| 21:20:49 | dansmith | neither the user_id or qfd ones probably need to care about things in-flight that don't have that set, | |
| 21:21:07 | dansmith | since you have to run this after you've upgraded your controller code, so things in flight there would already be setting the new value | |
| 21:21:13 | dansmith | meaing, things that don't have a cell yet | |
| 21:25:07 | melwitt | right | |
| 21:26:15 | melwitt | oh, hm ok. I misread at first | |
| 21:26:54 | melwitt | have to run this after upgraded controller code, so things in flight there would already be setting user_id | |
| 21:28:06 | dansmith | right you have to have upgraded the code that can handle the new format for a thing before you start running migrations to move them into the new format of course | |
| 21:28:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Optimize populate_queued_for_delete online data migration https://review.openstack.org/639840 | |
| 21:28:37 | melwitt | what about old controller, in-flight, upgrade controller, run online_data_migrations, doesn't that fall through the cracks? | |
| 21:28:39 | mriedem | while we're talking about this ^ | |
| 21:29:12 | mriedem | melwitt: if we hit that, the GET /servers/{server_id} would migrate the mapping | |
| 21:29:36 | melwitt | ok | |
| 21:30:58 | melwitt | ok, will take another stab at this | |
| 21:31:19 | mriedem | this, or something | |
| 21:31:25 | mriedem | something is getting stabbed | |
| 21:31:42 | openstackgerrit | sean mooney proposed openstack/os-vif master: add additional check and gate jobs for os-vif https://review.openstack.org/639732 | |
| 21:31:44 | dansmith | melwitt: for the qfd you mean? I guess maybe, but it would still be cleared once you delete that | |
| 21:31:58 | dansmith | melwitt: for user_id if you process by mapping and not by cell, then it doesn't matter | |
| 21:32:09 | melwitt | no, for user_id | |
| 21:32:52 | melwitt | when you said "user_id doesn't need to care about things in-flight that don't have it set" | |
| 21:34:43 | dansmith | melwitt: I'm confused if or what you're asking | |
| 21:34:50 | melwitt | dansmith: but I think I get it now that if we process by mapping, it will catch them | |
| 21:34:56 | dansmith | right | |
| 21:35:02 | dansmith | I said | |
| 21:35:21 | melwitt | I was just thinking if we go by cell then we would miss something in a super specific timed case | |
| 21:35:26 | dansmith | "probably don't need to care" meaning that leaking things in flight probably isn't as big of a deal.. but not saying we should ignore them or not try to get them | |
| 21:35:37 | dansmith | sure | |