)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7d344f6a55df0f8935fa1d8e3c62da60910fa292","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2162f2a2_03448158","updated":"2022-10-05 12:26:59.000000000","message":"Proposed an alternative in oslo.db at [1]\n\n[1] https://review.opendev.org/c/openstack/oslo.db/+/860407","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e66ca93d57b826491df3a9b4e88df8a032b5bf2a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"95e056a7_4f687d02","updated":"2022-10-06 07:54:49.000000000","message":"Soft -1 but here I\u0027d prefer us to find a consensus before we accept anything, as there are multiple alternatives I\u0027d personnally prefer.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6ff04f04d84a33ff65f08e6c3956ea194a2a915c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c56c7fc6_643003d2","updated":"2022-10-05 12:16:36.000000000","message":"im generally quite supportive of this.\n\nwe discussed this on irc a bit and i think we shoudl discuss it at the PTG\n\ni would be in favor of intoducign this as enabled in A and disabling the feature in b/c by default.\n\none of the topics we coverd on irc is this would not break the instance soft-delete feature as that does not set teh instace record to deleted.\nit sets the vm_state to soft_deleted\n\nso this chagne would not have any user visable api impact.\n\nwhile i think this need at least a specless blueprint and ptg discussion to get agreement on the direction im not sure this need a spec.\n\nother then perhaps more testing and additional docs this feel pretty complete to me.\nfor example we shoudl note that if you disabled soft-delete it is no longer required to arcive deleted rows or puge the shaddow tabels in a cron job.\n\nwe should also call out the implications for if you are using those for audit reasons\n\n\nthe other design approch i think we could take is instead of makign this a boolean we could  have a periodic that would do it after x number of days.\n\nbut i woudl be supprotive of keeping the scope small and advising that if you want the time based approch you use https://github.com/ovh/osarchiver/ to achive that be keepign soft delete on and having it delete or archive them via its configuration policy.\n\n\nfor that reason as there are 2 alterntives to the time based approch\nexternal tooling like osarchiver and the exsitng nova manage commands i think\nthere are not really any outstanding design considerations that would need a spec but we will see how other feels at ptg. \n","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"}],"releasenotes/notes/configurable-soft-delete-978cb724b572b37f.yaml":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e66ca93d57b826491df3a9b4e88df8a032b5bf2a","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ``True`` and the ``deleted_at`` column being populated for the"},{"line_number":9,"context_line":"    corresponding database row. However, when disabled, the row will simply"},{"line_number":10,"context_line":"    be deleted. Refer to the option documentation for more information on"},{"line_number":11,"context_line":"    the advantages and disadvantages of disabling soft delete."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"3b30945c_7f570159","line":11,"updated":"2022-10-06 07:54:49.000000000","message":"The problem I see here is that it would only delete records after this option is modified.\nIMO, we should be discussing this in a spec as we would have API concerns or upgrade ones.\nFor example, the concerns I have :\n- as this is a DB conf option, this means that all the services need to have this option. If for example an operator doesn\u0027t modify nova.conf for a nova-conductor but a nova-api service, then sometimes records could be deleted by the API while sometimes not.\n- why operators can\u0027t just run the archive and purge commands ?\n- could a nova-manage running daemon would better help instead of modifying this ?\n\nAs you see, we need to discuss about alternatives and we need to agree on a consensus.\nAs a result, this properly needs to be paperworked as a blueprint and probably requiring a spec.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58af0ca3e5265b3d73b1174dbf914ccb1582c677","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ``True`` and the ``deleted_at`` column being populated for the"},{"line_number":9,"context_line":"    corresponding database row. However, when disabled, the row will simply"},{"line_number":10,"context_line":"    be deleted. Refer to the option documentation for more information on"},{"line_number":11,"context_line":"    the advantages and disadvantages of disabling soft delete."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"9da5581b_7162f763","line":11,"in_reply_to":"2802d2fa_f26c5cd3","updated":"2022-10-06 10:09:24.000000000","message":"for what its worht i saw the fact that it would only delete rows after the option was modifed as a plus that avoid upgrade impact.\n\nregarding \nwhy operators can\u0027t just run the archive and purge commands ?\nthe can do and often get it wrong\n\nwe have had several custoemr issues over the year wehre they had misconfigured the cron jobs and had poor perforamce that resulted in customer cases that we had to traige and resolve.\n\nhaving nova self clean to me seams to be a bette experince for operators.\n\na adddtional deamon has been discussed in the past and that is an option as is usign external tooling.\n\nboth would be operationally more overhead and more complex solutions but are valid ones.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6d8270c717f0c18cdb11c4ebf348e2a6175017bc","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ``True`` and the ``deleted_at`` column being populated for the"},{"line_number":9,"context_line":"    corresponding database row. However, when disabled, the row will simply"},{"line_number":10,"context_line":"    be deleted. Refer to the option documentation for more information on"},{"line_number":11,"context_line":"    the advantages and disadvantages of disabling soft delete."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"2802d2fa_f26c5cd3","line":11,"in_reply_to":"3b30945c_7f570159","updated":"2022-10-06 09:14:15.000000000","message":"\u003e The problem I see here is that it would only delete records after this option is modified.\n\u003e IMO, we should be discussing this in a spec as we would have API concerns or upgrade ones.\n\u003e For example, the concerns I have :\n\u003e - as this is a DB conf option, this means that all the services need to have this option. If for example an operator doesn\u0027t modify nova.conf for a nova-conductor but a nova-api service, then sometimes records could be deleted by the API while sometimes not.\n\nRemember that the API database doesn\u0027t use soft-delete. This would only need to be set on the nodes running nova-conductor (i.e. those using the main database). As Sean noted in their review though, definitely need more docs here including a note that nova only uses soft delete in the main database.\n\n\u003e - why operators can\u0027t just run the archive and purge commands ?\n\nThey can. Per the help text for the option:\n\n  ... While \u0027nova-manage\u0027 provides tooling to archive and later purge\n  soft-deleted rows, this in turn adds additional operational overhead. Operators\n  may wish instead to disable this feature entirely and rely on other mechanisms\n  for auditing.\n\nThis config option is _optional_. Eventually we may wish to remove this functionality entirely, but I\u0027m not advocating for that position here. Let me know if this is not clear.\n\n\u003e - could a nova-manage running daemon would better help instead of modifying this ?\n\nWe don\u0027t need that: you can simply have a cron job that runs the \u0027nova-manage db archive_deleted_rows\u0027 and \u0027nova-manage db purge\u0027 commands periodically. As you probably know, recent versions of OSP configure such a job and I suspect most deployments have something similar. I\u0027m trying to solve a different problem here, namely I\u0027d like to find a way to avoid saving the things in the first place, meaning there\u0027s nothing to clean up in the first place.\n\n\u003e As you see, we need to discuss about alternatives and we need to agree on a consensus.\n\nYeah, I\u0027ve added it to the PTG agenda. I don\u0027t really think there are alternatives besides (a) the status quo or (b) removing soft-delete entirely, however.\n\n\u003e As a result, this properly needs to be paperworked as a blueprint and probably requiring a spec.\n\nI\u0027d hope to avoid that. Let\u0027s see how things go with the PTG slot.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0037e819f7c64922a35144e8614c91ec1e069942","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ``True`` and the ``deleted_at`` column being populated for the"},{"line_number":9,"context_line":"    corresponding database row. However, when disabled, the row will simply"},{"line_number":10,"context_line":"    be deleted. Refer to the option documentation for more information on"},{"line_number":11,"context_line":"    the advantages and disadvantages of disabling soft delete."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"b843a8e9_faefa136","line":11,"in_reply_to":"4d3de61b_b60979e2","updated":"2022-10-10 13:49:35.000000000","message":"So, let\u0027s just discuss it in the PTG.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8d2e63c6223096a0b5529feb5c828b00c0ff8f0e","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ``True`` and the ``deleted_at`` column being populated for the"},{"line_number":9,"context_line":"    corresponding database row. However, when disabled, the row will simply"},{"line_number":10,"context_line":"    be deleted. Refer to the option documentation for more information on"},{"line_number":11,"context_line":"    the advantages and disadvantages of disabling soft delete."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"4d3de61b_b60979e2","line":11,"in_reply_to":"9da5581b_7162f763","updated":"2022-10-06 21:16:00.000000000","message":"In my experience digging through DB dumps after something has gone way wrong, soft delete has been pretty valuable in deployment recovery scenarios ... that is why I hesitate with the idea of disabling it by default. I do see where you are coming from though, to avoid creating things that need to be cleaned up periodically in the first place.\n\nAt the moment, I would strongly recommend flipping this config option around and making it disable_soft_delete. We could ensure that operators fully understand the consequences of disabling it and if they accept the consequences, they can actively choose to do it. I imagine the config help explaining the pros and cons of disabling it to help operators make a decision. I do think some operators who have solid infra around auditing may like the ability to disable soft delete. In general, I think having it be configurable is a good idea.\n\nSpeaking of infra around auditing, I think if we had an alternative out-of-the-box mechanism included with nova for saving some history by default, I would feel a lot more comfortable with completely removing soft delete down the road. I\u0027m don\u0027t know if that would be stretching too far. We have versioned notifications which contain fields that map directly to DB record fields, IIUC. But that is not currently a default/turnkey setup. I\u0027m also not sure how efficient it would be loading up notifications from a log file, for example, and navigating them vs having an audit relational database for records.","commit_id":"ee78c2640db8d12fc9f6fd183c77c79088ac2e08"}]}