)]}'
{"neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"98a56c641de1a1527445ce7f5f671a6258289e0d","unresolved":false,"context_lines":[{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."},{"line_number":207,"context_line":"        "},{"line_number":208,"context_line":"        The method creates a short living connection to the Northbound"},{"line_number":209,"context_line":"        database. Because of multiple controllers can attempt to create the"},{"line_number":210,"context_line":"        Port Group at the same time the transaction can fail and raise"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_7cfa7875","line":207,"updated":"2020-03-19 15:40:47.000000000","message":"Oops","commit_id":"42b654a21f8745ed4f6c6ed1d2493c34d8dcd962"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"10a4237d9e89162685f4c2ec6bad04f35364204d","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        \"\"\"Pre-initialize the ML2/OVN driver.\"\"\""},{"line_number":201,"context_line":"        atexit.register(self._clean_hash_ring)"},{"line_number":202,"context_line":"        signal.signal(signal.SIGTERM, self._clean_hash_ring)"},{"line_number":203,"context_line":"        self._create_neutron_pg_drop()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_357b11f5","line":203,"updated":"2020-04-03 08:57:20.000000000","message":"Quick question, why not do a wait for the port group to be created here in the pre_fork_initialize ?\n\nIf that works, we only need to create 1 short live connection (we can reuse the one from the create method) instead of multiple short lived connections in the post fork (# of workers) to check for this port group.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"fcfb7cb398feb089b6abedbd6c0e2368a2e30f60","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        \"\"\"Pre-initialize the ML2/OVN driver.\"\"\""},{"line_number":201,"context_line":"        atexit.register(self._clean_hash_ring)"},{"line_number":202,"context_line":"        signal.signal(signal.SIGTERM, self._clean_hash_ring)"},{"line_number":203,"context_line":"        self._create_neutron_pg_drop()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_0a2dffa7","line":203,"in_reply_to":"df33271e_357b11f5","updated":"2020-04-03 14:33:53.000000000","message":"I think I originally pointed kuba at post_fork_initialize, and now I don\u0027t know why...this seems like a good idea.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1c69de1e72a9851bf05b7c170acfbc9d9f7cce64","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        \"\"\"Pre-initialize the ML2/OVN driver.\"\"\""},{"line_number":201,"context_line":"        atexit.register(self._clean_hash_ring)"},{"line_number":202,"context_line":"        signal.signal(signal.SIGTERM, self._clean_hash_ring)"},{"line_number":203,"context_line":"        self._create_neutron_pg_drop()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_892d78aa","line":203,"in_reply_to":"df33271e_357b11f5","updated":"2020-04-06 09:58:59.000000000","message":"That\u0027s a good question. The main reason was that Neutron doesn\u0027t handle well the forking - so it happens that API workers post_fork is called before the main process pre_fork ... so because this happens only once per start of the Neutron server, I thought that we should stay on safe side and check in the post-fork for the presence of the PG.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ef888194cbb566d012831180f6c6ac3f2e88bae9","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        \"\"\"Pre-initialize the ML2/OVN driver.\"\"\""},{"line_number":201,"context_line":"        atexit.register(self._clean_hash_ring)"},{"line_number":202,"context_line":"        signal.signal(signal.SIGTERM, self._clean_hash_ring)"},{"line_number":203,"context_line":"        self._create_neutron_pg_drop()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_5c4b30af","line":203,"in_reply_to":"df33271e_7c20b4f8","updated":"2020-04-06 10:32:41.000000000","message":"Oh fair, if that\u0027s the case it makes sense to have the check on post_fork.\n\nBut that said, it seems to be a bug in Neutron ? It does not make any sense to me to have pre and pos methods being called in parallel","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c634a7ed4c1fb5bc293376cd2e17a88da24a391e","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        \"\"\"Pre-initialize the ML2/OVN driver.\"\"\""},{"line_number":201,"context_line":"        atexit.register(self._clean_hash_ring)"},{"line_number":202,"context_line":"        signal.signal(signal.SIGTERM, self._clean_hash_ring)"},{"line_number":203,"context_line":"        self._create_neutron_pg_drop()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def _create_neutron_pg_drop(self):"},{"line_number":206,"context_line":"        \"\"\"Create neutron_pg_drop Port Group."}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_7c20b4f8","line":203,"in_reply_to":"df33271e_892d78aa","updated":"2020-04-06 10:28:56.000000000","message":"I had the same question and I tested this in debug mode. Both \"post_fork_initialize\" and \"pre_fork_initialize\" are called almost at the same time in parallel. Makes sense to add this check in the post_fork.\n\nI also checked [1] forcing this method to wait some time. Once we connect to the IDL, the event appears although it happened before the connection.\n\n[1] https://review.opendev.org/#/c/711404/11/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py@296","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"10a4237d9e89162685f4c2ec6bad04f35364204d","unresolved":false,"context_lines":[{"line_number":218,"context_line":"            try:"},{"line_number":219,"context_line":"                create_default_drop_port_group(pre_ovn_nb_api)"},{"line_number":220,"context_line":"            except RuntimeError as re:"},{"line_number":221,"context_line":"                if pre_ovn_nb_api.get_port_group("},{"line_number":222,"context_line":"                        ovn_const.OVN_DROP_PORT_GROUP_NAME):"},{"line_number":223,"context_line":"                    LOG.debug("},{"line_number":224,"context_line":"                        \"Port Group %(port_group)s already exists, \""},{"line_number":225,"context_line":"                        \"ignoring RuntimeError %(error)s\", {"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_15336dac","line":222,"range":{"start_line":221,"start_character":0,"end_line":222,"end_character":60},"updated":"2020-04-03 08:57:20.000000000","message":"We have a similar check in the create_default_drop_port(), do we need another one here ? Just staying on the safe side I guess","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1c69de1e72a9851bf05b7c170acfbc9d9f7cce64","unresolved":false,"context_lines":[{"line_number":218,"context_line":"            try:"},{"line_number":219,"context_line":"                create_default_drop_port_group(pre_ovn_nb_api)"},{"line_number":220,"context_line":"            except RuntimeError as re:"},{"line_number":221,"context_line":"                if pre_ovn_nb_api.get_port_group("},{"line_number":222,"context_line":"                        ovn_const.OVN_DROP_PORT_GROUP_NAME):"},{"line_number":223,"context_line":"                    LOG.debug("},{"line_number":224,"context_line":"                        \"Port Group %(port_group)s already exists, \""},{"line_number":225,"context_line":"                        \"ignoring RuntimeError %(error)s\", {"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_8991f8f4","line":222,"range":{"start_line":221,"start_character":0,"end_line":222,"end_character":60},"in_reply_to":"df33271e_15336dac","updated":"2020-04-06 09:58:59.000000000","message":"yes, because there still could be a second controller starting at the same time. So this is to handle this very unlikely happening situation where the port group was created in between the check in the transaction and actual commit.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"c3743a1e953254d70d6e7b76c71d9baed313bea0","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(rtheis): This will initialize all workers (API, RPC,"},{"line_number":233,"context_line":"        # plugin service and OVN) with OVN IDL connections."},{"line_number":234,"context_line":"        self._post_fork_event.clear()"},{"line_number":235,"context_line":"        self._wait_for_pg_drop_event()"},{"line_number":236,"context_line":"        self._ovn_client_inst \u003d None"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        is_maintenance \u003d (ovn_utils.get_method_class(trigger) \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_99c98f27","line":235,"range":{"start_line":235,"start_character":13,"end_line":235,"end_character":36},"updated":"2020-04-02 12:00:10.000000000","message":"So here we ensure that PG drop will be created and then the maintenance task will make sure that all ports that must belong to it, are added to the PG? If so, why can\u0027t we let the maint task create it in the first place?\n\nI think we discussed this already but I forgot :( sorry","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"a2ec972ebff0f5cdcc98136eedc1119100ba585a","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(rtheis): This will initialize all workers (API, RPC,"},{"line_number":233,"context_line":"        # plugin service and OVN) with OVN IDL connections."},{"line_number":234,"context_line":"        self._post_fork_event.clear()"},{"line_number":235,"context_line":"        self._wait_for_pg_drop_event()"},{"line_number":236,"context_line":"        self._ovn_client_inst \u003d None"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        is_maintenance \u003d (ovn_utils.get_method_class(trigger) \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_143ee6bf","line":235,"range":{"start_line":235,"start_character":13,"end_line":235,"end_character":36},"in_reply_to":"df33271e_398cc3d5","updated":"2020-04-02 12:16:18.000000000","message":"True, that is right! Thanks a lot for clarifying","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e601be4539e0fea0f9a406602770698923a6b265","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(rtheis): This will initialize all workers (API, RPC,"},{"line_number":233,"context_line":"        # plugin service and OVN) with OVN IDL connections."},{"line_number":234,"context_line":"        self._post_fork_event.clear()"},{"line_number":235,"context_line":"        self._wait_for_pg_drop_event()"},{"line_number":236,"context_line":"        self._ovn_client_inst \u003d None"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        is_maintenance \u003d (ovn_utils.get_method_class(trigger) \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_398cc3d5","line":235,"range":{"start_line":235,"start_character":13,"end_line":235,"end_character":36},"in_reply_to":"df33271e_99c98f27","updated":"2020-04-02 12:05:21.000000000","message":"It\u0027s a good question.\n\nWe don\u0027t rely on maintenance task to add all the ports back, that\u0027s done here on L1216. Thus all ports are filtered as soon as mech driver is initialized.\n\nThe maintenance task is asynchronous and creating it before workers start processing requests is just to avoid a case where a request to create a port comes before the maintenance task kicks in. It\u0027s a small chance but it\u0027s better to be safe than sorry.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f3e3eafb2331c479ef2b127727fcd7384b8af09c","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        nb_sync.migrate_to_port_groups(admin_context)"},{"line_number":231,"context_line":"        raise periodics.NeverAgain()"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    @periodics.periodic(spacing\u003d10, run_immediately\u003dTrue)"},{"line_number":234,"context_line":"    def create_pg_drop(self):"},{"line_number":235,"context_line":"        \"\"\"Create OVN_DROP_PORT_GROUP_NAME port group."},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_f8e1679c","line":233,"updated":"2020-03-05 15:30:04.000000000","message":"Since this happens in the maintenance thread, but *after* self._ovn_client exists, is it possible that the non-maintenance thread workers will go ahead and run before this has a chance to complete?\n\nI was thinking that maybe having this run after the db connections are made, but before post_fork_event is set would be a good idea?","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cccdc7cd30bc8fa8bc59ee6292327b40af3bf538","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        nb_sync.migrate_to_port_groups(admin_context)"},{"line_number":231,"context_line":"        raise periodics.NeverAgain()"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    @periodics.periodic(spacing\u003d10, run_immediately\u003dTrue)"},{"line_number":234,"context_line":"    def create_pg_drop(self):"},{"line_number":235,"context_line":"        \"\"\"Create OVN_DROP_PORT_GROUP_NAME port group."},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_3b204fac","line":233,"in_reply_to":"1fa4df85_f8e1679c","updated":"2020-03-06 07:41:07.000000000","message":"You mean just calling the create_pg_drop method from post_fork_initialize?","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"24eb74a50162edc40973769e6b551fc02143272c","unresolved":false,"context_lines":[{"line_number":2166,"context_line":"        pg_name \u003d ovn_const.OVN_DROP_PORT_GROUP_NAME"},{"line_number":2167,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_9da14549","line":2169,"range":{"start_line":2169,"start_character":20,"end_line":2169,"end_character":24},"updated":"2020-03-05 14:57:10.000000000","message":"Maybe this could be debug to not appear in the logs by default.","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f506acce15f0532918a7bf9a5cb5f5f0a53e1708","unresolved":false,"context_lines":[{"line_number":2166,"context_line":"        pg_name \u003d ovn_const.OVN_DROP_PORT_GROUP_NAME"},{"line_number":2167,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_7bdec7da","line":2169,"range":{"start_line":2169,"start_character":20,"end_line":2169,"end_character":24},"in_reply_to":"1fa4df85_9da14549","updated":"2020-03-06 07:05:48.000000000","message":"It appears once for the lifetime and if the entry is missing in the db, it basically means we won\u0027t be able to create ports. In my opinion info is good, what do you think?","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"24eb74a50162edc40973769e6b551fc02143272c","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_dd9b3d15","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"updated":"2020-03-05 14:57:10.000000000","message":"I\u0027m wondering: Why did we have a problem with concurrent creation in the first place if we have this may_exist\u003dTrue here and in line 2174 and ports\u003dNone (as was originally called in line 2182)?","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"9a85a8ef79956554c932cb4dd03febe4c529b388","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_9b1fe406","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_273d8ddc","updated":"2020-03-09 13:48:59.000000000","message":"Thanks for the explanation and the traceback. Now I\u0027m quite sure I reproduced a different bug, not the one fixed here. I tried adding a sleep() to the place you indicated to catch the race but could not reproduce this bug. I think the reason was my dev environment not having an HA neutron-server. It only has a single neutron server with 2 api workers. Which are likely sharing the same in-memory db copy.\n\nBy the way the functional tests run against patch set #5 seem to have caught a similar problem in the sync_repair mechanism driver.","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"a6db2fef7d7c1d2be2ef16e658633aa0b49f9fc2","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_c1a02e93","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_3bf60f65","updated":"2020-03-06 09:48:47.000000000","message":"I assume you asked me. :-)\n\nI\u0027m not sure if it\u0027s related. I was trying to reproduce the problem (as described in the bug report). And on a high level I saw the same failure (one out of two port creates failing), but the stack trace showed a different ovsdb operation to fail, not what\u0027s being changed here.\n\nSo I\u0027m not sure - maybe I found a different problem. Do you have the original stack trace when you opened the bug? We could compare the two and maybe know more after that.","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f506acce15f0532918a7bf9a5cb5f5f0a53e1708","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_1bf31376","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_9802b36e","updated":"2020-03-06 07:05:48.000000000","message":"That\u0027s right.","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6e152d8cc427bd7640df6ede721a95e3bb0bc0a9","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_273d8ddc","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_c1a02e93","updated":"2020-03-06 16:26:31.000000000","message":"The problem here was that we need a single entry in the OVN NB database - the neutron_pg_drop entry. This is a unique entry that is needed to be created only once in a cloud lifetime and before, it was created with first security group association with port. This patch makes the neutron_pg_drop being created on server startup and then never again.\n\nI don\u0027t know much about QoS in OVN but this seems like it\u0027s creating an entry per port every time a new port is created.\n\nI put the original trace in the launchpad: https://bugs.launchpad.net/neutron/+bug/1866068/comments/4\n\nI apologize for not pasting it in the description\n\nThanks for reproducing it! The race is when neutron_pg_drop is created by a different transaction between https://github.com/openstack/ovsdbapp/blob/575219d54b066d2b5fa0a6ffd30620a139d66211/ovsdbapp/schema/ovn_northbound/commands.py#L1240 and L1246","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"769d4f2cd8f3aade42ce56ea8377a052653f7d38","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_de349395","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_dd9b3d15","updated":"2020-03-05 16:41:43.000000000","message":"Coming back after I managed to reproduce this bug. The stacktrace I added to the bugreport shows another ovn nb operation to fail:\n\nhttps://opendev.org/openstack/neutron/src/commit/77616fd1773ebca4a6261eec91b2fc5c4ece8e77/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L419","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"649a942f91e617fcb0d086bd09093bcdd96e4575","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_9802b36e","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_dd9b3d15","updated":"2020-03-05 15:24:17.000000000","message":"I think it was multiple servers running at the same time and the pg getting created on one between the lookup() call in pg_add command and actually writing it to the db. We don\u0027t get a failure until *after the response from ovsdb-server*. So the in-memory copy doesn\u0027t have the update yet, but the db itself has changed.","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f506acce15f0532918a7bf9a5cb5f5f0a53e1708","unresolved":false,"context_lines":[{"line_number":2168,"context_line":"            if not self._nb_idl.get_port_group(pg_name):"},{"line_number":2169,"context_line":"                LOG.info(\"Creating %s port group\", pg_name)"},{"line_number":2170,"context_line":"                # If drop Port Group doesn\u0027t exist yet, create it."},{"line_number":2171,"context_line":"                txn.add(self._nb_idl.pg_add(pg_name, acls\u003d[], may_exist\u003dTrue))"},{"line_number":2172,"context_line":"                # Add ACLs to this Port Group so that all traffic is dropped."},{"line_number":2173,"context_line":"                acls \u003d ovn_acl.add_acls_for_drop_port_group(pg_name)"},{"line_number":2174,"context_line":"                for acl in acls:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_3bf60f65","line":2171,"range":{"start_line":2171,"start_character":62,"end_line":2171,"end_character":76},"in_reply_to":"1fa4df85_de349395","updated":"2020-03-06 07:05:48.000000000","message":"Is it related to this fix?","commit_id":"3a42757bfe7ab9ec62427c03a04a571817c0e023"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"56a12bfc3aeb4a3b7e405e03f6dadf00ecfcd0a2","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            for table in cls.tables:"},{"line_number":543,"context_line":"                helper.register_table(table)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        return cls(driver, connection_string, helper)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_736e5e00","line":545,"updated":"2020-04-02 21:45:34.000000000","message":"If we wanted to optimize as much as possible, we could limit to only the neutron_pg_drop row with something like:\n\n idl \u003d cls(driver, connection_string, helper)\n idl.cond_change(\u0027Port_Group\u0027, [[\u0027name\u0027, \u0027\u003d\u003d\u0027, \u0027neutron_pg_drop\u0027]])\n return idl\n\nat least it still seems to work when I try it. Not sure if premature optimization, though.","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"10a4237d9e89162685f4c2ec6bad04f35364204d","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            for table in cls.tables:"},{"line_number":543,"context_line":"                helper.register_table(table)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        return cls(driver, connection_string, helper)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_55eb3576","line":545,"in_reply_to":"df33271e_3fc93025","updated":"2020-04-03 08:57:20.000000000","message":"++ to test this","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1c69de1e72a9851bf05b7c170acfbc9d9f7cce64","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            for table in cls.tables:"},{"line_number":543,"context_line":"                helper.register_table(table)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        return cls(driver, connection_string, helper)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_dc2580ad","line":545,"in_reply_to":"df33271e_736e5e00","updated":"2020-04-06 09:58:59.000000000","message":"Thanks for the tip, I\u0027ll definitely try that out. I\u0027ll rename the class so it\u0027s specific to the port group and if we need to extend it in the future, we can extend the conditions. Thanks!","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fbdd127372ff66c426603555e7fdc77cc0f89c9","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            for table in cls.tables:"},{"line_number":543,"context_line":"                helper.register_table(table)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        return cls(driver, connection_string, helper)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_3fc93025","line":545,"in_reply_to":"df33271e_736e5e00","updated":"2020-04-03 07:34:55.000000000","message":"This is nice \u0026 elegant. It\u0027ll make the startup time of neutron-server faster (unless the conditional monitoring is bad for ovsdb-server :p)","commit_id":"f0af025ad28cfb899e1e4f874fb8f9cb8b714abf"}]}
