)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"a44b375cb2d679aeaecd9a677341859812979d3e","unresolved":true,"context_lines":[{"line_number":19,"context_line":"However, when the cache is properly populated, we already have the"},{"line_number":20,"context_line":"rules we should remove from the cache and don\u0027t need to ask neutron"},{"line_number":21,"context_line":"server for them."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I53e11f558a6cd84a02041758badddfe87a10c95c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1626cb2f_9c7e8e4b","line":22,"updated":"2023-06-01 12:48:46.000000000","message":"Do you have launchpad bug also for this issue?","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"},{"author":{"_account_id":9642,"name":"Guillaume Espanel","email":"guillaume.espanel@gmail.com","username":"quatre"},"change_message_id":"d808f60bde0be12edccb44f1370b669476d8f3a3","unresolved":true,"context_lines":[{"line_number":19,"context_line":"However, when the cache is properly populated, we already have the"},{"line_number":20,"context_line":"rules we should remove from the cache and don\u0027t need to ask neutron"},{"line_number":21,"context_line":"server for them."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I53e11f558a6cd84a02041758badddfe87a10c95c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"38e9dfaa_60043cc1","line":22,"in_reply_to":"1626cb2f_9c7e8e4b","updated":"2023-06-02 12:21:30.000000000","message":"We have no bug for this, but maybe there\u0027s an opportunity for one?","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ccc5ee6b221ea995c14ea02a1fc2801014e16d5c","unresolved":true,"context_lines":[{"line_number":19,"context_line":"However, when the cache is properly populated, we already have the"},{"line_number":20,"context_line":"rules we should remove from the cache and don\u0027t need to ask neutron"},{"line_number":21,"context_line":"server for them."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I53e11f558a6cd84a02041758badddfe87a10c95c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"a6850201_454d9ef4","line":22,"in_reply_to":"38e9dfaa_60043cc1","updated":"2023-06-02 13:46:54.000000000","message":"What Lajos is saying is that almost any patch pushed to Neutron should be accompanied by a Launchpad bug, describing the issue https://launchpad.net/neutron. In this case you should create n bug and refer it in the commit message.","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"},{"author":{"_account_id":9642,"name":"Guillaume Espanel","email":"guillaume.espanel@gmail.com","username":"quatre"},"change_message_id":"a8c76d6088e2ec9b4b4beda8102ac5b887f7e727","unresolved":false,"context_lines":[{"line_number":19,"context_line":"However, when the cache is properly populated, we already have the"},{"line_number":20,"context_line":"rules we should remove from the cache and don\u0027t need to ask neutron"},{"line_number":21,"context_line":"server for them."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I53e11f558a6cd84a02041758badddfe87a10c95c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"47d3256d_90235797","line":22,"in_reply_to":"6098576a_82920499","updated":"2023-06-05 13:46:02.000000000","message":"Done","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"6563bf532f763797930b37b8d585653d98e85b5e","unresolved":true,"context_lines":[{"line_number":19,"context_line":"However, when the cache is properly populated, we already have the"},{"line_number":20,"context_line":"rules we should remove from the cache and don\u0027t need to ask neutron"},{"line_number":21,"context_line":"server for them."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I53e11f558a6cd84a02041758badddfe87a10c95c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6098576a_82920499","line":22,"in_reply_to":"a6850201_454d9ef4","updated":"2023-06-02 13:48:34.000000000","message":"+1, yes that\u0027s what I meant, sorry for not telling exactly what I meant","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9c058a84bacb011e13a11ffb360e696f206c55f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9e9c2834_4dd7cb0b","updated":"2023-05-24 15:55:20.000000000","message":"As commented in IRC, please add some testing.\n\nThanks for your time.","commit_id":"d334b31cce0a57a4e89a1562322c51b335efe351"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"9b423898ccffd553e6ace902f38d0cd2a3520f06","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f2a6900b_27b2ba73","updated":"2023-05-25 13:00:10.000000000","message":"For the record, on a quite \"small\" region with 673 computes, running 1346 l2 agents (we have two l2 agents per computes), when I deleted 10 SG, neutron RPC received 12873 bulk_pull (counted with a network trace dump tool).\n\nEach RPC call will then be translated to a SQL request and a RPC answer.\nThe load on both Rabbit and DB is not small.","commit_id":"d334b31cce0a57a4e89a1562322c51b335efe351"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d33f7ded0255784ace49340229d44ccf4574cd80","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"90dce9d5_09947661","updated":"2023-06-05 16:24:24.000000000","message":"(sorry for deleting the vote, that was a mistake)","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fb5e205be32b7ee69b1bde9a0ec78475b31d9a85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4f1e0b66_2179d8ad","updated":"2023-06-05 16:16:09.000000000","message":"This patch is a candidate to be backported.","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"f0a9ed952cd863e47cc29a0c28ee157ce6d40773","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"337173a3_ca032d06","updated":"2023-06-30 11:06:33.000000000","message":"recheck\ntimeout and neutron-ovs-tempest-multinode-full failed with live migration test","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":21798,"name":"Bernard Cafarelli","email":"bcafarel@redhat.com","username":"bcafarel"},"change_message_id":"ed6372a5113c29edc49cdbc3d8172ef922bf3634","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5d797980_3571e521","updated":"2023-06-23 20:24:14.000000000","message":"recheck fullstack","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0f1578e9671953f9bef08aeefa63c5a8957422c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4d363230_863e26d9","updated":"2023-06-05 16:14:51.000000000","message":"recheck neutron-fullstack-with-uwsgi","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ad79265a4b51669bb20594ce76ef3b0edcd9f963","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bc14d53f_7faa8e18","updated":"2023-06-30 11:05:03.000000000","message":"recheck tempest","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8abb2204b8ea97aead930345ac88b3efec27a299","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ec1ca80a_0568a63b","updated":"2023-06-23 07:37:05.000000000","message":"recheck time out","commit_id":"ce12b6ac195c83fad424ee35c5e92b1d12d040ae"}],"neutron/api/rpc/handlers/securitygroups_rpc.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a4583c179a7353f9cdb3b4c978cb876bd0b4b19","unresolved":true,"context_lines":[{"line_number":303,"context_line":"        # about the security group rules. so we need to emulate a rule deletion"},{"line_number":304,"context_line":"        # when a security group is removed."},{"line_number":305,"context_line":"        filters \u003d {\u0027security_group_id\u0027: (existing.id, )}"},{"line_number":306,"context_line":"        for rule in self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters):"},{"line_number":307,"context_line":"            self.rcache.record_resource_delete(context, \u0027SecurityGroupRule\u0027,"},{"line_number":308,"context_line":"                                               rule.id)"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"53909224_9767dcb3","side":"PARENT","line":306,"range":{"start_line":306,"start_character":8,"end_line":306,"end_character":76},"updated":"2023-05-16 14:58:18.000000000","message":"This method won\u0027t execute an RPC call if that is not needed. It will first check if the requested resource is in cache. \"self.rcache.get_resources\" is calling [1] that is calling \"match_resources_with_func\" [2].\n\nWhat is your code improving here? Can you provide some test?\n\n[1]https://github.com/openstack/neutron/blob/ae4084c1735f63de7fc685ee51502f5d0d9db67c/neutron/agent/resource_cache.py#L118\n[2]https://github.com/openstack/neutron/blob/ae4084c1735f63de7fc685ee51502f5d0d9db67c/neutron/agent/resource_cache.py#L144","commit_id":"a06b44e12da4ab8fb037b34593f33c4df7dbafa1"},{"author":{"_account_id":9642,"name":"Guillaume Espanel","email":"guillaume.espanel@gmail.com","username":"quatre"},"change_message_id":"f9bb02bc3e0336f75356d950c0db115f69fa024c","unresolved":true,"context_lines":[{"line_number":303,"context_line":"        # about the security group rules. so we need to emulate a rule deletion"},{"line_number":304,"context_line":"        # when a security group is removed."},{"line_number":305,"context_line":"        filters \u003d {\u0027security_group_id\u0027: (existing.id, )}"},{"line_number":306,"context_line":"        for rule in self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters):"},{"line_number":307,"context_line":"            self.rcache.record_resource_delete(context, \u0027SecurityGroupRule\u0027,"},{"line_number":308,"context_line":"                                               rule.id)"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"72941679_b12a1380","side":"PARENT","line":306,"range":{"start_line":306,"start_character":8,"end_line":306,"end_character":76},"in_reply_to":"53909224_9767dcb3","updated":"2023-05-16 15:15:28.000000000","message":"unless I am missing something, get_resources is calling _flood_cache_for_query, which is performing the bulk_pull [1]. This is the behavior we currently see, and with many thousands nodes and big security groups, the impact of every node querying neutron-rpc for the rules belonging to a secgroup can be significant.\n\nI am not sure what would be relevant to test here that is not already tested. If you have any idea, let me know.\n\n[1]https://github.com/openstack/neutron/blob/ae4084c1735f63de7fc685ee51502f5d0d9db67c/neutron/agent/resource_cache.py#L81","commit_id":"a06b44e12da4ab8fb037b34593f33c4df7dbafa1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9c058a84bacb011e13a11ffb360e696f206c55f8","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        # about the security group rules. so we need to emulate a rule deletion"},{"line_number":304,"context_line":"        # when a security group is removed."},{"line_number":305,"context_line":"        filters \u003d {\u0027security_group_id\u0027: (existing.id, )}"},{"line_number":306,"context_line":"        for rule in self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters):"},{"line_number":307,"context_line":"            self.rcache.record_resource_delete(context, \u0027SecurityGroupRule\u0027,"},{"line_number":308,"context_line":"                                               rule.id)"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"24365771_cb1235a0","side":"PARENT","line":306,"range":{"start_line":306,"start_character":8,"end_line":306,"end_character":76},"in_reply_to":"5c5b5cb3_8fe16c44","updated":"2023-05-24 15:55:20.000000000","message":"After a very productive conversation via IRC, I realize that the goal of this method, ``_clear_child_sg_rules``, is to only delete the SG rules from the cache. In order to retrieve the SG rules, we don\u0027t need the SG (retrieved now in this line). That is an expensive RPC call is the SG is not cached (SG and SG rules are different resources cached in different locations).\n\nThe ``match_resources_with_func`` should work finding the expected SG rules (using the SG id) and then deleting them.\n\nThanks for the time coding this patch and explaining the rationale on IRC.","commit_id":"a06b44e12da4ab8fb037b34593f33c4df7dbafa1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"93aaa10bd4561980d6934828ffa5e0827eabf0b3","unresolved":true,"context_lines":[{"line_number":303,"context_line":"        # about the security group rules. so we need to emulate a rule deletion"},{"line_number":304,"context_line":"        # when a security group is removed."},{"line_number":305,"context_line":"        filters \u003d {\u0027security_group_id\u0027: (existing.id, )}"},{"line_number":306,"context_line":"        for rule in self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters):"},{"line_number":307,"context_line":"            self.rcache.record_resource_delete(context, \u0027SecurityGroupRule\u0027,"},{"line_number":308,"context_line":"                                               rule.id)"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5c5b5cb3_8fe16c44","side":"PARENT","line":306,"range":{"start_line":306,"start_character":8,"end_line":306,"end_character":76},"in_reply_to":"72941679_b12a1380","updated":"2023-05-24 10:24:29.000000000","message":"When ``RemoteResourceCache._flood_cache_for_query`` is called, it first search inside the cache of the ``RemoteResourceCache`` instance for this specific resource. If that resource is already stored inside, it returns [1] and calls \"match_resources_with_func\" [2].\n\nInstead what you are doing here is something that should not be allowed: you are skipping the cache search and you are jumping directly to retrieve the resource without checking if that resource is stored in the cache or not.\n\nActually now checking the code, the method ``match_resources_with_func`` should be private to prevent this misuse. If you need to retrieve an object from the RPC cache instance, you should use only ``get_resources``.\n\n[1]https://github.com/openstack/neutron/blob/0360eb8e127a9f162a987fe2fc8cfefdafdb9c32/neutron/agent/resource_cache.py#L79\n[2]https://github.com/openstack/neutron/blob/0360eb8e127a9f162a987fe2fc8cfefdafdb9c32/neutron/agent/resource_cache.py#L144","commit_id":"a06b44e12da4ab8fb037b34593f33c4df7dbafa1"}],"neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"523e2b5964a6f0d293b87762f1746acbd0c943d7","unresolved":true,"context_lines":[{"line_number":162,"context_line":"            self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters))"},{"line_number":163,"context_line":"        # reset the mock: we care only about what happens at deletion"},{"line_number":164,"context_line":"        self.sg_agent.security_groups_rule_updated.reset_mock()"},{"line_number":165,"context_line":"        self.rcache.record_resource_delete(self.ctx, \u0027SecurityGroup\u0027, s1.id)"},{"line_number":166,"context_line":"        self.assertEqual("},{"line_number":167,"context_line":"            s2.rules,"},{"line_number":168,"context_line":"            self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5674da0a_b64ac8ff","line":165,"range":{"start_line":165,"start_character":8,"end_line":165,"end_character":76},"updated":"2023-05-29 15:32:22.000000000","message":"What we should test in this method is the \"_clear_child_sg_rules\" new implementation, instead of calling the \"record_resource_delete\", that is something done inside this referred method.\n\nYou should also check several conditions:\n* Create s1 and s2, call \"_clear_child_sg_rules\" with s1; check only s2 rules are present (this test)\n* (continue, same test) then call \"_clear_child_sg_rules\" with s2, check no rules are present.\n* (other test) Create s1 and s2, call \"_clear_child_sg_rules\" with s3, check nothing has changed.","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"},{"author":{"_account_id":9642,"name":"Guillaume Espanel","email":"guillaume.espanel@gmail.com","username":"quatre"},"change_message_id":"a8c76d6088e2ec9b4b4beda8102ac5b887f7e727","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters))"},{"line_number":163,"context_line":"        # reset the mock: we care only about what happens at deletion"},{"line_number":164,"context_line":"        self.sg_agent.security_groups_rule_updated.reset_mock()"},{"line_number":165,"context_line":"        self.rcache.record_resource_delete(self.ctx, \u0027SecurityGroup\u0027, s1.id)"},{"line_number":166,"context_line":"        self.assertEqual("},{"line_number":167,"context_line":"            s2.rules,"},{"line_number":168,"context_line":"            self.rcache.get_resources(\u0027SecurityGroupRule\u0027, filters))"}],"source_content_type":"text/x-python","patch_set":2,"id":"afa12395_728c3140","line":165,"range":{"start_line":165,"start_character":8,"end_line":165,"end_character":76},"in_reply_to":"5674da0a_b64ac8ff","updated":"2023-06-05 13:46:02.000000000","message":"Done","commit_id":"1c80a98ccec7ddd4352a5172b29b98484927e205"}]}