)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a2358cdaf4e7d828f6ac1105ce393481f44a21c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a762f2ce_daaf4f6a","updated":"2022-10-05 12:26:30.000000000","message":"@zzzeek Would be interested in your thoughts on this. I\u0027ve proposed a similar change in nova but suspect other projects would benefit from this.","commit_id":"c8d210ccb0c2d3b1633cab425c61e9b22e4cab6f"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"e53db5b7199fb863abcb7ab8a00889a26f071fdf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6476059d_a26f575d","updated":"2022-10-05 14:22:11.000000000","message":"I\u0027m pretty skeptical on this one.  Whether a model does \"soft delete\" or \"real delete\" tends to be kind of a Big Deal, with direct implications for database integrity and such (think rows that are referenced by foreign key) as well as transactional isolation behaviors, and doesn\u0027t seem like something that end users would be flipping on and off in the configuration file.    Most concerning would be that applications would not be fully tested with the configuration in both positions, as this is a major, fundamental behavioral change.\n\nRealistically, it\u0027s the kind of thing you\u0027d be configuring on a per-model basis in a specific app for specific models where people want an option here, and if the app is exposing this through the conf file, it would need to have full test coverage for both scenarios.   Even then, I think it\u0027s a very difficult feature to support since supporting two different scenarios of deletion introduces a lot of behavioral branches.\n\n\nI\u0027d want to start with the background and problem space on this one and figure out the best solution.   My impression is that in practice, \"soft delete\" has not been very useful and we\u0027d like to be using \"hard delete\" so that we don\u0027t have tons of old records lying around.   I think making a real shift towards that model, *or* at least moving to an \"archive row\" model, where deleted rows are moved out to archive tables that are easier to truncate, would likely be a more robust course of action.\n","commit_id":"c8d210ccb0c2d3b1633cab425c61e9b22e4cab6f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2d5268eff50f0e571889d5490f09ff7c3d8deeb2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4957e386_d06ec72a","updated":"2022-10-06 09:25:05.000000000","message":"Yup, definitely need to think about this one more.","commit_id":"c8d210ccb0c2d3b1633cab425c61e9b22e4cab6f"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f663b25f9da16086aed8ead1e820f6099e83f47e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cb186f92_0ee84cee","updated":"2022-10-06 12:40:32.000000000","message":"why not just remove soft delete from nova altogether?  \n\nas for the config here, it\u0027s one thing if it works for Nova, OK.  but I\u0027d be really nervous about like, Neutron flipping *anything*.  neutron probably isn\u0027t using soft delete, I\u0027d have to check.    Soft delete is a real matter of preference, so overall while this class is here, doesnt feel like an \"oslo.db\" thing to me.   Maybe a class-level attribute that Nova can configure instead.","commit_id":"c8d210ccb0c2d3b1633cab425c61e9b22e4cab6f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2d5268eff50f0e571889d5490f09ff7c3d8deeb2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3f490269_1f09f467","in_reply_to":"6476059d_a26f575d","updated":"2022-10-06 09:25:05.000000000","message":"\u003e I\u0027m pretty skeptical on this one.  Whether a model does \"soft delete\" or \"real delete\" tends to be kind of a Big Deal, with direct implications for database integrity and such (think rows that are referenced by foreign key) as well as transactional isolation behaviors, and doesn\u0027t seem like something that end users would be flipping on and off in the configuration file.\n\nThis is a good point. I know that nova historically avoided use of foreign key constraints, meaning we wouldn\u0027t be able to rely on DELETE CASCADE behavior to delete e.g. everything that points to a newly deleted instance. I\u0027ll have to inspect how nova is doing it\u0027s DB archive though to see how this is figured out.\n\n\u003e Most concerning would be that applications would not be fully tested with the configuration in both positions, as this is a major, fundamental behavioral change.\n\nAlso a valid point. I _suspect_ nova could immediately toggle this in CI jobs to test the \"new way\" while relying on our existing unit and functional tests for the DB archive functionality to ensure nothing breaks with the \"old way\". Can\u0027t say this for every project though.\n\n\u003e Realistically, it\u0027s the kind of thing you\u0027d be configuring on a per-model basis in a specific app for specific models where people want an option here, and if the app is exposing this through the conf file, it would need to have full test coverage for both scenarios.   Even then, I think it\u0027s a very difficult feature to support since supporting two different scenarios of deletion introduces a lot of behavioral branches.\n\u003e \n\u003e \n\u003e I\u0027d want to start with the background and problem space on this one and figure out the best solution.   My impression is that in practice, \"soft delete\" has not been very useful and we\u0027d like to be using \"hard delete\" so that we don\u0027t have tons of old records lying around.\n\n👆 yeah, this. See also [1] from waaay back (feeling old yet?)\n\n\u003e I think making a real shift towards that model, *or* at least moving to an \"archive row\" model, where deleted rows are moved out to archive tables that are easier to truncate, would likely be a more robust course of action.\n\nFunnily enough this is what nova has - namely, you\u0027ve to run two commands to first archive rows and then truncate the archive tables - and there are complaints that the prune functionality found in other projects (straight up DELETEs) is simpler an therefore preferable.\n\n(As an aside, one issue we have found with the archiving model is that MySQL resets the auto-increment counter after the service is restarted. If you delete a row with ID N and \u0027max(ID for each remaining row) \u003c N\u0027, then an ID of N will eventually be assigned to a new row and you\u0027ll get a conflict when you try to archive that new row. Great fun)\n\n[1] https://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/no-more-soft-delete.html","commit_id":"c8d210ccb0c2d3b1633cab425c61e9b22e4cab6f"}]}