)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0628f3555aaa692f2b1d56c05af18a45a8a64674","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"16989490_14d98ea9","updated":"2024-03-12 00:04:09.000000000","message":"recheck","commit_id":"b74709fdbb128d4890748793bc844937926828f1"}],"manila/db/sqlalchemy/api.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"35f1b6e2aad2e09c2d53679648f047643c772b64","unresolved":true,"context_lines":[{"line_number":1916,"context_line":"        if len(share.instances) \u003d\u003d 0:"},{"line_number":1917,"context_line":"            session.query(models.ShareAccessMapping).filter_by("},{"line_number":1918,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1919,"context_line":"            ).soft_delete()"},{"line_number":1920,"context_line":"            session.query(models.ShareMetadata).filter_by("},{"line_number":1921,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1922,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5e70ee3a_17c222a6","line":1919,"updated":"2023-11-02 12:08:06.000000000","message":"Note to reviewers: this is the same query/soft_delete that we are doing in `share_access_delete_all_by_share`. The only apparent difference is that we use the same session (albeit with a new transaction) instead of a new, separate session. However, a quick printf-debugging sessions suggests this query isn\u0027t returning anything so this should presumably have no impact.\n\nAlso note that the new transaction was necessary or I get other test failures. You may want to take that back out if those additional failures are helpful.","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f8846607bc483ad063e2994c88d4fdf4048a7d3c","unresolved":true,"context_lines":[{"line_number":1916,"context_line":"        if len(share.instances) \u003d\u003d 0:"},{"line_number":1917,"context_line":"            session.query(models.ShareAccessMapping).filter_by("},{"line_number":1918,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1919,"context_line":"            ).soft_delete()"},{"line_number":1920,"context_line":"            session.query(models.ShareMetadata).filter_by("},{"line_number":1921,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1922,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a957b0c_f980d4c0","line":1919,"in_reply_to":"02efef44_f652b01f","updated":"2023-11-06 13:24:17.000000000","message":"without seeing what the race condition is testing or anything, just looking at the code, the previous code calls get_session() which appears to make a totally separate session.  so....enginefacade has the \"independent\" modifier to get one of those like this:\n\n\n    with enginefacade.writer.independent.using(context) as s2:\n    \ni agree it would be nicer to use just one session but I\u0027d need to understand what the test is and what the nature of the error is to understand what might be racing.","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d5d940479ac0fe7d5cf933bd5aaeff6d22377e0a","unresolved":true,"context_lines":[{"line_number":1916,"context_line":"        if len(share.instances) \u003d\u003d 0:"},{"line_number":1917,"context_line":"            session.query(models.ShareAccessMapping).filter_by("},{"line_number":1918,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1919,"context_line":"            ).soft_delete()"},{"line_number":1920,"context_line":"            session.query(models.ShareMetadata).filter_by("},{"line_number":1921,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1922,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"02efef44_f652b01f","line":1919,"in_reply_to":"5e70ee3a_17c222a6","updated":"2023-11-02 12:11:37.000000000","message":"fwiw, I stuck print statements in here for the value of `share` and the result of the two queries here. Nothing changes between the share in either invocation and the queries are always empty.","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"a1357befbad46d94d84524b5322b9565b714bd1b","unresolved":false,"context_lines":[{"line_number":1916,"context_line":"        if len(share.instances) \u003d\u003d 0:"},{"line_number":1917,"context_line":"            session.query(models.ShareAccessMapping).filter_by("},{"line_number":1918,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1919,"context_line":"            ).soft_delete()"},{"line_number":1920,"context_line":"            session.query(models.ShareMetadata).filter_by("},{"line_number":1921,"context_line":"                share_id\u003dshare[\u0027id\u0027],"},{"line_number":1922,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"668b33eb_e1bbc34f","line":1919,"in_reply_to":"7a957b0c_f980d4c0","updated":"2024-03-22 15:13:09.000000000","message":"Done","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"89f7c1b215bd3b3429c787ae188bfb9a08ff33a0","unresolved":true,"context_lines":[{"line_number":1924,"context_line":""},{"line_number":1925,"context_line":"        if need_to_update_usages:"},{"line_number":1926,"context_line":"            _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1927,"context_line":"                                          is_replica\u003dis_replica)"},{"line_number":1928,"context_line":""},{"line_number":1929,"context_line":""},{"line_number":1930,"context_line":"def _set_instances_share_data(context, instances):"}],"source_content_type":"text/x-python","patch_set":1,"id":"842aeae5_cf394182","line":1927,"updated":"2023-11-02 18:08:43.000000000","message":"I\u0027m beginning to think this may be the ultimate issue. I wonder if we want to move this out of here and up to the manager/API layer? Certainly it seems to be the only place in the DB layer where we are manipulating quotas as part of an operation on another resource. If so, we probablyh need to deconstruct/grok the method a little so we\u0027re not copy-pasting irrelevant/dead code.","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"a1357befbad46d94d84524b5322b9565b714bd1b","unresolved":false,"context_lines":[{"line_number":1924,"context_line":""},{"line_number":1925,"context_line":"        if need_to_update_usages:"},{"line_number":1926,"context_line":"            _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1927,"context_line":"                                          is_replica\u003dis_replica)"},{"line_number":1928,"context_line":""},{"line_number":1929,"context_line":""},{"line_number":1930,"context_line":"def _set_instances_share_data(context, instances):"}],"source_content_type":"text/x-python","patch_set":1,"id":"40b2eb36_6091f430","line":1927,"in_reply_to":"842aeae5_cf394182","updated":"2024-03-22 15:13:09.000000000","message":"Done","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"35f1b6e2aad2e09c2d53679648f047643c772b64","unresolved":true,"context_lines":[{"line_number":3160,"context_line":"    with session.begin():"},{"line_number":3161,"context_line":"        session.query(models.ShareAccessMapping).filter_by("},{"line_number":3162,"context_line":"            share_id\u003dshare_id"},{"line_number":3163,"context_line":"        ).soft_delete()"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":""},{"line_number":3166,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":1,"id":"c3b0535b_a2b2e178","line":3163,"updated":"2023-11-02 12:08:06.000000000","message":"...here","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"a1357befbad46d94d84524b5322b9565b714bd1b","unresolved":false,"context_lines":[{"line_number":3160,"context_line":"    with session.begin():"},{"line_number":3161,"context_line":"        session.query(models.ShareAccessMapping).filter_by("},{"line_number":3162,"context_line":"            share_id\u003dshare_id"},{"line_number":3163,"context_line":"        ).soft_delete()"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":""},{"line_number":3166,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":1,"id":"374b4d03_6d1252b1","line":3163,"in_reply_to":"c3b0535b_a2b2e178","updated":"2024-03-22 15:13:09.000000000","message":"Done","commit_id":"09174ae6d6006fa466717911910c122b05483d48"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"3092461ed7e81fb63c2ad1e06ab11b7df7fa6d45","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"2d24b804_49803440","line":1938,"updated":"2024-03-22 12:20:11.000000000","message":"cc @stephenfin - so far it seems like this extra bit here is indeed what causes the issue.   is there some straightforward way we can as you mentioned refactor this part of the use case to be upstream of the whole thing here and keep tempest passing?","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"b422290046d9c450db853e747c1c78bc6b6ea4be","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"36e1f832_99b9d13f","line":1938,"in_reply_to":"2d24b804_49803440","updated":"2024-03-22 12:55:25.000000000","message":"Tracing down inside, QUOTA.reserve() etc. are also using the database.   The difference here is the shared session on the context, so I have added a distinct session for this block and I think that will have been the problem.","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"5645a027fd89a198dcb6bf5c3fab1f6b988321b5","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"952e8397_6ddbaca6","line":1938,"in_reply_to":"36e1f832_99b9d13f","updated":"2024-03-22 13:40:47.000000000","message":"OK this failed.  the context being passed in,with any kind of state, is what is different here.   it sort of has to be that","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"08287ca8d766f4bd25505daf78db22cdd8ee5e2e","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"0d61305d_570beb05","line":1938,"in_reply_to":"36e1f832_99b9d13f","updated":"2024-03-25 10:26:44.000000000","message":"Yes, I\u0027ve proposed that [here](https://review.opendev.org/c/openstack/manila/+/913962). I\u0027m waiting on CI results but it passes linters and unit tests locally so _hopefully_ it\u0027s mostly there.","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"ab97483e8222d8ebd11d2fa9f49a09c7606b4e97","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"bb2b0e97_c4f3a6a0","line":1938,"in_reply_to":"952e8397_6ddbaca6","updated":"2024-03-22 14:13:28.000000000","message":"yah it is ultimately calling this:\n\n    # NOTE(stephenfin): We intentionally don\u0027t wrap the outer function here since\n    # we call the innter function multiple times and want each call to be in a\n    # separate transaction\n    @require_context\n    def quota_reserve(context, resources, project_quotas, user_quotas,\n                    share_type_quotas, deltas, expire, until_refresh,\n                    max_age, project_id\u003dNone, user_id\u003dNone, share_type_id\u003dNone,\n                    overquota_allowed\u003dFalse):\n        user_reservations \u003d _quota_reserve(\n    \n    \nso we need to suspend the tranaction on the context and have it be able to make it\u0027s own trans on those contexts.   however, better is to have those quota fns also use \"independent\".    so we will just have a lot of \"independent\" notation here, we can try scaling them back once we get all green","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"a1357befbad46d94d84524b5322b9565b714bd1b","unresolved":false,"context_lines":[{"line_number":1935,"context_line":"        ).soft_delete()"},{"line_number":1936,"context_line":"        share.soft_delete(session\u003dsession)"},{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"    if need_to_update_usages:"},{"line_number":1939,"context_line":"        _update_share_instance_usages(context, share, instance_ref,"},{"line_number":1940,"context_line":"                                      is_replica\u003dis_replica,"},{"line_number":1941,"context_line":"                                      deferred_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ffaaa685_ac59ac84","line":1938,"in_reply_to":"bb2b0e97_c4f3a6a0","updated":"2024-03-22 15:13:09.000000000","message":"Done","commit_id":"c55fc346b28ea3f24cd4f73d37d9c067412e5b95"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"24fd5b39d32919711949f59b9a0ff9d530e204cd","unresolved":true,"context_lines":[{"line_number":1922,"context_line":""},{"line_number":1923,"context_line":"        # NOTE(zzzeek) currently this potentially is required for current"},{"line_number":1924,"context_line":"        # tempest runs to pass"},{"line_number":1925,"context_line":"        with context_manager.writer.independent.using(context) as oob_session:"},{"line_number":1926,"context_line":"            oob_session.query(models.ShareAccessMapping).filter_by("},{"line_number":1927,"context_line":"                share_id\u003dshare[\u0027id\u0027]"},{"line_number":1928,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":13,"id":"84d2c184_29a8a931","line":1925,"updated":"2024-03-22 15:14:10.000000000","message":"this is the one last part we might try taking out\n\nhowever it\u0027s fine to keep it as well","commit_id":"299b36b3f894c2cf5c48b230f9683cfa63361166"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"c6a4993166b2f1b2cdfb527e85f3ad0de24db187","unresolved":false,"context_lines":[{"line_number":1922,"context_line":""},{"line_number":1923,"context_line":"        # NOTE(zzzeek) currently this potentially is required for current"},{"line_number":1924,"context_line":"        # tempest runs to pass"},{"line_number":1925,"context_line":"        with context_manager.writer.independent.using(context) as oob_session:"},{"line_number":1926,"context_line":"            oob_session.query(models.ShareAccessMapping).filter_by("},{"line_number":1927,"context_line":"                share_id\u003dshare[\u0027id\u0027]"},{"line_number":1928,"context_line":"            ).soft_delete()"}],"source_content_type":"text/x-python","patch_set":13,"id":"a939b760_05d0e7b6","line":1925,"in_reply_to":"84d2c184_29a8a931","updated":"2024-03-22 16:14:22.000000000","message":"Done","commit_id":"299b36b3f894c2cf5c48b230f9683cfa63361166"}]}