| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-20 | |||
| 15:42:55 | mriedem | 2. the actual stuff in conductor that populates RequestSpec.network_metadata from the instance info cache prior to calling the scheduler to find a target host | |
| 15:56:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add functional tests for numa-aware-vswitches https://review.openstack.org/583288 | |
| 15:56:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 15:56:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 16:00:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove reference to transient domain when detaching devices https://review.openstack.org/584433 | |
| 16:21:49 | openstackgerrit | Corey Bryant proposed openstack/nova master: In Python3.7 async is a keyword [1] https://review.openstack.org/584365 | |
| 16:25:56 | openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 16:25:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 16:33:42 | openstackgerrit | Pierre Blanc proposed openstack/nova master: Docs: Add guide to migrate instance with snapshot https://review.openstack.org/584442 | |
| 17:11:51 | tomtom001 | hello, does anyone know if the scheduler_default_filters=AllHostsFilter does anything? Are there any problems using it? | |
| 18:27:07 | mriedem | powerkvm ci is happy on this now https://review.openstack.org/#/c/560317/ | |
| 18:27:27 | mriedem | tomtom001: the AllHostsFilter is a noop | |
| 18:27:30 | mriedem | it accepts all hosts | |
| 18:28:25 | mriedem | superdan: on your bfv disk thing, i got the move / migrate patch up on top of that | |
| 18:28:37 | mriedem | fried_rolls also helped flush out some latent bugs in our disk usage reporting in the RT | |
| 18:29:10 | superdan | mriedem: oh awesome thanks, hadn't seen | |
| 18:29:16 | superdan | mriedem: since you're here.. on the down host thing... | |
| 18:29:35 | superdan | I was thinking -- can't we get away with the online migration without needing a marker record? | |
| 18:30:10 | superdan | we can query for max_limit records, where queued_for_delete is NULL and group_by or filter by cell to make the instance lookups more efficient | |
| 18:30:42 | superdan | we needed the marker for the initial roll-out of those records, because we didn't know where to pick up because we were creating them, | |
| 18:31:03 | superdan | but since we're just healing those that have no value set, we can use the NULL-ness to whittle that down I think | |
| 18:31:07 | mriedem | the migration is just copying how the reqspec migration was done | |
| 18:31:08 | mriedem | which uses a marker | |
| 18:31:28 | superdan | the creating of the reqspec? | |
| 18:31:30 | mriedem | yeah | |
| 18:31:33 | superdan | yeah, | |
| 18:31:40 | mriedem | i'm not sure that queued_for_delete will be NULL will it? | |
| 18:31:49 | mriedem | maybe it will, i forget how the defaults work in the new column | |
| 18:31:59 | superdan | this is different since we've already got all the records and we're just patching up | |
| 18:32:01 | superdan | it's not nullable=False, so I think it defaults to being nullable | |
| 18:32:02 | superdan | default is not server_default, | |
| 18:32:04 | superdan | so it's not in the schema AFAIK | |
| 18:32:42 | mriedem | so for every instance mapping with a NULL queued_for_delete, lookup it's instance and see if the instance is deleted (or soft deleted)? | |
| 18:33:09 | superdan | "Defaults to True unless primary_key is also True" | |
| 18:33:28 | superdan | you can still do bulk queries of each I think | |
| 18:33:47 | mriedem | by instance uuid? | |
| 18:34:05 | mriedem | so gimme 50 instance mappings with queued_for_delete=NULL, | |
| 18:34:06 | superdan | yeah, so you query out max_limit mappings, | |
| 18:34:11 | mriedem | right | |
| 18:34:17 | mriedem | and those instances might be spread across multiple cells | |
| 18:34:20 | superdan | and then do a get_by_filters({'uuid': uuids}) | |
| 18:34:28 | superdan | well, this appears to be doing one cell at a time no? | |
| 18:34:31 | superdan | it's not targeting | |
| 18:34:40 | mriedem | it's not multi-cell aware | |
| 18:34:42 | superdan | so just limit by cell when you're getting the mappings | |
| 18:34:43 | superdan | right | |
| 18:34:44 | mriedem | like the other migrations | |
| 18:35:18 | superdan | oh I see this is going by cell db first, so it's naturally scoped to a cell | |
| 18:35:27 | superdan | well anyway, | |
| 18:35:28 | mriedem | how do we know what cell we're in? | |
| 18:35:42 | mriedem | we have [database]/connection | |
| 18:35:55 | superdan | should be trivial to limit it per cell even starting from the mapping I think | |
| 18:36:37 | mriedem | would you page across cells until you've filled the limit of instances? | |
| 18:36:37 | superdan | it seems like it's worth avoiding the need for another marker-record-based migration...am I wrong? | |
| 18:37:29 | mriedem | i don't love a marker-based migration, but it works | |
| 18:37:37 | superdan | I think you'd just make this migration thing take a cell mapping, and then let nova-manage tell it what cell it's working on, and then nova-manage can take a cell_uuid or --all-cells like some of our other things | |
| 18:38:01 | mriedem | so this wouldn't be part of nova-manage db online_data_migrations then... | |
| 18:38:06 | mriedem | i don't love that | |
| 18:38:07 | superdan | no | |
| 18:38:09 | mriedem | b/c then it's another one off | |
| 18:38:10 | superdan | it should | |
| 18:38:11 | superdan | no | |
| 18:38:15 | superdan | it should be in there | |
| 18:38:31 | mriedem | we don't currently talk a cell uuid or -all-cells for online_data_migrations | |
| 18:38:38 | mriedem | *take | |
| 18:38:39 | superdan | I thought we had merged or proposed a thing to let that take a cell, but I see not, | |
| 18:38:44 | mriedem | no, | |
| 18:38:45 | superdan | so just make it figure out the current cell or something | |
| 18:39:03 | mriedem | in YVR we asked about making the migration stuff multi-cell aware and ops (cern) said they didn't care about it (nice to have but not necessary for now) | |
| 18:39:21 | superdan | thinking about this I gues: https://review.openstack.org/#/c/519275/ | |
| 18:39:22 | mriedem | i'm not sure how we figure out the current cell - we have the [database]/connection config but is that good enough? | |
| 18:40:00 | superdan | we fail to create a cell if you're duplicating that field, so I would think it would be yeah | |
| 18:40:08 | mriedem | about the template? | |
| 18:40:13 | mriedem | if the config is templated, | |
| 18:40:18 | mriedem | how do we do the lookup of the mapping? | |
| 18:40:26 | superdan | config isn't templated, the url in the db is | |
| 18:41:20 | superdan | well, I don't need to throw a wrench into this, | |
| 18:41:34 | superdan | I just... I dunno.. would rather not create a marker-based migration if we have any way to avoid it | |
| 18:42:11 | mriedem | we do have a way to avoid it if we start from the instance mappings rather than the instances, and query for instance mappings with queued_for_delete=NULL as you said | |
| 18:42:38 | superdan | yeah I know, that's what I'm saying :) | |
| 18:42:40 | mriedem | that would also make the migration essentially multi-cell aware if we pull the corresponding instances out of the cells for the mappings we found | |
| 18:42:53 | superdan | if we want to do that, then we can, yeah | |
| 18:43:05 | superdan | we have to have access to the api database to do this anyway, | |
| 18:43:13 | superdan | so it's not like we're adding a requirement to the config that we don't already have | |
| 18:44:01 | mriedem | it's essentially a re-write, | |
| 18:44:04 | mriedem | including the tests | |
| 18:44:10 | superdan | we could just iterate cell_mappings and only select instance_mappings per cell up to max_limit in any one go, so we don't have one batch that spans cells just to make the querying for instances easier | |
| 18:44:14 | mriedem | b/c you'd have to create mappings records directly in the database with null queued_for_delete values | |
| 18:44:32 | superdan | hmm? | |
| 18:44:39 | mriedem | like, | |
| 18:44:39 | superdan | why/when? | |
| 18:44:53 | mriedem | you can't just piggyback of the test that tssurya already wrote but change how the migratoin works, | |
| 18:45:01 | mriedem | because the functional test for what she has uses the objects | |
| 18:45:04 | mriedem | to populate the db | |
| 18:45:13 | mriedem | and creating the mapping from the object will make queued_for_delete not null | |
| 18:45:17 | superdan | oh you mean re-write of the tests | |
| 18:45:21 | superdan | not of the other patches in the series | |
| 18:45:26 | mriedem | yes the entire migration + tests would be a rewrite | |
| 18:45:40 | superdan | yup | |
| 18:45:49 | superdan | also, | |