| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-03 | |||
| 14:20:18 | efried | as you wish | |
| 14:21:00 | openstackgerrit | Sylvain Bauza proposed openstack/nova stable/stein: Libvirt: gracefully handle non-nic VFs https://review.openstack.org/649630 | |
| 14:21:09 | bauzas | mriedem: ^ | |
| 14:27:36 | mriedem | is there anything else we want to consider? https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/stein | |
| 14:29:12 | bauzas | mriedem: https://review.openstack.org/#/c/647310/ is controversial for RC2 | |
| 14:31:24 | dansmith | I thought we said no to that? | |
| 14:31:52 | dansmith | and sounds like lyarwood is on board with the original plan too | |
| 14:32:58 | openstackgerrit | Helena proposed openstack/nova-specs master: Spec for a new nova virt driver to manage an RSD, composable infrastructure deployment https://review.openstack.org/648665 | |
| 14:32:59 | lyarwood | yarp, missed that it had already been discussed in the change once I got back online yesterday. gertty-- | |
| 14:34:26 | lyarwood | Does anyone have any idea why nova-stable-maint already has +2 on stable/stein btw? | |
| 14:34:52 | mriedem | i'm not sure if the release team distinguishes anymore | |
| 14:34:54 | mriedem | smcginnis: ^? | |
| 14:36:26 | jaypipes | mriedem, dansmith: for online data migrations, did we envision those migration routines living forever or are we envisioning being able to delete some over time? | |
| 14:36:38 | mriedem | jaypipes: we already have deleted some over time | |
| 14:36:42 | dansmith | jaypipes: we have deleted many | |
| 14:36:50 | jaypipes | oh, ok. | |
| 14:36:55 | mriedem | most recent memory is i removed the request spec and flavor migration ones | |
| 14:37:07 | dansmith | we're not super good at setting that timer and then doing it, like any other cleanup | |
| 14:37:15 | jaypipes | mriedem, dansmith: after looking into mnaser's troubles, I think the removing the keypairs migration entirely might be the best solution. | |
| 14:37:20 | dansmith | but most of them should be relatively no-op-ish if done properly | |
| 14:37:21 | jaypipes | it's Newton-era | |
| 14:37:38 | jaypipes | dansmith: unfortunately that one does a full table scan across instance_extra each time it's run. | |
| 14:37:45 | mriedem | jaypipes: yeah i started looking at that the other day, but i don't think we have a blocker migration or anything in place to make sure the migration has completed before we rip that out | |
| 14:37:47 | jaypipes | dansmith: looking for WHERE keypairs=NULL | |
| 14:37:47 | mriedem | like we did for flavors | |
| 14:38:06 | dansmith | mriedem: we might not be able to for that one | |
| 14:38:19 | jaypipes | dansmith: and for large DBs like mnaser's 3M+ instance_extra records, it really is a resource hog. | |
| 14:38:22 | dansmith | mriedem: but since it's so old, if it hadn't finished, we probably know that other things would be broken | |
| 14:38:24 | mriedem | if there are keypairs in the cell db wouldn't that be an indication? | |
| 14:38:26 | dansmith | jaypipes: I hear you | |
| 14:38:51 | mriedem | for the request spec migration, we didn't have a blocker migration, just a nova-status upgrade check: https://github.com/openstack/nova/commit/ed4fe3ead62c09ec7de7b6a11072295a99997b4f#diff-91e852cb498abb50ca653ef6418bd65a | |
| 14:39:05 | mriedem | oh i added the upgrade check in rocky, dropped the reqspec migration code in stein | |
| 14:39:11 | dansmith | jaypipes: I don't agree that going back to putting this inside the schema migration would make people happy, as his cloud would still be down while he waits for that to happen, but.. you know :) | |
| 14:40:02 | dansmith | jaypipes: hopefully you just meant some sort of version-like sentinel instead of scanning to determine if things needed to be done :) | |
| 14:40:04 | jaypipes | dansmith: why would his cloud be down? | |
| 14:40:14 | dansmith | jaypipes: if we did it in the middle of a schema migration? | |
| 14:40:34 | jaypipes | yeah. | |
| 14:40:52 | dansmith | because he'd be halfway between two schema versions with two versions of code that can't use it? | |
| 14:41:06 | dansmith | i.e. the reason we decoupled those tasks in the first place | |
| 14:41:11 | jaypipes | sorry, you misunderstand... | |
| 14:41:46 | mriedem | wouldn't dropping the keypairs migration be similar to dropping the instance groups migration? https://github.com/openstack/nova/commit/1160921c2d053ce33279ca4ec1f00572271e7c95#diff-91e852cb498abb50ca653ef6418bd65a | |
| 14:42:19 | mriedem | jaypipes: fyi https://review.openstack.org/#/q/topic:remove-newton-online-compat-code+(status:open+OR+status:merged) | |
| 14:42:21 | mriedem | for a blueprint | |
| 14:42:26 | jaypipes | dansmith: if we did the data migration as an Alembic/sqlalchemy-migrate migration, then we wouldn't have to ever run the "check to see if this pre-condition exists" (like the SELECT against the instance_extra table...) more than once. | |
| 14:42:35 | jaypipes | mriedem: ack | |
| 14:42:59 | mriedem | i seem to remember we've fucked up the schema migration scripts before as well | |
| 14:43:05 | jaypipes | dansmith: I'm not saying the data migration would be the *same* migration as the schema migration. just that we could do it *as* a migraiton. | |
| 14:43:12 | dansmith | jaypipes: that's what I said above about the sentinel approach for scanning, but definitely do not agree they should be in the same migration | |
| 14:43:14 | jaypipes | instead of the whole separate nova-manage thing. | |
| 14:43:17 | mriedem | and since you can't downgrade the schema to get back, you'd have to reset the version in the migrations table and re-run those if we had a bug | |
| 14:43:21 | jaypipes | but that train has passed... | |
| 14:43:27 | dansmith | jaypipes: okay, well, you can see why people might be confused about such a statement :) | |
| 14:43:40 | jaypipes | mriedem: not once, ever, has anyone ever done a downgrade of a schema migration. | |
| 14:43:54 | mriedem | that's not true | |
| 14:44:12 | mriedem | hell we used to gate on being able to upgrade and downgrade the schema | |
| 14:44:26 | jaypipes | gating != ever being done in production, ever. | |
| 14:44:27 | dansmith | it didn't actually work for real data | |
| 14:44:28 | dansmith | yeah | |
| 14:44:30 | dansmith | heh | |
| 14:44:41 | jaypipes | it just doesn't happen, sorry. | |
| 14:44:51 | dansmith | which is why I don't want to couple the two processes | |
| 14:45:08 | jaypipes | the procedure is a) backup your stuff, b) run migrations, c) test and if problem, d) restore from backup. | |
| 14:45:21 | dansmith | but jaypipes, we could of course just set up our own version counter to be a sentinel to avoid re-running migrations, | |
| 14:45:33 | jaypipes | nobody ever ran a schema downgrade in production. I can almost guarantee that. | |
| 14:45:44 | mriedem | i should introduce you to some chinese people i know then | |
| 14:45:59 | mriedem | anyway, i'm not saying it's something that should be done | |
| 14:46:03 | jaypipes | mriedem: then, clearly, SOMEONE is WRONG on the Internet. | |
| 14:46:10 | jaypipes | :P | |
| 14:46:12 | dansmith | jaypipes: we certainly have people that think they want it, but I definitely agree they're misguided :) | |
| 14:46:26 | mriedem | i have also told them ^ | |
| 14:46:32 | jaypipes | I'm a bit slap-happy today, guys, you'll have to forgive me. | |
| 14:48:12 | mriedem | i'd be happy to drop the newton-era keypair migrations, but i think it's more complicated than just deleting that code, since we normally have something that checks to see if you have completed your homework before we just drop the migration routine and all of the compat code | |
| 14:48:19 | mriedem | which requires some thinking | |
| 14:48:34 | dansmith | well, | |
| 14:49:11 | dansmith | if the migration is acutely painful we could drop it without dropping the compat code and then take our time with the latter I think | |
| 14:49:18 | dansmith | or fix the migration to be more efficient if that's really possible | |
| 14:49:36 | dansmith | ISTR that since they're one-to-many and across DBs it's harder than that | |
| 14:49:45 | dansmith | but it's been a long time | |
| 14:49:50 | jaypipes | mriedem: yeah, I was pondering what that check would be other than SELECT COUNT(*) FROM instance_extra WHERE keypairs IS NULL, though. | |
| 14:50:39 | mriedem | there is also https://review.openstack.org/#/c/517158/ which might mean some of the migration routine is dead now | |
| 14:50:44 | mriedem | i.e. https://github.com/openstack/nova/blob/master/nova/objects/keypair.py#L242 | |
| 14:51:29 | mriedem | oh nvm that just means we wouldn't hit this https://github.com/openstack/nova/blob/master/nova/objects/keypair.py#L260 | |
| 14:53:02 | mriedem | https://github.com/openstack/nova/commit/be8242cb5a0f8396f6b8c042813847db0571df14#diff-f5877540177ee26b63552ec5f56d74fb | |
| 14:53:45 | mriedem | so as of ocata, the keypairs table in the cell dbs should be empty | |
| 14:53:56 | mriedem | and the keypair information per instance should be in the instance_extra table yeah | |
| 14:53:56 | mriedem | ? | |
| 14:56:59 | mriedem | i'm confused, if you can't upgrade to ocata while there are keypairs in the cell db https://github.com/openstack/nova/commit/be8242cb5a0f8396f6b8c042813847db0571df14#diff-f5877540177ee26b63552ec5f56d74fb then how would the 'migrate to api db' still hit anything here? https://github.com/openstack/nova/blob/master/nova/objects/keypair.py#L265 | |
| 14:57:39 | dansmith | mriedem: I think it's not hitting anything, it's just scanning the whole table which is the problem | |
| 14:57:58 | mriedem | "select([func.count()]).select_from(keypairs).where( keypairs.c.deleted == 0).scalar()" is the query jaypipes just said | |
| 14:58:08 | mriedem | oh nvm it's not | |
| 14:59:26 | mriedem | based on the commit message in https://review.openstack.org/#/c/517158/ i think we probably have reasonable justification to just kill that 'migrate to api db' data migration from newton that is still getting run | |
| 14:59:36 | jaypipes | mriedem: yeah, the problematic one is the scan on instance_extra | |
| 15:00:40 | mriedem | i'm not sure why i didn't go further on https://review.openstack.org/#/c/517158/2/nova/cmd/manage.py and remove keypair_obj.migrate_keypairs_to_api_db - i probably just ran out of time and was doing them in order, and wanted to handle the request spec one first, which i did in rocky/stein | |
| 15:00:57 | mriedem | so.....i think we can probably remove migrate_keypairs_to_api_db now? | |
| 15:01:02 | jaypipes | mriedem: maybe you were performing a schema downgrade in production. | |
| 15:01:11 | mriedem | i will cut you | |
| 15:01:55 | mriedem | if we think we can drop this, and we're doing an rc2 for stein we might want to get off this pot and include it in rc2 if it's really causing pain for upgrades | |
| 15:02:20 | mriedem | but i feel like i'm trying to convince myself this is ok | |
| 15:02:35 | mriedem | like everything i do in nova, which comes back to bite my ass | |
| 15:04:31 | dansmith | seems risky for an rc2 | |