| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-03 | |||
| 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 | ? | |
| 14:53:56 | mriedem | and the keypair information per instance should be in the instance_extra table yeah | |
| 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 | |
| 15:04:37 | dansmith | not from any real data I have | |
| 15:04:49 | dansmith | but rc2 should be "we can't release without this" and this doesn't seem to fit that, IMHO | |
| 15:05:29 | dansmith | we probably need to nuke all those "added in neutron" migrations | |
| 15:07:04 | mriedem | i've got a local change, sec | |
| 15:07:11 | dansmith | only one more now I guess | |
| 15:07:20 | dansmith | looking at your patch from earlier there were abunch | |
| 15:07:25 | mriedem | yeah https://review.openstack.org/#/q/topic:remove-newton-online-compat-code+(status:open+OR+status:merged) | |
| 15:07:29 | mriedem | i tried | |
| 15:07:30 | mriedem | got tired | |
| 15:22:39 | cdent | Is there a concise description on the rules about migrations between AZs somewhere? cold and live, force and not force? | |
| 15:23:15 | mriedem | likely not, at least off the top of my head, but i could probably explain it quick | |
| 15:23:26 | mriedem | then we could report a docs bug to fill that in later | |
| 15:24:22 | cdent | I'll take that | |
| 15:24:29 | mriedem | tl;dr if the user creates the server with a specific AZ or https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.default_schedule_zone is not None (meaning it goes into some default AZ), then the instance is restricted to that AZ for all move operations, | |
| 15:24:38 | mriedem | UNLESS it is forced like with live migrate or evacuate | |
| 15:25:17 | mriedem | which, by default, with OSC you get a forced live migration every time | |
| 15:25:25 | mriedem | so very easy to shoot yourself in the foot there as an admin | |
| 15:25:42 | mriedem | which is why there are at least 5 patches to osc to fix that, listed at L18 here https://etherpad.openstack.org/p/DEN-osc-compute-api-gaps | |
| 15:26:00 | cdent | \o/ | |
| 15:26:56 | cdent | are there any restrictions that distinguish the default AZ as special with regard to other AZs? | |
| 15:27:05 | mriedem | docs on migrate caveats regarding AZs could probably live here https://docs.openstack.org/nova/latest/user/aggregates.html#availability-zones-azs if you want to report a bug and i can wordsmith it later | |
| 15:27:11 | mriedem | no | |
| 15:27:31 | mriedem | if DEFAULT.default_schedule_zone is not None and the user doesn't request an AZ explicitly, it's treated as if the user did request the default AZ | |
| 15:27:43 | cdent | I guess if you specify that a sever is in foo-AZ (non-default) then you can't cold migrate from there to any other `one? | |
| 15:28:06 | mriedem | that happens here https://github.com/openstack/nova/blob/357da989c194a8b59842629cb64b2809143a4eae/nova/api/openstack/compute/servers.py#L641 | |
| 15:28:19 | mriedem | cdent: correct | |
| 15:28:40 | cdent | okay. thanks. I'll make a bug, summarize this stuff there. | |