| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-09 | |||
| 10:07:43 | gibi | that fullfills this requirement without any API behavior change | |
| 10:07:59 | gibi | as the API always returns both deleted and not delete actions :) | |
| 10:08:25 | gibi | if the requirement is to retrive deleted instance actions _only_ | |
| 10:08:29 | gibi | then there is an API change | |
| 10:10:40 | sean-k-mooney | gibi: ya i dont think there really is a usecase fo deleted _only_ | |
| 10:11:04 | sean-k-mooney | so i think we are ok to not change the db | |
| 10:11:07 | sean-k-mooney | *api | |
| 10:11:27 | gibi | yepp, I'm on that side yesa | |
| 10:11:43 | sean-k-mooney | so the cahnge is really to two things | |
| 10:12:03 | sean-k-mooney | the server delete sql query/funciton will need to soft delete the instance actions | |
| 10:12:23 | sean-k-mooney | and the instance actions quies will need to ignore the deleted field | |
| 10:12:29 | sean-k-mooney | and that shoudl be sufficnet | |
| 10:12:46 | pslestang | bauzas: I can change the BP to remove the possibility to retrieve deleted instance action as the solution proposed does not need such feature | |
| 10:12:57 | sean-k-mooney | oh and server restore shoudl also restore the instance_actions | |
| 10:13:05 | bauzas | pslestang: okay then | |
| 10:14:45 | pslestang | bauzas: done | |
| 10:15:43 | bauzas | gibi: sean-k-mooney: would we need to modify the DB if we accept to soft-delete the instance actions ? | |
| 10:16:05 | bauzas | pslestang: maybe that's here we would need to discuss about upgrades | |
| 10:16:15 | gibi | bauzas: we need to modify the DB yes, and need to modify the DB query the API does to ignore the new column | |
| 10:16:37 | bauzas | that's my question | |
| 10:16:47 | gibi | so if you are strict then there is an upgrade impact, as the db schema changes | |
| 10:16:47 | bauzas | for upgrades | |
| 10:17:10 | bauzas | I mean | |
| 10:17:18 | bauzas | for upgrading there are two directioins | |
| 10:17:34 | bauzas | 1/ we want to modify the DB for already existed instance actions | |
| 10:17:51 | bauzas | 2/ we don't do it and we only soft-delete the new instance actions | |
| 10:18:08 | bauzas | 1/ would mean that we would need to have a nova-manage command | |
| 10:18:27 | bauzas | 2/ wouldn't | |
| 10:18:53 | bauzas | for both, we need to discuss how the API query would do | |
| 10:19:01 | bauzas | remember that we have cells v2 | |
| 10:19:22 | bauzas | so we would need to know whether the cell DB is upgraded or not | |
| 10:19:33 | bauzas | before calling the instance actions table | |
| 10:19:41 | bauzas | (from the API I mean) | |
| 10:20:04 | bauzas | anyway, I need to get my kid from school in a few mins | |
| 10:20:27 | bauzas | but I guess we probably need to want to discuss about the upgrade questions in some... spec ? | |
| 10:20:45 | gibi | bauzas: that is your call :) I'm happy to discuss this in a spec | |
| 10:21:01 | bauzas | I don't wanna use my baton | |
| 10:21:08 | bauzas | it's more a core question | |
| 10:21:18 | bauzas | about upgrades | |
| 10:21:19 | gibi | regarding nova-manage that feels optional to me, and probably pslestang can state if OVH needs it or not | |
| 10:21:30 | gibi | so this is a requirement question | |
| 10:21:34 | bauzas | yeah | |
| 10:21:36 | pslestang | bauzas: by the way we already need to change the nova-manage purge behaviour which actually rely on update_at column for instance_action_* tables instead of deleted_at | |
| 10:21:44 | bauzas | correct | |
| 10:22:08 | bauzas | we also need to look at which methods look at instance actions | |
| 10:22:23 | bauzas | we know the API for sure | |
| 10:22:40 | bauzas | but I wonder whether we also look at the actions within nova directly | |
| 10:22:52 | sean-k-mooney | bauzas: we dont need to modify the db but we do need to modify hte db queries | |
| 10:22:59 | sean-k-mooney | the schema will remain the same | |
| 10:23:15 | bauzas | sean-k-mooney: agreed this isn't a DB schema modifcatioin | |
| 10:23:19 | gibi | sean-k-mooney: ooh the same already has deleted column, I missed that | |
| 10:23:20 | bauzas | but this is about the values | |
| 10:23:40 | sean-k-mooney | gibi: yep it already inherits form teh softdelete mixin | |
| 10:23:40 | pslestang | gibi: at OVH we do not use nova-manage but it could be useful for someone | |
| 10:23:41 | bauzas | for the moment, soft-deleted instances have not deleted actions | |
| 10:23:51 | sean-k-mooney | gibi: we just currently dont soft delete it | |
| 10:24:10 | bauzas | if we start to soft-delete actions when deleting instances, then I wonder what happens for existing instance related actions | |
| 10:24:26 | sean-k-mooney | the remain not deleted | |
| 10:24:30 | sean-k-mooney | but that wont mater | |
| 10:24:40 | bauzas | sean-k-mooney: this is one direction | |
| 10:24:45 | sean-k-mooney | since we will just start ignoring that field | |
| 10:24:47 | bauzas | or one solution | |
| 10:25:02 | sean-k-mooney | well setting a colume to the value it currently has in an update is valid | |
| 10:25:06 | bauzas | anyway, needs to get my daughter, moving | |
| 10:25:16 | sean-k-mooney | so the resotre can ignore it too and just always set it to 0 | |
| 10:25:34 | sean-k-mooney | even if its already 0 becuase we did not migrate the existing ones to eb soft deleted | |
| 10:26:24 | sean-k-mooney | pslestang: wehn you archive the deleted rows it will delete the instance actions | |
| 10:26:55 | sean-k-mooney | so by the time you get to purge they shoudl already be gone form the main table | |
| 10:27:41 | sean-k-mooney | archiveing the delete rows will remove the instance form the main table which would break the forien key constraitnt on the isntnace action table | |
| 10:28:08 | sean-k-mooney | so the rows have to be removed to archive the instance to the shadow tables | |
| 10:28:10 | pslestang | sean-k-mooney: from what I saw when you archive the deleted rows it does not delete the instance actions but copy them in shadow tables | |
| 10:28:37 | sean-k-mooney | it shoudl delete them form the main table and copy them to the shadow table | |
| 10:28:44 | sean-k-mooney | basically a move | |
| 10:28:49 | pslestang | yes that's it | |
| 10:29:00 | sean-k-mooney | yep that is the expect behavior | |
| 10:29:19 | sean-k-mooney | and pruge should remove them for the shadow tabels | |
| 10:29:43 | pslestang | and when we purge de archived rows, it deletes the instances based on deleted_at date, and deleted the insatnce_action bases on updated_at date | |
| 10:30:13 | sean-k-mooney | if you are using --before yes | |
| 10:30:27 | pslestang | yep | |
| 10:30:43 | sean-k-mooney | we could likely adapt that to use delete_at if its popluatedand fall abck to updated at if not | |
| 10:31:02 | sean-k-mooney | or just leave it as is | |
| 10:33:25 | pslestang | I prefer to adapt the code to make it consistent with the soft-delete of instance_actions | |
| 10:33:42 | sean-k-mooney | bauzas: im prety sure we dont look at the actions in nova they are write only | |
| 10:34:16 | sean-k-mooney | pslestang: ok would you prefer to write this all down in a short spec which we can try an appove in the review day on the 14th | |
| 10:34:29 | sean-k-mooney | or just proceed with this as a specless blueprint | |
| 10:34:57 | pslestang | I can write a short spec | |
| 10:34:58 | sean-k-mooney | i dont thinks we really need a spec but i think having one to document this would be good so im happy to review it if you write it | |
| 10:35:06 | pslestang | sure | |
| 10:35:20 | sean-k-mooney | cool feel free to add me and likely gibi/bauzas to it | |
| 10:35:22 | pslestang | is it better to change the one I wrote or to create a new one? | |
| 10:35:36 | sean-k-mooney | oh you already have one? | |
| 10:35:57 | pslestang | I was talking about the BP | |
| 10:36:26 | sean-k-mooney | oh reuse the blueprint you have now but file a spec with the same name | |
| 10:36:51 | pslestang | ok understand, I will dot it asap | |
| 10:37:03 | sean-k-mooney | cool | |
| 10:46:31 | bauzas | pslestang: thanks pslestang for writing up a quick spec | |
| 10:46:39 | bauzas | you can see the template and the already existing specs too | |
| 10:47:05 | bauzas | I'm not really asking about a paperwork, just want to be sure we don't miss any important issue | |
| 10:48:56 | sean-k-mooney | yep for me the spec is more just documentaiton/todo list in this case rather then a detailed design requirement because its complex | |
| 10:49:29 | sean-k-mooney | this is a relitively simple change we just dont want to miss any interop or upgrade impact | |