)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4036fadb9bfdb643281f33ad8ae256fd6f8f13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fdb2ded2_c5ac4096","updated":"2026-08-11 20:54:34.000000000","message":"I\u0027m still trying to understand when we want an `ExitStack` to wrap up all the brokers from `get_brokers()` and when we\u0027re OK to just do a `with self.get_brokers()[0] as broker:` or something.","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"374befbe8f6f5bcae1dcf6bcbfcf88ed9d540ea6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b6f19866_032fa693","in_reply_to":"fdb2ded2_c5ac4096","updated":"2026-08-12 13:56:10.000000000","message":"The idea was to close the brokers that were actually read\": get_brokers() builds new ContainerBrokers, `__init__` doesn\u0027t connect, so a broker you never touch holds no handle.\n\nPS2 replaces this with ContainerBroker.open_brokers(), which closes all of them, so this reads:\n\n```\nwith broker.open_brokers() as brokers:\n    source_broker \u003d brokers[0]\n```","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cc1fc747d1aa4359bba1dd01d34de16ac30bbf1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"00ccf5fe_8195a6b7","updated":"2026-08-12 17:34:05.000000000","message":"Do we have a sense of what kind of a lifecycle we *want* brokers/their conns to have? With the previous patch, I came away with a strong sense that any connection that got opened should get closed on the same greenthread, to ensure that we\u0027ve got a clean slate at the end of any request/response pair.\n\nThe daemons seem... messier, though. Looking at the container replicator for example, we spin up a bunch of greenthreads which each open up some brokers, open up a bunch of reconciler brokers on those same threads, then pop up *more* greenthreads to handle replicating the reconciler DBs to the right places...\n\nI was hoping to be able to apply something like https://paste.opendev.org/show/bB1E05emyWSrFWIyWryI/, run probe tests, and see that there weren\u0027t any new tracebacks, but I\u0027m having troubles -- both with supposedly unclosed connections *and* with trying to remove connections that weren\u0027t in the (corolocal) tracking list...\n\nAny ideas for where/how I could set up some more invasive conn-tracking that would let me leverage probe tests to gain confidence that we\u0027ve got everything covered?","commit_id":"b00de7faead868993730b67ca2a21619b6a82c8c"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"f81f15a4cd7f54b079273ee860dcaf09d96fce39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2667deb1_fa15e378","in_reply_to":"00ccf5fe_8195a6b7","updated":"2026-08-13 14:19:47.000000000","message":"\u003e Do we have a sense of what kind of a lifecycle we want brokers/their conns to have? With the previous patch, I came away with a strong sense that any connection that got opened should get closed on the same greenthread, to ensure that we\u0027ve got a clean slate at the end of any request/response pair.\n\nLooking at the container replicator that\u0027s not the case today on master?\n\n\u003e The daemons seem... messier, though. Looking at the container replicator for example, we spin up a bunch of greenthreads which each open up some brokers, open up a bunch of reconciler brokers on those same threads, then pop up more greenthreads to handle replicating the reconciler DBs to the right places...\n\nRight - do you want to change the lifecycle as well?\n\n\u003e Any ideas for where/how I could set up some more invasive conn-tracking that would let me leverage probe tests to gain confidence that we\u0027ve got everything covered?\n\nI added another test (test_replicate_reconcilers_closes_db_connections) - do you think this is sufficient?\n\nWe could also manually check with strace if there are any leaks, for example:\n```\n#!/usr/bin/env python3\n\"\"\"\nCheck an strace log for sqlite .db file descriptors that were opened but\nnever closed before the traced process exited.\n\nUsage:\n    strace -f -tt -e trace\u003dopenat,open,close -o /tmp/db-trace.log \\\\\n        swift-container-replicator /etc/swift/container-server/1.conf.d --once\n    python3 check_db_leaks.py /tmp/db-trace.log\n\n\"\"\"\nimport re\nimport sys\n\nOPEN_RE \u003d re.compile(r\u0027open(?:at)?\\(.*?\"([^\"]+\\.db)\".*?\u003d\\s*(\\d+)\\s*$\u0027)\nCLOSE_RE \u003d re.compile(r\u0027close\\((\\d+)\\)\\s*\u003d\\s*0\u0027)\n\n\nif __name__ \u003d\u003d \u0027__main__\u0027:\n    lines \u003d open(sys.argv[1])\n    open_fds \u003d {}\n    for line in lines:\n        m \u003d OPEN_RE.search(line)\n        if m:\n            path, fd \u003d m.group(1), int(m.group(2))\n            open_fds[fd] \u003d path\n            continue\n        m \u003d CLOSE_RE.search(line)\n        if m:\n            open_fds.pop(int(m.group(1)), None)\n\n    for fd, path in sorted(open_fds.items()):\n        print(\u0027  fd %d: %s\u0027 % (fd, path))\n```\n\nUpdated/new tests should also fail on master with only the change in `swift/common/db.py` applied:\n\n```\ngit checkout 2cf58ee851a8f983b83f3ff403bd7b8d0f7ca9c5 -- \\\nswift/common/db.py \\\ntest/unit/account/test_reaper.py \\\ntest/unit/account/test_auditor.py \\\ntest/unit/common/test_db_replicator.py \\\ntest/unit/container/test_replicator.py \\\ntest/unit/container/test_sharder.py \\\ntest/unit/container/test_sync.py \\\ntest/unit/container/test_updater.py\n\npytest \"test/unit/account/test_reaper.py::TestReaper::test_reap_device_closes_db_connection\" \\\n\"test/unit/account/test_auditor.py::TestAuditorRealBroker::test_audit_closes_db_connection\" \\\n\"test/unit/common/test_db_replicator.py::TestReplicatorSync::test_replicate_object_closes_db_connections\" \\\n\"test/unit/container/test_replicator.py::TestReplicatorSync::test_replicate_reconcilers_closes_db_connections\"  \\\n\"test/unit/container/test_sharder.py::TestSharder::test_get_shard_broker_closes_broker_on_error\" \\\n\"test/unit/container/test_sharder.py::TestSharder::test_one_shard_cycle_closes_db_connections\" \\ \n\"test/unit/container/test_sync.py::TestContainerSync::test_container_sync_closes_db_connection\" \\\n\"test/unit/container/test_updater.py::TestContainerUpdater::test_process_container_closes_db_connection\"\n```","commit_id":"b00de7faead868993730b67ca2a21619b6a82c8c"}],"swift/container/sharder.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4036fadb9bfdb643281f33ad8ae256fd6f8f13","unresolved":true,"context_lines":[{"line_number":1274,"context_line":"                (\u0027True\u0027, Timestamp.now().internal)})"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"        put_timestamp \u003d put_timestamp if initialized else None"},{"line_number":1277,"context_line":"        return part, shard_broker, node[\u0027id\u0027], put_timestamp"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"    def _audit_root_container(self, broker):"},{"line_number":1280,"context_line":"        # This is the root container, and therefore the tome of knowledge,"}],"source_content_type":"text/x-python","patch_set":1,"id":"6f0004b9_eb076575","line":1277,"updated":"2026-08-11 20:54:34.000000000","message":"I\u0027m a little concerned that we could pop a timeout or something before getting to this return, in which case this never makes it to `dest_brokers` in `_move_objects`, and won\u0027t get closed out properly.","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"374befbe8f6f5bcae1dcf6bcbfcf88ed9d540ea6","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"                (\u0027True\u0027, Timestamp.now().internal)})"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"        put_timestamp \u003d put_timestamp if initialized else None"},{"line_number":1277,"context_line":"        return part, shard_broker, node[\u0027id\u0027], put_timestamp"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"    def _audit_root_container(self, broker):"},{"line_number":1280,"context_line":"        # This is the root container, and therefore the tome of knowledge,"}],"source_content_type":"text/x-python","patch_set":1,"id":"8841295d_80f3e9a8","line":1277,"in_reply_to":"6f0004b9_eb076575","updated":"2026-08-12 13:56:10.000000000","message":"Good catch! Fixed in PS2 by wrapping the body in try/except that closes the broker and re-raises plus a  test.","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4036fadb9bfdb643281f33ad8ae256fd6f8f13","unresolved":true,"context_lines":[{"line_number":2012,"context_line":"        start \u003d time.time()"},{"line_number":2013,"context_line":"        # only cleave from the retiring db - misplaced objects handler will"},{"line_number":2014,"context_line":"        # deal with any objects in the fresh db"},{"line_number":2015,"context_line":"        with broker.get_brokers()[0] as source_broker:"},{"line_number":2016,"context_line":"            # if this range has been cleaved before but replication"},{"line_number":2017,"context_line":"            # failed then the shard db may still exist and it may not be"},{"line_number":2018,"context_line":"            # necessary to merge all the rows again"}],"source_content_type":"text/x-python","patch_set":1,"id":"a640dd96_6b3a24ce","line":2015,"updated":"2026-08-11 20:54:34.000000000","message":"Other places you use an `ExitStack` to wrap up *all* the brokers from `get_brokers()`, even when we\u0027re only interested in one of them -- why are we just doing a `with` for this one broker?","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"374befbe8f6f5bcae1dcf6bcbfcf88ed9d540ea6","unresolved":false,"context_lines":[{"line_number":2012,"context_line":"        start \u003d time.time()"},{"line_number":2013,"context_line":"        # only cleave from the retiring db - misplaced objects handler will"},{"line_number":2014,"context_line":"        # deal with any objects in the fresh db"},{"line_number":2015,"context_line":"        with broker.get_brokers()[0] as source_broker:"},{"line_number":2016,"context_line":"            # if this range has been cleaved before but replication"},{"line_number":2017,"context_line":"            # failed then the shard db may still exist and it may not be"},{"line_number":2018,"context_line":"            # necessary to merge all the rows again"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c1d61dd_400d3a03","line":2015,"in_reply_to":"a640dd96_6b3a24ce","updated":"2026-08-12 13:56:10.000000000","message":"See above comment","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4036fadb9bfdb643281f33ad8ae256fd6f8f13","unresolved":true,"context_lines":[{"line_number":2170,"context_line":"                broker,"},{"line_number":2171,"context_line":"                \u0027Moving any misplaced objects from sharding container\u0027)"},{"line_number":2172,"context_line":"            bounds \u003d self._make_default_misplaced_object_bounds(broker)"},{"line_number":2173,"context_line":"            with broker.get_brokers()[0] as src_broker:"},{"line_number":2174,"context_line":"                cleaving_context.misplaced_done \u003d \\"},{"line_number":2175,"context_line":"                    self._move_misplaced_objects("},{"line_number":2176,"context_line":"                        broker, src_broker\u003dsrc_broker, src_bounds\u003dbounds)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a81680ca_b170d8f0","line":2173,"updated":"2026-08-11 20:54:34.000000000","message":"Again, no `ExitStack`?","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"374befbe8f6f5bcae1dcf6bcbfcf88ed9d540ea6","unresolved":false,"context_lines":[{"line_number":2170,"context_line":"                broker,"},{"line_number":2171,"context_line":"                \u0027Moving any misplaced objects from sharding container\u0027)"},{"line_number":2172,"context_line":"            bounds \u003d self._make_default_misplaced_object_bounds(broker)"},{"line_number":2173,"context_line":"            with broker.get_brokers()[0] as src_broker:"},{"line_number":2174,"context_line":"                cleaving_context.misplaced_done \u003d \\"},{"line_number":2175,"context_line":"                    self._move_misplaced_objects("},{"line_number":2176,"context_line":"                        broker, src_broker\u003dsrc_broker, src_bounds\u003dbounds)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f6dae3e_b38f2a85","line":2173,"in_reply_to":"a81680ca_b170d8f0","updated":"2026-08-12 13:56:10.000000000","message":"Changed in PS2","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4036fadb9bfdb643281f33ad8ae256fd6f8f13","unresolved":true,"context_lines":[{"line_number":2611,"context_line":"                    self.db_logger.exception("},{"line_number":2612,"context_line":"                        broker, \u0027Unhandled exception while dumping \u0027"},{"line_number":2613,"context_line":"                        \u0027progress: %s\u0027, error)"},{"line_number":2614,"context_line":"                self._periodic_report_stats()"},{"line_number":2615,"context_line":""},{"line_number":2616,"context_line":"        self._report_stats()"},{"line_number":2617,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"afa1b0a7_b944d43e","line":2614,"updated":"2026-08-11 20:54:34.000000000","message":"nit: This line could probably come back out of the `with` block.","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"374befbe8f6f5bcae1dcf6bcbfcf88ed9d540ea6","unresolved":false,"context_lines":[{"line_number":2611,"context_line":"                    self.db_logger.exception("},{"line_number":2612,"context_line":"                        broker, \u0027Unhandled exception while dumping \u0027"},{"line_number":2613,"context_line":"                        \u0027progress: %s\u0027, error)"},{"line_number":2614,"context_line":"                self._periodic_report_stats()"},{"line_number":2615,"context_line":""},{"line_number":2616,"context_line":"        self._report_stats()"},{"line_number":2617,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"16500c72_ee694ef6","line":2614,"in_reply_to":"afa1b0a7_b944d43e","updated":"2026-08-12 13:56:10.000000000","message":"Done","commit_id":"17dbe7ac609d08578f93bc17c3509192dbcbcd62"}]}