)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e1fbc3294c1196be1641022285ed260c836f1b13","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e6233bdb_e3fc61ee","updated":"2026-08-07 15:07:06.000000000","message":"+1 on the approach — I diagnosed the same race from the 484729 gate failures\nand independently wrote the same fix, so I\u0027ll abandon my duplicate (1000102)\nin favour of this one, which was uploaded first. I\u0027ll also mark my bug\n#2163047 as a duplicate of #2163042. Four things below, one of which I think\nis worth a new patchset.\n\nThe one that matters: the tests here don\u0027t actually pin the fix. I checked by\nreverting the implementation to the read-modify-write cycle this change\nreplaces — keeping the coalesce and the negative guard, just moving the\narithmetic back into Python:\n\n    row \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id).first()\n    if row is None:\n        return\n    if row.num_dependent_backups is None and delta \u003c 0:\n        return\n    counter \u003d (row.num_dependent_backups or 0) + delta\n    if counter \u003c 0:\n        return\n    model_query(context, models.Backup).filter_by(id\u003dbackup_id).update(\n        {\u0027num_dependent_backups\u0027: counter}, synchronize_session\u003dFalse)\n\nAll 9 tests still pass — the 7 in test_backup_dependents.py and both new ones\nin test_backup.py. They verify the arithmetic, which was never the bug; they\ncan\u0027t see the atomicity, which was. A serial test fundamentally cannot tell\nthe two implementations apart, so the only thing that catches a regression\nhere is asserting on the statement the ORM emits.\n\nWhat I used, which does fail against the read-modify-write version above:\n\n    def _capture_backup_updates(self):\n        \"\"\"Collect the UPDATE statements issued against the backups table.\"\"\"\n        statements \u003d []\n\n        def record(conn, cursor, statement, params, context, executemany):\n            if statement.lstrip().upper().startswith(\u0027UPDATE BACKUPS\u0027):\n                statements.append(statement)\n\n        sqlalchemy.event.listen(sqla_engine.Engine, \u0027before_cursor_execute\u0027,\n                                record)\n        self.addCleanup(sqlalchemy.event.remove, sqla_engine.Engine,\n                        \u0027before_cursor_execute\u0027, record)\n        return statements\n\n    def test_adjust_is_atomic(self):\n        backup \u003d tests_utils.create_backup(self.ctxt)\n        db.backup_adjust_dependents(self.ctxt, backup.id, 1)\n        statements \u003d self._capture_backup_updates()\n\n        db.backup_adjust_dependents(self.ctxt, backup.id, -1)\n\n        self.assertEqual(1, len(statements))\n        # The right hand side has to reference the column itself, not a value\n        # the caller read beforehand.\n        assignment \u003d statements[0].split(\u0027SET\u0027, 1)[1].split(\u0027WHERE\u0027, 1)[0]\n        self.assertIn(\u0027coalesce(backups.num_dependent_backups\u0027,\n                      assignment.lower())\n\n    def test_floor_is_atomic(self):\n        backup \u003d tests_utils.create_backup(self.ctxt)\n        statements \u003d self._capture_backup_updates()\n\n        db.backup_adjust_dependents(self.ctxt, backup.id, -1)\n\n        # One statement that both tests and applies the decrement, rather than\n        # a read followed by a conditional write.\n        self.assertEqual(1, len(statements))\n        condition \u003d statements[0].split(\u0027WHERE\u0027, 1)[1].lower()\n        self.assertIn(\u0027coalesce(backups.num_dependent_backups\u0027, condition)\n\nNeeds `import sqlalchemy` and `from sqlalchemy import engine as sqla_engine`.\nTake it or leave it — the point is that something has to assert the shape of\nthe UPDATE, or the next refactor can quietly reintroduce the bug with a green\ngate.","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"4b8f064aca4dd357d8407ebea38b416421523339","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"74f71c7b_5318e934","updated":"2026-08-07 14:44:16.000000000","message":"Damn - he beat me to it....","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e32365c9d1cfd489897fa3a8db3ec72ebfe46278","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cd71d899_957d3240","in_reply_to":"e6233bdb_e3fc61ee","updated":"2026-08-07 15:43:13.000000000","message":"Done","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ad5857fc_28b8343b","updated":"2026-08-10 13:07:23.000000000","message":"-1 for the release note; a few spelling nits and a unit test concern noted inline.  As far as I can tell, the code change looks fine, so my concerns could be addressed in a followup patch, especially as this is blocking several projects\u0027 gates.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac1d505ea1c9230f348900dc3b2eb2594871e6a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e6f27152_4d0e02d3","updated":"2026-08-10 13:06:36.000000000","message":"Couple of small nots","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"260a86449353c46668948e7c2f311a761215d164","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a69b87bb_946f47fd","updated":"2026-08-07 15:53:03.000000000","message":"Thanks @simon@purestorage.com for reviewing \u0026 updating the patch with your findings, much appreciated!","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3ca2a01b30529b008c99156fff3e2e05772052c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6d40b59d_e8b9cadd","in_reply_to":"e6f27152_4d0e02d3","updated":"2026-08-10 13:06:48.000000000","message":"*notes","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef9488f72e399ee56b3d591b799701f20eb118c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2b6bdca9_15c9101e","updated":"2026-08-10 14:48:15.000000000","message":"Okay, this looks good to me now","commit_id":"3357cef2e7e32264ff6fd62def8eba0bc0a7c53e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3468153b9619c493677bd931d5ab0b84b411b5cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ffaf0ad9_f0bae967","updated":"2026-08-10 17:57:25.000000000","message":"Ninja-approving to clear the gate.","commit_id":"4a2600ec321ca809f7a47b9880a4854c9eda0cad"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1662597df4ba1d2ba505fad5f9e4dc6cbb61e484","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1bcae678_37c2488f","updated":"2026-08-10 15:06:57.000000000","message":"Thanks for the quick revisions! LGTM.","commit_id":"4a2600ec321ca809f7a47b9880a4854c9eda0cad"}],"cinder/backup/manager.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e1fbc3294c1196be1641022285ed260c836f1b13","unresolved":true,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"from cinder.backup import rpcapi as backup_rpcapi"},{"line_number":50,"context_line":"from cinder import context"},{"line_number":51,"context_line":"from cinder.db import api as db"},{"line_number":52,"context_line":"from cinder import exception"},{"line_number":53,"context_line":"from cinder.i18n import _"},{"line_number":54,"context_line":"from cinder.keymgr import migration as key_migration"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fbd860b_2c490806","line":51,"updated":"2026-08-07 15:07:06.000000000","message":"BackupManager already has this module as self.db — cinder/db/base.py:28 sets\nself.db \u003d cinder.db.api, and backup/manager.py uses self.db. twelve times\nalready. So this import is a second handle on the same object, and the two\ncall sites below could just be self.db.backup_adjust_dependents(...). Purely\ncosmetic, ignore if you prefer the explicit import.","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"260a86449353c46668948e7c2f311a761215d164","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"from cinder.backup import rpcapi as backup_rpcapi"},{"line_number":50,"context_line":"from cinder import context"},{"line_number":51,"context_line":"from cinder.db import api as db"},{"line_number":52,"context_line":"from cinder import exception"},{"line_number":53,"context_line":"from cinder.i18n import _"},{"line_number":54,"context_line":"from cinder.keymgr import migration as key_migration"}],"source_content_type":"text/x-python","patch_set":2,"id":"a9eb0e74_29b1c263","line":51,"in_reply_to":"9fbd860b_2c490806","updated":"2026-08-07 15:53:03.000000000","message":"Acknowledged","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e32365c9d1cfd489897fa3a8db3ec72ebfe46278","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"from cinder.backup import rpcapi as backup_rpcapi"},{"line_number":50,"context_line":"from cinder import context"},{"line_number":51,"context_line":"from cinder.db import api as db"},{"line_number":52,"context_line":"from cinder import exception"},{"line_number":53,"context_line":"from cinder.i18n import _"},{"line_number":54,"context_line":"from cinder.keymgr import migration as key_migration"}],"source_content_type":"text/x-python","patch_set":2,"id":"45b834cf_da66f77f","line":51,"in_reply_to":"9fbd860b_2c490806","updated":"2026-08-07 15:43:13.000000000","message":"Done","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"}],"cinder/db/api.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e1fbc3294c1196be1641022285ed260c836f1b13","unresolved":true,"context_lines":[{"line_number":6581,"context_line":""},{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@main_context_manager.writer"},{"line_number":6584,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6585,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6586,"context_line":""},{"line_number":6587,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":2,"id":"b460e2a6_1fb83658","line":6584,"updated":"2026-08-07 15:07:06.000000000","message":"Worth adding the retry decorator the other writers in this module use:\n\n    @require_context\n    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)\n    @main_context_manager.writer\n    def backup_adjust_dependents(context, backup_id, delta):\n\nA single-row UPDATE narrows the window a lot but doesn\u0027t close it — under\nMySQL/Galera this can still come back as a deadlock or a write conflict, and\nlosing the retry means losing exactly the update this change exists to\nprotect. There\u0027s precedent throughout db/api.py (backup_metadata_update,\ntransfer_accept and half a dozen others). Note the decorator has to sit\noutside main_context_manager.writer so the retry re-runs the transaction.","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"260a86449353c46668948e7c2f311a761215d164","unresolved":false,"context_lines":[{"line_number":6581,"context_line":""},{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@main_context_manager.writer"},{"line_number":6584,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6585,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6586,"context_line":""},{"line_number":6587,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":2,"id":"122c0cc4_b36c24ee","line":6584,"in_reply_to":"b460e2a6_1fb83658","updated":"2026-08-07 15:53:03.000000000","message":"Acknowledged","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e32365c9d1cfd489897fa3a8db3ec72ebfe46278","unresolved":false,"context_lines":[{"line_number":6581,"context_line":""},{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@main_context_manager.writer"},{"line_number":6584,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6585,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6586,"context_line":""},{"line_number":6587,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":2,"id":"d4ef3bb2_875cf6fb","line":6584,"in_reply_to":"b460e2a6_1fb83658","updated":"2026-08-07 15:43:13.000000000","message":"Done","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac1d505ea1c9230f348900dc3b2eb2594871e6a0","unresolved":true,"context_lines":[{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":6584,"context_line":"@main_context_manager.writer"},{"line_number":6585,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6586,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6587,"context_line":""},{"line_number":6588,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":3,"id":"207ef7a0_89e09a82","line":6585,"updated":"2026-08-10 13:06:36.000000000","message":"nit: given delta will always be +1/-1, would it be make sense to split the public interface into `backup_add_dependent`/`backup_remove_dependent`? You could have them both use a common `_backup_adjust_dependents` method if needed","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":false,"context_lines":[{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":6584,"context_line":"@main_context_manager.writer"},{"line_number":6585,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6586,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6587,"context_line":""},{"line_number":6588,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":3,"id":"85750f3a_e5780d78","line":6585,"in_reply_to":"207ef7a0_89e09a82","updated":"2026-08-10 14:06:13.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":6582,"context_line":"@require_context"},{"line_number":6583,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":6584,"context_line":"@main_context_manager.writer"},{"line_number":6585,"context_line":"def backup_adjust_dependents(context, backup_id, delta):"},{"line_number":6586,"context_line":"    \"\"\"Atomically add delta to a backup\u0027s num_dependent_backups."},{"line_number":6587,"context_line":""},{"line_number":6588,"context_line":"    Done in the DB so concurrent dependants don\u0027t lose each other\u0027s updates."}],"source_content_type":"text/x-python","patch_set":3,"id":"b157a811_b7d371d3","line":6585,"in_reply_to":"207ef7a0_89e09a82","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":true,"context_lines":[{"line_number":6592,"context_line":"    A single statement narrows the window but does not close it: on"},{"line_number":6593,"context_line":"    MySQL/Galera concurrent dependants can still collide as a deadlock or a"},{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"0f6e5bff_82d024f4","line":6595,"updated":"2026-08-10 13:07:23.000000000","message":"nit: s/dependants/dependents/ on lines 6588 and 6593","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":6592,"context_line":"    A single statement narrows the window but does not close it: on"},{"line_number":6593,"context_line":"    MySQL/Galera concurrent dependants can still collide as a deadlock or a"},{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"662b01e3_a5543f0d","line":6595,"in_reply_to":"0f6e5bff_82d024f4","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":false,"context_lines":[{"line_number":6592,"context_line":"    A single statement narrows the window but does not close it: on"},{"line_number":6593,"context_line":"    MySQL/Galera concurrent dependants can still collide as a deadlock or a"},{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ad271a9f_a0bfcf92","line":6595,"in_reply_to":"0f6e5bff_82d024f4","updated":"2026-08-10 14:06:13.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac1d505ea1c9230f348900dc3b2eb2594871e6a0","unresolved":true,"context_lines":[{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":6599,"context_line":"    if delta \u003c 0:"},{"line_number":6600,"context_line":"        query \u003d query.filter(counter \u003e\u003d -delta)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b605f505_8adf76d4","line":6597,"updated":"2026-08-10 13:06:36.000000000","message":"Can you add a comment here indicating that this is here because the database defaults to `NULL`, not `0`? Any maybe a `TODO` if you plan on changing that.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":true,"context_lines":[{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":6599,"context_line":"    if delta \u003c 0:"},{"line_number":6600,"context_line":"        query \u003d query.filter(counter \u003e\u003d -delta)"}],"source_content_type":"text/x-python","patch_set":3,"id":"be0f1fff_2d3f158a","line":6597,"in_reply_to":"b605f505_8adf76d4","updated":"2026-08-10 14:06:13.000000000","message":"Actually, we can drop the `coalesce` and associated comments since rosmaita has noted the migrations actually set a default here","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":6594,"context_line":"    write conflict, and losing that update is the very thing this guards"},{"line_number":6595,"context_line":"    against, so retry it."},{"line_number":6596,"context_line":"    \"\"\""},{"line_number":6597,"context_line":"    counter \u003d sa.func.coalesce(models.Backup.num_dependent_backups, 0)"},{"line_number":6598,"context_line":"    query \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":6599,"context_line":"    if delta \u003c 0:"},{"line_number":6600,"context_line":"        query \u003d query.filter(counter \u003e\u003d -delta)"}],"source_content_type":"text/x-python","patch_set":3,"id":"76fe62f6_61efe205","line":6597,"in_reply_to":"b605f505_8adf76d4","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1662597df4ba1d2ba505fad5f9e4dc6cbb61e484","unresolved":false,"context_lines":[{"line_number":6587,"context_line":"    \"\"\""},{"line_number":6588,"context_line":"    # The model now defaults this column to 0, but that only applies at"},{"line_number":6589,"context_line":"    # insert time: rows written before then still hold NULL, and NULL + 1"},{"line_number":6590,"context_line":"    # would be NULL. Coalesce so the arithmetic counts from zero either way."},{"line_number":6591,"context_line":"    # TODO(sdodsley): a follow-up can backfill the remaining NULLs with"},{"line_number":6592,"context_line":"    # UPDATE backups SET num_dependent_backups \u003d 0 WHERE"},{"line_number":6593,"context_line":"    # num_dependent_backups IS NULL, after which this coalesce can go. That"}],"source_content_type":"text/x-python","patch_set":6,"id":"25b22b18_f288bbcd","line":6590,"updated":"2026-08-10 15:06:57.000000000","message":"Note to reviewers: see the discussion in the init migration on PS3 for why we need the coalesce here even though the init migration has default\u003d0; since it was missing on the model, it never got inserted despite what the migration specified.","commit_id":"4a2600ec321ca809f7a47b9880a4854c9eda0cad"}],"cinder/db/migrations/versions/921e1a36b076_initial.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"3537b7db_5935ee4b","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"updated":"2026-08-10 13:07:23.000000000","message":"Hmmm ... we already set 0 as the default value, so I wouldn\u0027t expect to see NULL in there, even though the column is nullable. But the key point is we don\u0027t need to do a db migration to get default\u003d0 for this patch.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"0c101789_efe90ed6","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"16f420b3_6adf1c57","updated":"2026-08-10 14:07:15.000000000","message":"Checked this on clean master: a newly created backup row\nholds NULL, and the DDL is a bare INTEGER with no DEFAULT.\n\nThe default\u003d0 in both init migrations is a SQLAlchemy *client-side* default —\nevaluated in Python at INSERT time only if the ORM model carries it, and it\nemits no DDL. Only server_default\u003d produces a DEFAULT clause. models.py had no\ndefault\u003d either until this patch adds one, so nothing ever supplied the value\nand the INSERT omits the column.\n\nSo NULL isn\u0027t a leftover from an old release, it\u0027s the state of every backup\nthat has never had a dependent — nearly all of them. That makes the coalesce\nload-bearing: without it NULL + 1 is NULL, and the first dependent of any\nexisting backup would silently fail to register.\n\nAgreed no migration is needed for this patch. PS4 carries a TODO for the\nfollow-up Stephen suggested: backfill the NULLs, add a server default, then\nthe coalesce can go.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d7361b3a6a204f999da58ea50e363c8c11bf5ec1","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"548ecd5f_1bfcf242","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"16f420b3_6adf1c57","updated":"2026-08-10 14:07:18.000000000","message":"Yes, I agree we should drop the `coalesce`. It\u0027s not needed as the database has always defaulted to `0` (per the migrations).","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"33163140ddb6143ae12777f335d65c32574250ee","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"b05c285f_6f5fe55a","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"2714509b_227d0646","updated":"2026-08-10 14:47:32.000000000","message":"Yeah, sorry, we crossed. This makes sense.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0291d6581fb130c03d787cfd909651ed3f0d4a60","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"16f420b3_6adf1c57","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"3537b7db_5935ee4b","updated":"2026-08-10 13:41:57.000000000","message":"This was also done in the legacy db init: https://opendev.org/openstack/cinder/src/tag/xena-eol/cinder/db/legacy_migrations/versions/135_cinder_init.py#L539\n\nMy point in bringing this up is that maybe we don\u0027t need the complexity of the coalesce? On the other hand, since the column is nullable, maybe it\u0027s safer to have the coalesce.  But in the normal run of code, there should never be a NULL in that column.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"399f2a660aaae3cea8aba81822fb702eb801361a","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"69b55077_17153cc6","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"548ecd5f_1bfcf242","updated":"2026-08-10 14:12:31.000000000","message":"@stephenfin@redhat.com Read the above comment - we may have crossed...","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"f2df45e46db15605c139ae28d6306909bbd13b2c","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        sa.Column(\u0027parent_id\u0027, sa.String(36)),"},{"line_number":572,"context_line":"        sa.Column(\u0027temp_volume_id\u0027, sa.String(36)),"},{"line_number":573,"context_line":"        sa.Column(\u0027temp_snapshot_id\u0027, sa.String(36)),"},{"line_number":574,"context_line":"        sa.Column(\u0027num_dependent_backups\u0027, sa.Integer, default\u003d0),"},{"line_number":575,"context_line":"        sa.Column(\u0027snapshot_id\u0027, sa.String(36)),"},{"line_number":576,"context_line":"        sa.Column(\u0027data_timestamp\u0027, sa.DateTime),"},{"line_number":577,"context_line":"        sa.Column(\u0027restore_volume_id\u0027, sa.String(36)),"}],"source_content_type":"text/x-python","patch_set":3,"id":"2714509b_227d0646","line":574,"range":{"start_line":574,"start_character":54,"end_line":574,"end_character":64},"in_reply_to":"69b55077_17153cc6","updated":"2026-08-10 14:18:06.000000000","message":"OK - going to drop the default\u003d0 in the models.py in PS5","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"}],"cinder/db/models.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac1d505ea1c9230f348900dc3b2eb2594871e6a0","unresolved":true,"context_lines":[{"line_number":1040,"context_line":"    object_count \u003d sa.Column(sa.Integer)"},{"line_number":1041,"context_line":"    temp_volume_id \u003d sa.Column(sa.String(36))"},{"line_number":1042,"context_line":"    temp_snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1043,"context_line":"    num_dependent_backups \u003d sa.Column(sa.Integer, default\u003d0)"},{"line_number":1044,"context_line":"    snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1045,"context_line":"    data_timestamp \u003d sa.Column(sa.DateTime)"},{"line_number":1046,"context_line":"    restore_volume_id \u003d sa.Column(sa.String(36))"}],"source_content_type":"text/x-python","patch_set":3,"id":"4ea533af_4e7b19e7","line":1043,"updated":"2026-08-10 13:06:36.000000000","message":"Doesn\u0027t this need a database migration? This will currently be `NULL`, right?\n\nLater: Ah, I see you\u0027re handling this with [coalesce](https://review.opendev.org/c/openstack/cinder/+/1000097/3/cinder/db/api.py#6597). Maybe punt this to a separate, follow-up PR and add a migration? This allows us to remove the `coalesce` in a future change then. You might also be able to fix the upgrade impact noted in the reno then too.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"    object_count \u003d sa.Column(sa.Integer)"},{"line_number":1041,"context_line":"    temp_volume_id \u003d sa.Column(sa.String(36))"},{"line_number":1042,"context_line":"    temp_snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1043,"context_line":"    num_dependent_backups \u003d sa.Column(sa.Integer, default\u003d0)"},{"line_number":1044,"context_line":"    snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1045,"context_line":"    data_timestamp \u003d sa.Column(sa.DateTime)"},{"line_number":1046,"context_line":"    restore_volume_id \u003d sa.Column(sa.String(36))"}],"source_content_type":"text/x-python","patch_set":3,"id":"0b235bdc_1d3f8b4e","line":1043,"in_reply_to":"4ea533af_4e7b19e7","updated":"2026-08-10 14:07:15.000000000","message":"Acknowledged","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":true,"context_lines":[{"line_number":1040,"context_line":"    object_count \u003d sa.Column(sa.Integer)"},{"line_number":1041,"context_line":"    temp_volume_id \u003d sa.Column(sa.String(36))"},{"line_number":1042,"context_line":"    temp_snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1043,"context_line":"    num_dependent_backups \u003d sa.Column(sa.Integer, default\u003d0)"},{"line_number":1044,"context_line":"    snapshot_id \u003d sa.Column(sa.String(36))"},{"line_number":1045,"context_line":"    data_timestamp \u003d sa.Column(sa.DateTime)"},{"line_number":1046,"context_line":"    restore_volume_id \u003d sa.Column(sa.String(36))"}],"source_content_type":"text/x-python","patch_set":3,"id":"94dc4e4f_b8f7d981","line":1043,"in_reply_to":"4ea533af_4e7b19e7","updated":"2026-08-10 14:06:13.000000000","message":"Actually, per rosmaita\u0027s comments, this is just aligning the model with the migrations (and therefore the underlying database). We should just call this out in the commit message for future us.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"}],"cinder/tests/unit/backup/test_backup.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":true,"context_lines":[{"line_number":1708,"context_line":"        backup.save()"},{"line_number":1709,"context_line":"        self.backup_mgr.delete_backup(self.ctxt, backup)"},{"line_number":1710,"context_line":""},{"line_number":1711,"context_line":"    def test_finish_backup_counts_dependant(self):"},{"line_number":1712,"context_line":"        \"\"\"Completing an incremental backup counts it on its parent.\"\"\""},{"line_number":1713,"context_line":"        vol_id \u003d self._create_volume_db_entry(size\u003d1)"},{"line_number":1714,"context_line":"        parent \u003d self._create_backup_db_entry(volume_id\u003dvol_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f1131696_42fb236b","line":1711,"range":{"start_line":1711,"start_character":34,"end_line":1711,"end_character":43},"updated":"2026-08-10 13:07:23.000000000","message":"nit: would be good to use the same spelling as used in the num_dependent_backups field.  (Same issue on line 1724)","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":false,"context_lines":[{"line_number":1708,"context_line":"        backup.save()"},{"line_number":1709,"context_line":"        self.backup_mgr.delete_backup(self.ctxt, backup)"},{"line_number":1710,"context_line":""},{"line_number":1711,"context_line":"    def test_finish_backup_counts_dependant(self):"},{"line_number":1712,"context_line":"        \"\"\"Completing an incremental backup counts it on its parent.\"\"\""},{"line_number":1713,"context_line":"        vol_id \u003d self._create_volume_db_entry(size\u003d1)"},{"line_number":1714,"context_line":"        parent \u003d self._create_backup_db_entry(volume_id\u003dvol_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"31940ce2_2a6c24e5","line":1711,"range":{"start_line":1711,"start_character":34,"end_line":1711,"end_character":43},"in_reply_to":"f1131696_42fb236b","updated":"2026-08-10 14:06:13.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":1708,"context_line":"        backup.save()"},{"line_number":1709,"context_line":"        self.backup_mgr.delete_backup(self.ctxt, backup)"},{"line_number":1710,"context_line":""},{"line_number":1711,"context_line":"    def test_finish_backup_counts_dependant(self):"},{"line_number":1712,"context_line":"        \"\"\"Completing an incremental backup counts it on its parent.\"\"\""},{"line_number":1713,"context_line":"        vol_id \u003d self._create_volume_db_entry(size\u003d1)"},{"line_number":1714,"context_line":"        parent \u003d self._create_backup_db_entry(volume_id\u003dvol_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"68935164_4668bc99","line":1711,"range":{"start_line":1711,"start_character":34,"end_line":1711,"end_character":43},"in_reply_to":"f1131696_42fb236b","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"}],"cinder/tests/unit/test_backup_dependents.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac1d505ea1c9230f348900dc3b2eb2594871e6a0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e9a52961_175e8b10","updated":"2026-08-10 13:06:36.000000000","message":"This belongs in `cinder/tests/unit/db`, not the root directory","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"096d51ee_f1eb5881","in_reply_to":"e9a52961_175e8b10","updated":"2026-08-10 14:07:15.000000000","message":"Acknowledged","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d7361b3a6a204f999da58ea50e363c8c11bf5ec1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5a6d0318_579cc8bb","in_reply_to":"e9a52961_175e8b10","updated":"2026-08-10 14:07:18.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":true,"context_lines":[{"line_number":80,"context_line":"    def test_decrement(self):"},{"line_number":81,"context_line":"        backup \u003d tests_utils.create_backup(self.ctxt)"},{"line_number":82,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, 1)"},{"line_number":83,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, 1)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, -1)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"04145524_109aff84","line":83,"updated":"2026-08-10 13:07:23.000000000","message":"nit: I don\u0027t like this pattern where the method we\u0027re testing is being used to set up the test context, because by hypothesis, we don\u0027t know for sure that the method works.  I\u0027d prefer using something along the lines of your _set_null() to put the value we want directly into the db, and then use backup_adjust_dependents() to decrement (since that\u0027s what we\u0027re testing here).  Same thing for the other tests below.","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    def test_decrement(self):"},{"line_number":81,"context_line":"        backup \u003d tests_utils.create_backup(self.ctxt)"},{"line_number":82,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, 1)"},{"line_number":83,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, 1)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        db.backup_adjust_dependents(self.ctxt, backup.id, -1)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"27e5d084_8a4b4e0b","line":83,"in_reply_to":"04145524_109aff84","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"}],"releasenotes/notes/backup-num-dependent-backups-atomic-7c1f0a9d24b8e3f5.yaml":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e1fbc3294c1196be1641022285ed260c836f1b13","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8516ced2_257e6c14","line":9,"updated":"2026-08-07 15:07:06.000000000","message":"Could this mention that counters which have already drifted aren\u0027t repaired?\nThe fix stops new drift, but nothing recomputes existing values, so anyone who\nhas already hit this upgrades and still has undeletable backups with no\nindication why. Something like:\n\n    upgrade:\n      - |\n        This stops the dependent-backup counter from drifting, but does not\n        repair values that have already drifted. A backup that wrongly reports\n        dependent backups, and so cannot be deleted, will continue to do so\n        until its row is corrected in the database.\n\nPossible follow-ups, not for this change: a cinder-manage subcommand that\nrecomputes the counter from the child rows, or dropping the stored counter and\nderiving has_dependent_backups from an EXISTS query on parent_id, which would\nbe self-healing. The latter is bigger — the column is a versioned object field\nand appears in notification payloads via volume_utils — so it needs a\ncompatibility path.","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"260a86449353c46668948e7c2f311a761215d164","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"97acb97c_ebaccadd","line":9,"in_reply_to":"8516ced2_257e6c14","updated":"2026-08-07 15:53:03.000000000","message":"Acknowledged","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e32365c9d1cfd489897fa3a8db3ec72ebfe46278","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"84f1196f_b357f641","line":9,"in_reply_to":"8516ced2_257e6c14","updated":"2026-08-07 15:43:13.000000000","message":"Done","commit_id":"5eabc816659a7fa6ac951e1d5bd17dfbe4eee5c2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ab96be95b921e7660515075669993964a03c01","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    database."},{"line_number":9,"context_line":"upgrade:"},{"line_number":10,"context_line":"  - |"},{"line_number":11,"context_line":"    This stops the dependent-backup counter (``num_dependent_backups``) from"},{"line_number":12,"context_line":"    drifting, but does not repair values that have already drifted. A backup"},{"line_number":13,"context_line":"    that wrongly reports dependent backups, and so cannot be deleted, will"},{"line_number":14,"context_line":"    continue to do so until its row is corrected in the database."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"e318ac6e_927de0ab","line":11,"range":{"start_line":11,"start_character":4,"end_line":11,"end_character":8},"updated":"2026-08-10 13:07:23.000000000","message":"remember, the \"upgrade notes\" and \"bug fixes\" are spread around in the generated release notes, so it won\u0027t be clear what \"This\" is referring to.  I suggest replacing \"This\" with something like\n\n``The fix for `Bug #2163042 \u003chttps://bugs.launchpad.net/cinder/+bug/2163042\u003e`_ stops ...``","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"25efeaeee87be6628b64b0fbee595cf7e25e1584","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    database."},{"line_number":9,"context_line":"upgrade:"},{"line_number":10,"context_line":"  - |"},{"line_number":11,"context_line":"    This stops the dependent-backup counter (``num_dependent_backups``) from"},{"line_number":12,"context_line":"    drifting, but does not repair values that have already drifted. A backup"},{"line_number":13,"context_line":"    that wrongly reports dependent backups, and so cannot be deleted, will"},{"line_number":14,"context_line":"    continue to do so until its row is corrected in the database."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1caf1815_68154e09","line":11,"range":{"start_line":11,"start_character":4,"end_line":11,"end_character":8},"in_reply_to":"e318ac6e_927de0ab","updated":"2026-08-10 14:06:13.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"115d2e179f11d3b34fdff513c2fd81281bedd246","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    database."},{"line_number":9,"context_line":"upgrade:"},{"line_number":10,"context_line":"  - |"},{"line_number":11,"context_line":"    This stops the dependent-backup counter (``num_dependent_backups``) from"},{"line_number":12,"context_line":"    drifting, but does not repair values that have already drifted. A backup"},{"line_number":13,"context_line":"    that wrongly reports dependent backups, and so cannot be deleted, will"},{"line_number":14,"context_line":"    continue to do so until its row is corrected in the database."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ca1fffa4_99903cc5","line":11,"range":{"start_line":11,"start_character":4,"end_line":11,"end_character":8},"in_reply_to":"e318ac6e_927de0ab","updated":"2026-08-10 14:07:15.000000000","message":"Done","commit_id":"cd07780e519b8919bbc3ebaf6357a920f04163ac"}]}