)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"453530db_320235b4","updated":"2026-03-13 13:30:28.000000000","message":"couple of things inline","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"89ff69dfb1a078a462e7397e61bd52e19528eeee","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6c51597e_02875223","updated":"2026-03-25 19:47:15.000000000","message":"OK we have a lot of SQL errors similar to what we see in some of the unit test (excluded today in threading job). @smooney@redhat.com saw similar in cyborg and created a fix for it. It would make sense to try to adapt that fix to nova and see if it helps us or not. https://review.opendev.org/c/openstack/cyborg/+/970152/8/cyborg/tests/unit/db_lock_fixture.py\n\nWe discussed this in the eventlet sync and Sean offered help doing it. So @ashigupt@redhat.com please coordinate with Sean about who brings this fix over.","commit_id":"b74469be62ec6fd1825a9c30ef8cebad14ab864d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d121345d_dae41fd4","in_reply_to":"6c51597e_02875223","updated":"2026-03-31 14:44:00.000000000","message":"@ashigupt@redhat.com Have you looked at Sean\u0027s patch? It has some overlapping idea with yours but feels cleaner especially as it does not try apply long timeouts.","commit_id":"b74469be62ec6fd1825a9c30ef8cebad14ab864d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c25d580e8d0b4a6aa3c3d20cc54290ade599c90c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"74f2340d_b0ba41d7","updated":"2026-04-10 14:50:32.000000000","message":"I spent two days trying to understand what is happening with the in memory / file based sqlite switch. I mostly failed. :) \n\nYou can see most of my discussion in @smooney@redhat.com\u0027s patch https://review.opendev.org/c/openstack/nova/+/983069/comment/b395d3fb_0d3b47d6/ and in the minimum necessary change patch https://review.opendev.org/c/openstack/nova/+/983995\n\n\nBut one thing I\u0027m getting certain about is that we need to switch to file based SQLite DB as\n* named in memory SQLite is not possible with SQLAchemy\u0027s URL schema handling\n* something is magic in a way how SQLite / SQLAchemy / oslo.db is handling parallel DB access depending on if there is any file based DB in the process or only memory based DBs. If there is at least one file based one then the memory based DB also start behaving in an more serialized manner. We agreed with Sean over a call on Friday that we don\u0027t fully understand it.\n\nI also talked with Sean about the way forward between Ashish\u0027s functional patch here and Sean\u0027s unit test patch in https://review.opendev.org/c/openstack/nova/+/983069\n\nBoth touching the DB fixtures in our codebase used by both unit and functional test cases. So a synced approach is needed. I think the first step in that is having a patch that translates both the Database and the CellDatabases fixture to use file based SQLite DBs. This will make some of the unit test pass that is excluded today and it can be the basis of continue working on the functional tests.\n\nSean is busy and asked us to take over the unit test patch. @ashigupt@redhat.com would you be able to take the ideas from the patch or the patch itself and add it to your series? (if you take just the idea then please add attribution to the commit message)\n\nI also suggest to at least think about / try the minimal change from https://review.opendev.org/c/openstack/nova/+/983995 to see how far we can go with that only and when do we need to do the writer transaction serialization via the extra lock from Sean\u0027s patch.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"41f8af35_d8e778f5","updated":"2026-03-31 14:44:00.000000000","message":"This is getting complicated very quickly. I don\u0027t have yet the full context on how our functional test CellDB handling works. @dms@danplanet.com and @melwittt@gmail.com worked more on that code path so they might have opinion about the problem / solution quicker than me.\n\nOne suggestion is to reduce scope. Neuter the test_case_id fixtures for now they are not essential to make the functional test pass as they are only there to detect interference between test cases. Just disable them for native threading with a TODO that we want to rebuild them later. Let\u0027s focus on the SQLite complication and the CellDB difference first. Sean\u0027s changes from cyborgs seems to be simpler and more generic for the SQLite issue, but maybe our CellDB handling makes it non applicable. \n\nAnother suggestion. If everything is failing with multiple different issues then it might be easier to focus on a smaller test case, one that using less service. For example https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_2077980.py feels simple enough.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6cdc04c028b47044ec1efb2b824797befa9ca41d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3ee968ab_8358b94a","updated":"2026-03-31 12:09:40.000000000","message":"This patch now does too many thing at once. At least the SQLite related changes needs to be in a separate commit to help understanding it","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a078f53df1de5969f11a339dba10455aeca2d147","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"01b1ad82_77875ffe","in_reply_to":"3c815d54_403e2773","updated":"2026-04-15 14:56:50.000000000","message":"@melwittt@gmail.com also proposed two patches:\n* https://review.opendev.org/c/openstack/nova/+/984481 Is a variant of Sean\u0027s and your file base SQLite patch\n* https://review.opendev.org/c/openstack/nova/+/984480 It is the more interesting one it provides an alternative how to fix the DB routing in the CellDatabases fixture. I have some question inline there to understand more but it is promising.\n\nMy suggestion how to move forward:\n* Take a stepwise approach.\n* We know that file based SQLite is probably the way forward. Make a patch that adds only that but to both DB code paths (CellDatabases and DatabaseFixture). \n* Add WAL as a next step / commit\n* After we understood how to fix the cell routing in CellDatabases add that as the next commit\nSo how far this brings us in terms of unit and functional tests passing.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"431be53a09793e9e2d1603873e96729c50826ee1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3c815d54_403e2773","in_reply_to":"74f2340d_b0ba41d7","updated":"2026-04-10 15:15:35.000000000","message":"Sure @gibizer@gmail.com, Let me check out both the patches","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"}],"nova/test.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":186,"context_line":"            super(TestCase, self).setUp()"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        # Select the appropriate fixture based on concurrency mode"},{"line_number":189,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":190,"context_line":"            self.useFixture("},{"line_number":191,"context_line":"                nova_fixtures.PropagateTestCaseIdToChildThreads(self.id()))"},{"line_number":192,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"2bbf0f27_2c685437","line":189,"updated":"2026-03-13 13:30:28.000000000","message":"If we move this conditional into the fixture class (see my comments there) then we can spare two child classes and handle the whole divergence with a single, hidden conditional.","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            super(TestCase, self).setUp()"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        # Select the appropriate fixture based on concurrency mode"},{"line_number":189,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":190,"context_line":"            self.useFixture("},{"line_number":191,"context_line":"                nova_fixtures.PropagateTestCaseIdToChildThreads(self.id()))"},{"line_number":192,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"af55e34c_f761f0a9","line":189,"in_reply_to":"2bbf0f27_2c685437","updated":"2026-03-27 12:52:56.000000000","message":"Done","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"}],"nova/tests/fixtures/conf.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":36,"context_line":"# Many services (API, conductor, scheduler, N computes) share main.db;"},{"line_number":37,"context_line":"# without a generous busy handler, ``database is locked`` still surfaces under"},{"line_number":38,"context_line":"# burst RPC + heartbeats even with WAL."},{"line_number":39,"context_line":"_SQLITE_THREADING_BUSY_TIMEOUT_SEC \u003d \u0027600\u0027"},{"line_number":40,"context_line":"_SQLITE_THREADING_BUSY_TIMEOUT_MS \u003d 600000"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"_sqlite_threading_wal_listener_installed \u003d False"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9e343e65_680a0ac8","line":40,"range":{"start_line":39,"start_character":0,"end_line":40,"end_character":42},"updated":"2026-03-31 14:44:00.000000000","message":"What are the defaults of these? Do we know what makes a write longer in our functional test? It is very strange that a DB write operation takes 600 seconds. We should not try to tolerate that but track down why a write takes such a long time.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":36,"context_line":"# Many services (API, conductor, scheduler, N computes) share main.db;"},{"line_number":37,"context_line":"# without a generous busy handler, ``database is locked`` still surfaces under"},{"line_number":38,"context_line":"# burst RPC + heartbeats even with WAL."},{"line_number":39,"context_line":"_SQLITE_THREADING_BUSY_TIMEOUT_SEC \u003d \u0027600\u0027"},{"line_number":40,"context_line":"_SQLITE_THREADING_BUSY_TIMEOUT_MS \u003d 600000"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"_sqlite_threading_wal_listener_installed \u003d False"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"6b9595a6_c11a1747","line":40,"range":{"start_line":39,"start_character":0,"end_line":40,"end_character":42},"in_reply_to":"9e343e65_680a0ac8","updated":"2026-04-03 14:10:55.000000000","message":"Agree - maybe these were just chosen empirically to handle very high levels of concurrency? The difference between in-memory and on-disk transaction locking is probably pretty significant and this shift (which I also would like to avoid) is maybe tripping over some significant lock contention?","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    \"\"\"True when eventlet patching is disabled.\"\"\""},{"line_number":47,"context_line":"    return ("},{"line_number":48,"context_line":"        os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027).lower()"},{"line_number":49,"context_line":"        in _NATIVE_THREADING_ENV_VALUES)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"def _install_sqlite_threading_wal_listener():"}],"source_content_type":"text/x-python","patch_set":8,"id":"7808d57b_265b274e","line":49,"updated":"2026-03-31 14:44:00.000000000","message":"Can we use utils.concurrency_mode_threading() instead?","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":53,"context_line":"    \"\"\"Tune SQLite once per process for native threading."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    File-backed SQLite with NullPool and many OS threads otherwise raises"},{"line_number":56,"context_line":"    ``database is locked``. Enabling WAL, ``timeout\u003d`` on the URL and"},{"line_number":57,"context_line":"    ``PRAGMA busy_timeout`` together reduce lock contention in"},{"line_number":58,"context_line":"    multi-threaded SQLite usage."},{"line_number":59,"context_line":"    \"\"\""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    global _sqlite_threading_wal_listener_installed"}],"source_content_type":"text/x-python","patch_set":8,"id":"048e2db5_75a56783","line":58,"range":{"start_line":56,"start_character":28,"end_line":58,"end_character":32},"updated":"2026-03-31 14:44:00.000000000","message":"How does this reduce lock contention? As far as I understand the increased timeout values does not reduce the contention but actually instruct the DB engine to tolerate contention longer.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":70,"context_line":"            return"},{"line_number":71,"context_line":"        cursor \u003d dbapi_connection.cursor()"},{"line_number":72,"context_line":"        try:"},{"line_number":73,"context_line":"            cursor.execute(\u0027PRAGMA journal_mode\u003dWAL\u0027)"},{"line_number":74,"context_line":"            cursor.execute("},{"line_number":75,"context_line":"                \u0027PRAGMA busy_timeout\u003d%d\u0027"},{"line_number":76,"context_line":"                % _SQLITE_THREADING_BUSY_TIMEOUT_MS)"}],"source_content_type":"text/x-python","patch_set":8,"id":"270dad7b_8d59c6fe","line":73,"updated":"2026-03-31 14:44:00.000000000","message":"I\u0027m not convince yet we want all this. But I think if we want this it is enough to set it once per DB, we does not need to set it once per connection as per \"Persistence of WAL mode\" https://sqlite.org/wal.html","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":70,"context_line":"            return"},{"line_number":71,"context_line":"        cursor \u003d dbapi_connection.cursor()"},{"line_number":72,"context_line":"        try:"},{"line_number":73,"context_line":"            cursor.execute(\u0027PRAGMA journal_mode\u003dWAL\u0027)"},{"line_number":74,"context_line":"            cursor.execute("},{"line_number":75,"context_line":"                \u0027PRAGMA busy_timeout\u003d%d\u0027"},{"line_number":76,"context_line":"                % _SQLITE_THREADING_BUSY_TIMEOUT_MS)"}],"source_content_type":"text/x-python","patch_set":8,"id":"78229abc_237c2fb9","line":73,"in_reply_to":"270dad7b_8d59c6fe","updated":"2026-04-03 14:10:55.000000000","message":"Also, a lot more docs/comments in here about what we\u0027re doing here would be good (if we stick with the disk approach).","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        self.conf.set_default(\u0027periodic_enable\u0027, False)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        # api_database / database groups"},{"line_number":112,"context_line":"        # NOTE: With native threading, default sqlite:// creates an"},{"line_number":113,"context_line":"        # in-memory DB per connection. RPC threads do not see"},{"line_number":114,"context_line":"        # schema/data. File-backed sqlite gives one shared"},{"line_number":115,"context_line":"        # api/main DB per process."},{"line_number":116,"context_line":"        if _running_native_threading_tests():"}],"source_content_type":"text/x-python","patch_set":8,"id":"bc99b8e5_e5a38f12","line":113,"range":{"start_line":112,"start_character":16,"end_line":113,"end_character":38},"updated":"2026-03-31 14:44:00.000000000","message":"There are multiple ways to ask for an in memory SQLite db some can be shared across threads within the same process as per \"In-memory Databases And Shared Cache\" https://www.sqlite.org/inmemorydb.html\n\nTrue that the default will not be shared across connections. But we don\u0027t necessary need a file based db on disk to be able to share it.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        self.conf.set_default(\u0027periodic_enable\u0027, False)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        # api_database / database groups"},{"line_number":112,"context_line":"        # NOTE: With native threading, default sqlite:// creates an"},{"line_number":113,"context_line":"        # in-memory DB per connection. RPC threads do not see"},{"line_number":114,"context_line":"        # schema/data. File-backed sqlite gives one shared"},{"line_number":115,"context_line":"        # api/main DB per process."},{"line_number":116,"context_line":"        if _running_native_threading_tests():"}],"source_content_type":"text/x-python","patch_set":8,"id":"fd9770dd_a0eb30d1","line":113,"range":{"start_line":112,"start_character":16,"end_line":113,"end_character":38},"in_reply_to":"bc99b8e5_e5a38f12","updated":"2026-04-03 14:10:55.000000000","message":"Agree, and `sqlite://$test-case-id` or similar may also help weed out cases where we\u0027ve lost track of which test case a thread is supposed to be attached to.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        # api_database / database groups"},{"line_number":112,"context_line":"        # NOTE: With native threading, default sqlite:// creates an"},{"line_number":113,"context_line":"        # in-memory DB per connection. RPC threads do not see"},{"line_number":114,"context_line":"        # schema/data. File-backed sqlite gives one shared"},{"line_number":115,"context_line":"        # api/main DB per process."},{"line_number":116,"context_line":"        if _running_native_threading_tests():"},{"line_number":117,"context_line":"            _install_sqlite_threading_wal_listener()"}],"source_content_type":"text/x-python","patch_set":8,"id":"45cb6e0b_00f64566","line":114,"range":{"start_line":113,"start_character":38,"end_line":114,"end_character":22},"updated":"2026-03-31 14:44:00.000000000","message":"I don\u0027t understand this statement. Could you ellaborate?","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":131,"context_line":"            self.conf.set_default(\u0027connection\u0027, \u0027sqlite://\u0027, group\u003d\u0027database\u0027)"},{"line_number":132,"context_line":"        self.conf.set_default(\u0027sqlite_synchronous\u0027, False,"},{"line_number":133,"context_line":"                              group\u003d\u0027api_database\u0027)"},{"line_number":134,"context_line":"        self.conf.set_default(\u0027sqlite_synchronous\u0027, False, group\u003d\u0027database\u0027)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        # key_manager group"},{"line_number":137,"context_line":"        self.conf.set_default(\u0027backend\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"624b48b8_fdaf218f","line":134,"updated":"2026-03-31 14:44:00.000000000","message":"I\u0027m wondering if this is something related to our problem?\n\n(config optinon defined in oslo.db https://github.com/openstack/oslo.db/blob/b16fbbd8a9f9c58d702ab73c2b717c259c1343d3/oslo_db/options.py#L18 )","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"}],"nova/tests/fixtures/notifications.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":155,"context_line":"        # NOTE(gibi) not all eventlet spawn is under our control, so there can"},{"line_number":156,"context_line":"        # be senders without test_case_id set, find the first ancestor that"},{"line_number":157,"context_line":"        # was spawned from nova.utils.spawn[_n] and therefore has the id set."},{"line_number":158,"context_line":"        while not getattr(current, \u0027test_case_id\u0027, None):"},{"line_number":159,"context_line":"            current \u003d current.parent"},{"line_number":160,"context_line":"        return current.test_case_id"},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"a9449969_bf16ef67","side":"PARENT","line":158,"updated":"2026-03-13 13:30:28.000000000","message":"could this logic work with threading as well?","commit_id":"039ddb9d987f12a7699484fd82b1450b9d5b69eb"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":true,"context_lines":[{"line_number":155,"context_line":"        # NOTE(gibi) not all eventlet spawn is under our control, so there can"},{"line_number":156,"context_line":"        # be senders without test_case_id set, find the first ancestor that"},{"line_number":157,"context_line":"        # was spawned from nova.utils.spawn[_n] and therefore has the id set."},{"line_number":158,"context_line":"        while not getattr(current, \u0027test_case_id\u0027, None):"},{"line_number":159,"context_line":"            current \u003d current.parent"},{"line_number":160,"context_line":"        return current.test_case_id"},{"line_number":161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"4d728357_2bcab2ea","side":"PARENT","line":158,"in_reply_to":"a9449969_bf16ef67","updated":"2026-03-27 12:52:56.000000000","message":"I don\u0027t think so as threads don’t have that chain, so we  cannot reuse the same traversal for threading","commit_id":"039ddb9d987f12a7699484fd82b1450b9d5b69eb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @staticmethod"},{"line_number":153,"context_line":"    def _get_sender_test_case_id():"},{"line_number":154,"context_line":"        from nova import utils"},{"line_number":155,"context_line":"        # In threading mode, eventlet.getcurrent() doesn\u0027t work the same way"},{"line_number":156,"context_line":"        # and we can\u0027t traverse a parent chain. Use thread-local storage"},{"line_number":157,"context_line":"        # if available, otherwise fall back to eventlet greenthread approach."}],"source_content_type":"text/x-python","patch_set":5,"id":"660a120c_9e7ae5ee","line":154,"updated":"2026-03-13 13:30:28.000000000","message":"can we move this to the top of the file, or is it circular dep?","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @staticmethod"},{"line_number":153,"context_line":"    def _get_sender_test_case_id():"},{"line_number":154,"context_line":"        from nova import utils"},{"line_number":155,"context_line":"        # In threading mode, eventlet.getcurrent() doesn\u0027t work the same way"},{"line_number":156,"context_line":"        # and we can\u0027t traverse a parent chain. Use thread-local storage"},{"line_number":157,"context_line":"        # if available, otherwise fall back to eventlet greenthread approach."}],"source_content_type":"text/x-python","patch_set":5,"id":"069b50f2_d269fbd1","line":154,"in_reply_to":"660a120c_9e7ae5ee","updated":"2026-03-27 12:52:56.000000000","message":"Done","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":153,"context_line":"    def _get_sender_test_case_id():"},{"line_number":154,"context_line":"        from nova import utils"},{"line_number":155,"context_line":"        # In threading mode, eventlet.getcurrent() doesn\u0027t work the same way"},{"line_number":156,"context_line":"        # and we can\u0027t traverse a parent chain. Use thread-local storage"},{"line_number":157,"context_line":"        # if available, otherwise fall back to eventlet greenthread approach."},{"line_number":158,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":159,"context_line":"            # In threading mode, we use thread-local storage set by"}],"source_content_type":"text/x-python","patch_set":5,"id":"b9077169_2a45f88b","line":156,"range":{"start_line":156,"start_character":13,"end_line":156,"end_character":47},"updated":"2026-03-13 13:30:28.000000000","message":"why?","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":153,"context_line":"    def _get_sender_test_case_id():"},{"line_number":154,"context_line":"        from nova import utils"},{"line_number":155,"context_line":"        # In threading mode, eventlet.getcurrent() doesn\u0027t work the same way"},{"line_number":156,"context_line":"        # and we can\u0027t traverse a parent chain. Use thread-local storage"},{"line_number":157,"context_line":"        # if available, otherwise fall back to eventlet greenthread approach."},{"line_number":158,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":159,"context_line":"            # In threading mode, we use thread-local storage set by"}],"source_content_type":"text/x-python","patch_set":5,"id":"129a3f78_8f91fa31","line":156,"range":{"start_line":156,"start_character":13,"end_line":156,"end_character":47},"in_reply_to":"b7f02138_b2b25eb4","updated":"2026-04-03 14:10:55.000000000","message":"But if we\u0027re in control of all the spawn calls (due to the mock) it seems like we can provide that same linkage. You\u0027re already setting the `test_case_id` during spawn in the next file right?","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":true,"context_lines":[{"line_number":153,"context_line":"    def _get_sender_test_case_id():"},{"line_number":154,"context_line":"        from nova import utils"},{"line_number":155,"context_line":"        # In threading mode, eventlet.getcurrent() doesn\u0027t work the same way"},{"line_number":156,"context_line":"        # and we can\u0027t traverse a parent chain. Use thread-local storage"},{"line_number":157,"context_line":"        # if available, otherwise fall back to eventlet greenthread approach."},{"line_number":158,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":159,"context_line":"            # In threading mode, we use thread-local storage set by"}],"source_content_type":"text/x-python","patch_set":5,"id":"b7f02138_b2b25eb4","line":156,"range":{"start_line":156,"start_character":13,"end_line":156,"end_character":47},"in_reply_to":"b9077169_2a45f88b","updated":"2026-03-27 12:52:56.000000000","message":"As native threads do not keep a reference to their parent thread, we cannot walk a parent chain like in eventlet greenthreads. \n\nLet me know if the interpretation looks incorrect, I can try to reword it","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"850e4ce487c62f597dff557d3d9bda8dbc8219ab","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":159,"context_line":"            return getattr(threading.current_thread(), \u0027test_case_id\u0027, None)"},{"line_number":160,"context_line":"        else:"},{"line_number":161,"context_line":"            # Eventlet mode: use the existing parent chain traversal"},{"line_number":162,"context_line":"            current \u003d eventlet.getcurrent()"},{"line_number":163,"context_line":"            if current is None:"},{"line_number":164,"context_line":"                return None"},{"line_number":165,"context_line":"            # NOTE(gibi) not all eventlet spawn is under our control, so there"},{"line_number":166,"context_line":"            # can be senders without test_case_id set, find the first ancestor"},{"line_number":167,"context_line":"            # that was spawned from nova.utils.spawn[_n] and therefore has the"},{"line_number":168,"context_line":"            # id set."},{"line_number":169,"context_line":"            while current is not None and not getattr("},{"line_number":170,"context_line":"                    current, \u0027test_case_id\u0027, None):"},{"line_number":171,"context_line":"                if not hasattr(current, \u0027parent\u0027):"},{"line_number":172,"context_line":"                    return None"},{"line_number":173,"context_line":"                current \u003d current.parent"},{"line_number":174,"context_line":"            if current is None:"},{"line_number":175,"context_line":"                return None"},{"line_number":176,"context_line":"            return current.test_case_id"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def _notify(self, priority, ctxt, event_type, payload):"},{"line_number":179,"context_line":"        sender_test_case_id \u003d self._get_sender_test_case_id()"}],"source_content_type":"text/x-python","patch_set":8,"id":"2efac672_bea64338","line":176,"range":{"start_line":161,"start_character":0,"end_line":176,"end_character":39},"updated":"2026-03-31 14:44:00.000000000","message":"As far as I understand this is the eventlet case, but it is changed compared to the baseline. Even if the native threading case needs to be changed as it works differently I don\u0027t see right now why the eventlet case needs to be changed.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        if utils.concurrency_mode_threading():"},{"line_number":159,"context_line":"            return getattr(threading.current_thread(), \u0027test_case_id\u0027, None)"},{"line_number":160,"context_line":"        else:"},{"line_number":161,"context_line":"            # Eventlet mode: use the existing parent chain traversal"},{"line_number":162,"context_line":"            current \u003d eventlet.getcurrent()"},{"line_number":163,"context_line":"            if current is None:"},{"line_number":164,"context_line":"                return None"},{"line_number":165,"context_line":"            # NOTE(gibi) not all eventlet spawn is under our control, so there"},{"line_number":166,"context_line":"            # can be senders without test_case_id set, find the first ancestor"},{"line_number":167,"context_line":"            # that was spawned from nova.utils.spawn[_n] and therefore has the"},{"line_number":168,"context_line":"            # id set."},{"line_number":169,"context_line":"            while current is not None and not getattr("},{"line_number":170,"context_line":"                    current, \u0027test_case_id\u0027, None):"},{"line_number":171,"context_line":"                if not hasattr(current, \u0027parent\u0027):"},{"line_number":172,"context_line":"                    return None"},{"line_number":173,"context_line":"                current \u003d current.parent"},{"line_number":174,"context_line":"            if current is None:"},{"line_number":175,"context_line":"                return None"},{"line_number":176,"context_line":"            return current.test_case_id"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def _notify(self, priority, ctxt, event_type, payload):"},{"line_number":179,"context_line":"        sender_test_case_id \u003d self._get_sender_test_case_id()"}],"source_content_type":"text/x-python","patch_set":8,"id":"10a210b6_72e39e42","line":176,"range":{"start_line":161,"start_character":0,"end_line":176,"end_character":39},"in_reply_to":"2efac672_bea64338","updated":"2026-04-03 14:10:55.000000000","message":"I\u0027m also really unsure why we need all the new cases to be careful about missing parent and things, and just returning None everywhere. To me that indicates that we\u0027re not getting everything we think we are and that this is just here to make obscure cases pass.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":1927,"context_line":"            return orig_spawn_on("},{"line_number":1928,"context_line":"                executor, _wrap_with_test_case_id(func), *args, **kwargs)"},{"line_number":1929,"context_line":""},{"line_number":1930,"context_line":"        def wrapped_spawn(func, *args, **kwargs):"},{"line_number":1931,"context_line":"            # Wrap spawn to propagate test_case_id. Since spawn() internally"},{"line_number":1932,"context_line":"            # calls spawn_on(), we call orig_spawn_on directly to avoid"},{"line_number":1933,"context_line":"            # double-wrapping (the function is already wrapped here, and"}],"source_content_type":"text/x-python","patch_set":5,"id":"15fb1e6b_8247a674","line":1930,"updated":"2026-03-13 13:30:28.000000000","message":"we don\u0027t need this as utils.spawn is implemented in terms of utils.spawn_on, and that is already taking care of the id propagation above.","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":false,"context_lines":[{"line_number":1927,"context_line":"            return orig_spawn_on("},{"line_number":1928,"context_line":"                executor, _wrap_with_test_case_id(func), *args, **kwargs)"},{"line_number":1929,"context_line":""},{"line_number":1930,"context_line":"        def wrapped_spawn(func, *args, **kwargs):"},{"line_number":1931,"context_line":"            # Wrap spawn to propagate test_case_id. Since spawn() internally"},{"line_number":1932,"context_line":"            # calls spawn_on(), we call orig_spawn_on directly to avoid"},{"line_number":1933,"context_line":"            # double-wrapping (the function is already wrapped here, and"}],"source_content_type":"text/x-python","patch_set":5,"id":"fd39fec5_f095cad4","line":1930,"in_reply_to":"15fb1e6b_8247a674","updated":"2026-03-27 12:52:56.000000000","message":"Done","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d593a4271272302d36394bc7dfaf616755d4667","unresolved":true,"context_lines":[{"line_number":1983,"context_line":"        import threading"},{"line_number":1984,"context_line":"        return threading.current_thread()"},{"line_number":1985,"context_line":""},{"line_number":1986,"context_line":"    def _set_test_case_id(self, context, test_case_id):"},{"line_number":1987,"context_line":"        context.test_case_id \u003d test_case_id"},{"line_number":1988,"context_line":""},{"line_number":1989,"context_line":"    def _get_test_case_id(self, context):"},{"line_number":1990,"context_line":"        return getattr(context, \u0027test_case_id\u0027, None)"},{"line_number":1991,"context_line":""},{"line_number":1992,"context_line":""},{"line_number":1993,"context_line":"class ReaderWriterLock(lockutils.ReaderWriterLock):"}],"source_content_type":"text/x-python","patch_set":5,"id":"767f2f8c_aedb3fff","line":1990,"range":{"start_line":1986,"start_character":0,"end_line":1990,"end_character":53},"updated":"2026-03-13 13:30:28.000000000","message":"because these are the same in both children they can be moved back to the common parent.\n\nThis leaves us with the only divergence in the impl of _get_current_context(). I\u0027m fine having that implemented as two classes. Alternative it can be as simple as keeping the single class and having a conditional:\n```\ndef _get_current_context(self)\n    if utils.concurrency_mode_threading():\n        import threading\n        return threading.current_thread()\n    else:\n        import eventlet\n        return eventelt.getcurrent()\n```","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":35153,"name":"Ashish Gupta","email":"ashigupt@redhat.com","username":"ashigupt","status":"Redhat"},"change_message_id":"e2f578137553b144d7066b837d24dc26b8d6a0d7","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"        import threading"},{"line_number":1984,"context_line":"        return threading.current_thread()"},{"line_number":1985,"context_line":""},{"line_number":1986,"context_line":"    def _set_test_case_id(self, context, test_case_id):"},{"line_number":1987,"context_line":"        context.test_case_id \u003d test_case_id"},{"line_number":1988,"context_line":""},{"line_number":1989,"context_line":"    def _get_test_case_id(self, context):"},{"line_number":1990,"context_line":"        return getattr(context, \u0027test_case_id\u0027, None)"},{"line_number":1991,"context_line":""},{"line_number":1992,"context_line":""},{"line_number":1993,"context_line":"class ReaderWriterLock(lockutils.ReaderWriterLock):"}],"source_content_type":"text/x-python","patch_set":5,"id":"78804446_527d9667","line":1990,"range":{"start_line":1986,"start_character":0,"end_line":1990,"end_character":53},"in_reply_to":"767f2f8c_aedb3fff","updated":"2026-03-27 12:52:56.000000000","message":"Done","commit_id":"f354c67d9800fda418697fe2ae2e6b9e5047f9c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":1959,"context_line":""},{"line_number":1960,"context_line":"    def _set_test_case_id(self, context, test_case_id):"},{"line_number":1961,"context_line":"        \"\"\"Set test_case_id on the given context.\"\"\""},{"line_number":1962,"context_line":"        context.test_case_id \u003d test_case_id"},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"    def _get_test_case_id(self, context):"},{"line_number":1965,"context_line":"        \"\"\"Get test_case_id from the given context, or None if not set.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"0720f127_1622d5a1","line":1962,"updated":"2026-04-03 14:10:55.000000000","message":"This function executes a single line of code and makes that single line look changed below when it\u0027s really not. Can we just leave this \"inlined\"?","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":2000,"context_line":"        # Note: nova.utils.spawn_n doesn\u0027t exist, but if it\u0027s added in the"},{"line_number":2001,"context_line":"        # future, it should also be wrapped here."},{"line_number":2002,"context_line":"        self.useFixture("},{"line_number":2003,"context_line":"            fixtures.MonkeyPatch(\u0027nova.utils.spawn_on\u0027, wrapped_spawn_on))"},{"line_number":2004,"context_line":""},{"line_number":2005,"context_line":""},{"line_number":2006,"context_line":"class ReaderWriterLock(lockutils.ReaderWriterLock):"}],"source_content_type":"text/x-python","patch_set":8,"id":"2bf4b4de_dc5cfc42","line":2003,"updated":"2026-04-03 14:10:55.000000000","message":"This whole section is pretty hard to follow (it was already complex before). But you\u0027re removing some comments (or at least not replacing them with equivalents). Can you add more here to explain things? I\u0027m not sure why we need the double wrapper now, for example.\n\nAlso, a lot of the complexity here appears to be related to the case where \"we were spawned with other than nova.utils.spawn\" ... yet I don\u0027t see how that\u0027s possible since we\u0027re only entering this code via mock of `nova.utils.spawn` itself. Maybe gibi can provide some more context here. I\u0027ve looked at the original change from 2021 but I\u0027m really struggling to understand the nuance here.","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c84863581a09c4f1cbee48b7703b0472ba3d6536","unresolved":true,"context_lines":[{"line_number":2027,"context_line":"    See https://github.com/eventlet/eventlet/issues/731 for details."},{"line_number":2028,"context_line":""},{"line_number":2029,"context_line":"    [1] https://github.com/eventlet/eventlet/blob/v0.32.0/eventlet/green/"},{"line_number":2030,"context_line":"    threading.py#L128"},{"line_number":2031,"context_line":"    \"\"\""},{"line_number":2032,"context_line":""},{"line_number":2033,"context_line":"    def __init__(self, *a, **kw):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5125428b_481dc97c","line":2030,"updated":"2026-04-03 14:10:55.000000000","message":"Unrelated gardening?","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"}],"tools/generate-functional-exclude.sh":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6cdc04c028b47044ec1efb2b824797befa9ca41d","unresolved":true,"context_lines":[{"line_number":2,"context_line":"# Generate exclude list for functional threading tests"},{"line_number":3,"context_line":"# Removes comments from the exclude file for stestr"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"grep -v \"^#\" threading_functional_test_excludes.txt | grep -v \"^$\" \u003e /tmp/functional-exclude.txt"}],"source_content_type":"text/x-sh","patch_set":8,"id":"aa201e15_11fab1b1","line":5,"updated":"2026-03-31 12:09:40.000000000","message":"don\u0027t need to duplicate this, just make the original script take the exclude list as a parameter and pass such parameter from the tox.ini per target","commit_id":"d04a59d0fa74b384a5a1f10d603be4704b9a98c2"}]}
