| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-15 | |||
| 13:51:08 | mriedem | maybe i'll start in the ML | |
| 13:51:15 | mriedem | since we wouldn't want to do this for soft delete | |
| 13:53:41 | dansmith | mriedem: we could also do with a bug to clean up all the conversion back and forth to/from the legacy version that never got cleaned up | |
| 13:53:45 | stvnoyes1 | jaypipes: i was interested to see what would happen doing a swap to a stopped vm with the cli. the cli command returned without an error. The volume status of both vols were unchanged. And there was that Domain not found error in the logs. I found a similar non-error using the API, the test only failed because the new volume never became 'in-use'. | |
| 13:53:47 | bauzas | mriedem: I have a branch for that | |
| 13:54:14 | bauzas | mriedem: that's one of the things I have in my plate and never had time for that in Ocata | |
| 13:54:17 | mriedem | dansmith: yeah bauzas has a blueprint for that from ocata, | |
| 13:54:29 | mriedem | i don't remember seeing anything about actually deleting these things though | |
| 13:54:34 | bauzas | mriedem: hence me planning to fix it by Pike | |
| 13:54:41 | mriedem | i'll brain dump in the ML | |
| 13:54:43 | bauzas | mriedem: https://review.openstack.org/#/c/391060/ | |
| 13:54:45 | bauzas | dansmith: ^ | |
| 13:54:51 | mriedem | ah | |
| 13:55:12 | bauzas | it requires a rebase | |
| 13:55:47 | jaypipes | stvnoyes1: gotcha. would you expect instead to have an "unsupported operation on stopped server" error message return instead of no error? | |
| 13:56:51 | mriedem | bauzas: left my concern inline, but that potentially cripples a restored soft deleted instance, | |
| 13:57:04 | bauzas | mriedem: cool, thanks | |
| 13:57:11 | mriedem | i'm not sure how we deal with that, since we don't want the compute doing a call back up to the API to delete the request spec when the instance is hard deleted | |
| 13:57:25 | bauzas | mriedem: the problem is that we never agreed on soft-deleting the Spec with alaski | |
| 13:57:40 | dansmith | he means soft delete of the instance | |
| 13:57:45 | dansmith | (right?) | |
| 13:57:45 | bauzas | yeah | |
| 13:57:49 | bauzas | I understand that | |
| 13:57:49 | dansmith | not soft delete of the instance db record | |
| 13:57:55 | bauzas | sure | |
| 13:58:12 | bauzas | but if we trigger the Spec hard deletion on an instance soft-delete | |
| 13:58:20 | stvnoyes1 | jaypipes: agree, it would be better to get some kind of error. since a server can stop at any time, doing a check in the API code would help a lot but it wouldn't cover all the cases. | |
| 13:58:26 | bauzas | then, how could we restore it if we restore the instance ? | |
| 13:58:34 | dansmith | you don't restore it, | |
| 13:58:49 | dansmith | you have to defer deletion of it until the soft-deleted instance is purged somehow | |
| 13:58:54 | dansmith | although that happens on the compute node | |
| 13:59:04 | bauzas | yeah, the trigger is then totally different | |
| 13:59:33 | bauzas | we could keep the spec record until the instance is hard-deleted | |
| 13:59:41 | bauzas | but I need to see where we trigger that | |
| 13:59:45 | dansmith | right, so you have to just not delete it in that case, and either leak it (as we do now for everything) or at least delete if you hard-delete | |
| 14:00:05 | bauzas | dansmith: you said we hard-delete on the compute side ? | |
| 14:00:14 | bauzas | a periodic I guess ? | |
| 14:00:21 | dansmith | compute is what does the purge yeah, in a periodic | |
| 14:00:30 | mriedem | dansmith: right | |
| 14:00:37 | mriedem | well, SOFT_DELETED in the API sense | |
| 14:00:39 | mriedem | not the DB sense | |
| 14:00:40 | bauzas | okay, that would ease my branch :) | |
| 14:01:11 | bauzas | IIRC, we already merged the .delete() method I added in Newton | |
| 14:01:23 | bauzas | the caller of that .delete() never merged in time in Newton | |
| 14:01:24 | mriedem | which delete() method? | |
| 14:01:28 | bauzas | the Spec one | |
| 14:01:36 | mriedem | yes RequestSpec.destroy() exists, | |
| 14:01:38 | mriedem | but nothing calls it | |
| 14:01:45 | bauzas | exactly | |
| 14:01:53 | bauzas | because I wrote the destroy() | |
| 14:01:58 | bauzas | it was in the same series | |
| 14:01:59 | mriedem | so i think we're left with leaking request specs for soft-deleted instances for now i guess | |
| 14:02:04 | bauzas | yup | |
| 14:02:11 | bauzas | lemme clarify | |
| 14:02:26 | bauzas | in Newton, I discovered that (because I missed that when I reviewed alaski's persistence series) | |
| 14:02:29 | mriedem | we could provide a nova-manage command to purge those if you wanted... | |
| 14:02:37 | bauzas | so I wrote a series | |
| 14:02:45 | bauzas | .destroy() as a bottom patch | |
| 14:02:49 | bauzas | the bottom merged | |
| 14:02:51 | dansmith | mriedem: we have InstanceMappings to purge in that case as well | |
| 14:02:57 | bauzas | but not the top one | |
| 14:03:13 | mriedem | dansmith: nova-manage api_db purge_zombies | |
| 14:03:13 | bauzas | and in Ocata, I had that in my plate, but the placement things diverted me | |
| 14:03:28 | bauzas | so, we're now in Pike and it's still in my plate :) | |
| 14:03:36 | mriedem | eat faster | |
| 14:03:42 | dansmith | mriedem: aye | |
| 14:04:17 | bauzas | mriedem: dansmith: I wrote a db online migrations that was creating the spec records for old instances, could we envisage some hack for purging ? | |
| 14:04:40 | bauzas | ie. we trigger deletes on the periodic task | |
| 14:04:54 | bauzas | but we could also write a db online migration script that would do that | |
| 14:05:19 | dansmith | we shouldn't abuse online migrations for periodic cleanup | |
| 14:05:21 | dansmith | IMHO | |
| 14:05:32 | bauzas | okay | |
| 14:05:34 | bauzas | fine by me | |
| 14:05:40 | bauzas | I'll revisit that change | |
| 14:06:04 | bauzas | if we have a periodic, do we still need a purge command ? | |
| 14:06:15 | bauzas | dansmith: mriedem^ | |
| 14:06:20 | dansmith | we can't have a periodic, IMHO | |
| 14:06:39 | dansmith | having all the conductors purging all the time would be bad I think | |
| 14:06:46 | dansmith | apis don't run periodics | |
| 14:06:49 | mriedem | yeah that ^ | |
| 14:06:50 | dansmith | and the compute node can't do it | |
| 14:06:55 | mriedem | computes can't talk to api | |
| 14:07:24 | mriedem | so yeah, it's on-demand purge for now it sounds like | |
| 14:07:35 | mriedem | the scheduler has periodics... | |
| 14:07:41 | mriedem | and this is related to scheduling stuff | |
| 14:07:50 | dansmith | it's too heavy for scheduler, IMHO, | |
| 14:07:57 | dansmith | requires hitting cell and api databases, | |
| 14:08:02 | mriedem | true | |
| 14:08:09 | mriedem | just like discover_hosts :) | |
| 14:08:21 | dansmith | and scheduler is supposed to be single-threaded | |
| 14:09:02 | dansmith | yeah, but that is _actually_ scheduling-related and also getting to be nearly zero time :) | |
| 14:09:14 | dansmith | and disabled by default and not required for normal operation | |
| 14:09:30 | bauzas | okay I see | |
| 14:09:52 | bauzas | dansmith: so you propose nova-manage api_db purge_zombies ? fine by me, lemme write it quickly | |
| 14:09:55 | dansmith | and also, remember the suggestion of potentially making the scheduler just a pass-through library call | |
| 14:10:07 | bauzas | yeah I see your point | |
| 14:10:17 | bauzas | and we can't use compute periodic because of the upcall | |
| 14:10:20 | dansmith | it'd be nice if we could just roll this into archive_rows or something | |
| 14:10:27 | dansmith | archiving rows in the api db is just deleting them | |
| 14:10:52 | bauzas | dansmith: well, it would require some change in .destroy() but nothing really hard :) | |