| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-24 | |||
| 09:36:22 | gibi | in your feature you add a new ovo which is not soft deletable, that is good | |
| 09:36:48 | gibi | then in AA if you want, you can work on bumping the Aggregate OVO to 2.0 and add a microversion to hide deleted_at | |
| 09:36:49 | Uggla | so I change it as a test to inherit from NovaPersistantObject (without soft delete) | |
| 09:38:21 | Uggla | gibi but is it something that could be done in AA or later to have a "deprecation" time ? | |
| 09:39:17 | gibi | there is multiple things. The API change needs a microversion bump, it does not need a deprecation period but we need to support old microversions( you can fake deleted_at in the API response for old microversions) | |
| 09:39:36 | gibi | the Aggregate OVO change needs a major ovo version bump to 2.0 to remove a field | |
| 09:39:52 | sean-k-mooney | same for all other objects | |
| 09:40:00 | gibi | that means if we pass Aggregate OVOs over RPC then we need to support Aggregate 1.x and 2.0 parallel for an extra release | |
| 09:40:23 | sean-k-mooney | we cant do the db contraction till CC | |
| 09:40:32 | gibi | yes, that is the 3rd thing | |
| 09:40:34 | sean-k-mooney | because of teh new life cycle | |
| 09:41:12 | gibi | but the Aggregate table has no deleted_at so no need to contract there | |
| 09:41:17 | sean-k-mooney | well technially we can do it in BB if we deperecate teh soft delete feature in AA | |
| 09:41:23 | Uggla | yes so do you think it worth it ? | |
| 09:41:43 | sean-k-mooney | yes long term but i can upgrade my -1 to a -2 if you like | |
| 09:41:57 | sean-k-mooney | you should not be doing this as part of the share change | |
| 09:42:06 | sean-k-mooney | its its own spereate thing | |
| 09:42:22 | Uggla | sean-k-mooney, no this is outside of the share stuff. | |
| 09:42:34 | sean-k-mooney | right it needs a spec | |
| 09:42:36 | gibi | Uggla: it depends. I think the important part is not to add new ovos with soft delete. Cleaning up the old ones is a bit of busy work in my eyes | |
| 09:43:05 | sean-k-mooney | it has a prerrty large upgrad impact so it cant reasonable be done as a bugfix or specless blueprint | |
| 09:43:28 | sean-k-mooney | gibi: this kind of need to be done in tandom with removign shadow tables for it to be useful | |
| 09:43:30 | gibi | yes I agree to draft a spec for it if you want to work on this | |
| 09:43:40 | gibi | sean-k-mooney: good point | |
| 09:44:23 | Uggla | gibi, agree I don't want to work on something if you think the benefits are low. | |
| 09:44:24 | sean-k-mooney | personally removing the shaddow tabels woudl be more useful in my view then the soft delete mechanisum | |
| 09:44:52 | sean-k-mooney | Uggla: its not that its low its that there is a lot more work to do then just that patch | |
| 09:45:22 | gibi | Uggla: you can bring this up on the PTG to get a wider set of opinions about it | |
| 09:45:37 | sean-k-mooney | Uggla: its quite a large change to do right and ensure we dotn break rooling upgrades | |
| 09:47:12 | Uggla | sean-k-mooney, agree I know that changing that is a lot of work due to the API impact etc... But from your point of view is it something that "bring value" ? | |
| 09:47:14 | sean-k-mooney | by the way if an object suppots soft delele is determin by if it has the softDeleteMixin in the db model | |
| 09:47:16 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L139 | |
| 09:47:19 | sean-k-mooney | not the ovo | |
| 09:47:39 | sean-k-mooney | Uggla: its something i have wanted to do for a few releases | |
| 09:48:20 | sean-k-mooney | basically i want to remove or custom db backup/audit facilaties sicne operator have written there own | |
| 09:48:45 | sean-k-mooney | nova i think is the only project with shadow tabels so they wroge a solution that worked for all of them | |
| 09:50:43 | sean-k-mooney | https://github.com/ovh/osarchiver/ | |
| 09:50:45 | sean-k-mooney | ^ | |
| 09:51:08 | sean-k-mooney | i woudl like to replace or in tree shadow tabels with that solution eventually | |
| 09:52:20 | sean-k-mooney | i would propose formally deprecating Shadow tabels in AA and remove them in BB | |
| 09:52:38 | sean-k-mooney | soft delete we might want to keep becuase you can undelete instnaces | |
| 09:52:50 | sean-k-mooney | if the soft delete functionlatiy is enbaled itn the config | |
| 09:56:06 | sean-k-mooney | Uggla: gibi by the way we do not support fog deleteing for the api db resouces | |
| 09:56:07 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/api/models.py#L61-L95 | |
| 09:56:59 | sean-k-mooney | we do for all the cell db resouce excpt server tags and console auth tokens | |
| 09:57:01 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L1023-L1073 | |
| 09:57:05 | gibi | yep I noted that above that the aggregate db table has not deleted_at column so no need to contract | |
| 09:57:48 | sean-k-mooney | so for the share mappign we dont actully need a new ovo | |
| 09:58:02 | sean-k-mooney | we can just not add the soft delete mixing to the table in the db model | |
| 09:59:38 | sean-k-mooney | Uggla: https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/models.py#L129-L137 is where soft delete is implemented | |
| 10:00:12 | gibi | the ovo base class adds deleted_at column today | |
| 10:00:16 | gibi | to the ovo | |
| 10:00:23 | sean-k-mooney | to the object sent over the wire | |
| 10:00:36 | sean-k-mooney | but that does not mean it needs to be in the api reponce or the db model | |
| 10:01:23 | sean-k-mooney | we can just ignore them. if we want to have a new base class that does not have them that is also ok | |
| 10:01:25 | gibi | my original comment on Uggla's patch was to not hack out the deleted_at column from the ShareMapping ovo but not add it in the first place | |
| 10:01:50 | sean-k-mooney | ya im fine with that too | |
| 10:02:01 | gibi | https://review.opendev.org/c/openstack/nova/+/839401/6/nova/objects/share_mapping.py#49 | |
| 10:02:54 | sean-k-mooney | so they dont need to do that | |
| 10:03:05 | sean-k-mooney | if they just dont defien the columns in the db model | |
| 10:03:52 | gibi | honestly I don't want to to have columns in a new ovo that are not used | |
| 10:04:05 | sean-k-mooney | ack | |
| 10:04:18 | sean-k-mooney | we have that alredy for many object | |
| 10:04:37 | gibi | yes, but don't increase that debt if not really necesary | |
| 10:04:44 | sean-k-mooney | sure | |
| 10:05:05 | sean-k-mooney | so i think we are agreed Uggla should add one new base class without those fields | |
| 10:05:09 | sean-k-mooney | and not touch any of the rest | |
| 10:05:38 | sean-k-mooney | and we can disucss at the ptg if we will have time to do object cleanup in AA or BB and the faith of shadow tables | |
| 10:05:55 | gibi | agreed | |
| 10:05:56 | gibi | :) | |
| 10:06:24 | Uggla | sounds good to me. thanks. | |
| 10:08:28 | gibi | sean-k-mooney: re pci: I have to retract my original statement about nova splitting pools per PF. It splitted in my original test due to numa node differences (the fixture creates numa automatically). So on the same numa node we create pools with multiple PFs (of PCI devs). But as I already confirmed that the scheduler logic can split a request over multiple pools I can (and will) cahnge the pooling | |
| 10:08:34 | gibi | logic not to merge pools from different PFs or PCI devs | |
| 10:09:37 | sean-k-mooney | ack | |
| 10:09:39 | gibi | I realized that when I created a host with 3 PCI devs. so numa0 got two | |
| 10:09:47 | sean-k-mooney | ah | |
| 10:09:55 | sean-k-mooney | ya so i know it did it for numa | |
| 10:10:02 | sean-k-mooney | but within a numa node i was not sure | |
| 10:10:11 | sean-k-mooney | i think it also splits for differnnt tags | |
| 10:10:15 | sean-k-mooney | i.e. physnets | |
| 10:10:21 | sean-k-mooney | or tursted vs non trusted | |
| 10:10:24 | gibi | yes I assumed so, what I did not relaized that the fixture automatically split devices equally between numa0 and numa1 | |
| 10:10:31 | sean-k-mooney | but i guess it combines if they are the same | |
| 10:10:49 | sean-k-mooney | ah yes it does | |
| 10:11:06 | sean-k-mooney | you can just create the device manually but the fixture i generally nicer to use | |
| 10:11:14 | gibi | yep | |
| 10:13:36 | gibi | basic cold migrate and resize works with miniumal changes in the conductor so I think evac and unshelve will be easy too | |
| 10:13:53 | gibi | and live migration is not supported for flavor based PCI so that is super easy :D | |
| 10:14:31 | gibi | then I will look at resize revert, reschedule, and multi create in this order | |
| 10:15:14 | sean-k-mooney | lol yep live migration should be trivial :P | |
| 10:15:45 | sean-k-mooney | fortunetly you can also verify them on real hardware for a change too once its working in the func test env | |
| 10:15:58 | gibi | yes I will do a final round in the lab too | |
| 10:16:17 | sean-k-mooney | i still need to go extend the reservation fo those nodes | |
| 10:16:20 | sean-k-mooney | ill do that now | |
| 10:16:22 | gibi | thanks | |
| 10:16:52 | gibi | I use those nodes to run the func and unit tests in bluk too as it is a lot faster there :D | |
| 10:17:24 | gibi | I rsync up my local dev repo and run tox via ssh | |
| 10:17:31 | sean-k-mooney | yep thats why i often do dev on my home server | |
| 10:17:49 | opendevreview | ribaudr proposed openstack/nova master: Default Nova persistent objects without soft delete. https://review.opendev.org/c/openstack/nova/+/854355 | |
| 10:18:05 | sean-k-mooney | its avaiabel until 02-Nov-2022 but that seem longer then we need any prefered end date | |