)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"f7a67d33e146c4cd2fb50e06fdda599f759d8f54","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dane LeBlanc \u003cleblancd@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-20 18:30:47 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Reuse callers session in ML2 DB methods"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch changes the get_port_from_device_mac() and"},{"line_number":10,"context_line":"get_sg_ids_grouped_by_port() methods in ML2 db.py module so that they do"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"dac4157f_bbff9800","line":7,"updated":"2015-04-21 04:52:56.000000000","message":"can you tweak the commit message to make it clear that this is not an ML2-specific change?","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"d27db5788eb48274c502a71bc1e7581753a3a820","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dane LeBlanc \u003cleblancd@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-20 18:30:47 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Reuse callers session in ML2 DB methods"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch changes the get_port_from_device_mac() and"},{"line_number":10,"context_line":"get_sg_ids_grouped_by_port() methods in ML2 db.py module so that they do"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"dac4157f_92136e0d","line":7,"in_reply_to":"dac4157f_bbff9800","updated":"2015-04-21 22:53:42.000000000","message":"I\u0027ll add a paragraph explaining why the plugin API methods are being changed in order to enable the reuse of the context session in the ML2 plugin.  But I could use suggestions on how to word the summary line of the commit message (since the problem area being addressed is in the ML2 DB module, and the other changes are done to support this change). Let me know if you have some ideas for the summary line, or other clarifications that I can make in the commit message.","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"83cf4d355aeed7b332020bf950a3898aefd78815","unresolved":false,"context_lines":[{"line_number":11,"context_line":"they do not create a new database session (via get_session()), but"},{"line_number":12,"context_line":"instead reuse the session associated with the caller\u0027s context."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"The motivation for this change is to avoid a possible database"},{"line_number":15,"context_line":"deadlock as described in:"},{"line_number":16,"context_line":"   https://bugs.launchpad.net/neutron/+bug/1440183/comments/13"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1aca2d91_8a92249e","line":14,"updated":"2015-05-07 20:24:33.000000000","message":"I\u0027m fine with this change just because it fits in with the pattern we have of passing the context everywhere, but this particular change will not help with deadlocks. \n\nThere is nothing in the RPC session that starts a transaction with a lock and yields.\n\nCan you remove this particular paragraph? There is already quite a bit of confusion about what causes the deadlocks, and we don\u0027t want to imply that what was done in this particular case will reduce the chance of them.","commit_id":"0a9ae575550469fb8f853a97a5a3bfa4d321c083"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"ddbe6c09e2bce6bff4dd8d588fab25effe47aaa4","unresolved":false,"context_lines":[{"line_number":11,"context_line":"they do not create a new database session (via get_session()), but"},{"line_number":12,"context_line":"instead reuse the session associated with the caller\u0027s context."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"The motivation for this change is to avoid a possible database"},{"line_number":15,"context_line":"deadlock as described in:"},{"line_number":16,"context_line":"   https://bugs.launchpad.net/neutron/+bug/1440183/comments/13"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1aca2d91_40ed0dee","line":14,"in_reply_to":"1aca2d91_2abb10e7","updated":"2015-05-07 21:21:21.000000000","message":"Sure, I\u0027ll remove this. I had added this based on an earlier review comment saying that we shouldn\u0027t make internal API changes lightly. To be clear, there is a risky yield that happens in get_sg_ids_grouped_by_port() without this change (yield happens during the __exit__() processing at the end of \u0027with session.begin(...)\u0027, but you are absolutely correct that there is nothing currently in the calling chain that is doing a lock, so there is no chance of deadlock.","commit_id":"0a9ae575550469fb8f853a97a5a3bfa4d321c083"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"5f59309bf703354e100de9b64e988af7280591ad","unresolved":false,"context_lines":[{"line_number":11,"context_line":"they do not create a new database session (via get_session()), but"},{"line_number":12,"context_line":"instead reuse the session associated with the caller\u0027s context."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"The motivation for this change is to avoid a possible database"},{"line_number":15,"context_line":"deadlock as described in:"},{"line_number":16,"context_line":"   https://bugs.launchpad.net/neutron/+bug/1440183/comments/13"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1aca2d91_2abb10e7","line":14,"in_reply_to":"1aca2d91_8a92249e","updated":"2015-05-07 20:28:33.000000000","message":"Agreed.","commit_id":"0a9ae575550469fb8f853a97a5a3bfa4d321c083"}],"neutron/db/securitygroups_rpc_base.py":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"f7a67d33e146c4cd2fb50e06fdda599f759d8f54","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                                    \"or get_ports_from_devices.\")"},{"line_number":60,"context_line":"                                  % self.__class__.__name__)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def get_ports_from_devices(self, context, devices):"},{"line_number":63,"context_line":"        \"\"\"Bulk method of get_port_from_device."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        Subclasses may override this to provide better performance for DB"}],"source_content_type":"text/x-python","patch_set":3,"id":"dac4157f_0059715a","line":62,"updated":"2015-04-21 04:52:56.000000000","message":"it\u0027s a plugin api, which should not be changed lightly.","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"d27db5788eb48274c502a71bc1e7581753a3a820","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                                    \"or get_ports_from_devices.\")"},{"line_number":60,"context_line":"                                  % self.__class__.__name__)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def get_ports_from_devices(self, context, devices):"},{"line_number":63,"context_line":"        \"\"\"Bulk method of get_port_from_device."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        Subclasses may override this to provide better performance for DB"}],"source_content_type":"text/x-python","patch_set":3,"id":"dac4157f_26c6a848","line":62,"in_reply_to":"dac4157f_0059715a","updated":"2015-04-21 22:53:42.000000000","message":"The motivation for this plugin api change is that we\u0027re trying to\neliminate a possible database deadlock condition in the\nget_sg_ids_grouped_by_port() in ML2 db.py, and in order to do\nthis, we need the context (or its associated database session)\npassed down via this plugin API. The problem in\nget_sg_ids_grouped_by_port() is that it creates a *new* session\nand starts a transaction:\n    session \u003d db_api.get_session()\n    ...\n    with session.begin(subtransactions\u003dTrue)\n        \u003csome database queries\u003e\nAs explained in this comment:\nhttps://bugs.launchpad.net/neutron/+bug/1440183/comments/13\nat the __exit__ point of this session.begin context, there is a\ncall to the oslo_db method _thread_yield in this case. This\nyield can cause a deadlock e.g. if one of the callers into this\nget_ports_from_devices API has placed a prior update lock on\ndatabase records, and the thread that is yielded to then tries to\naccess the same records.","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"f7a67d33e146c4cd2fb50e06fdda599f759d8f54","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        Subclasses may override this to provide better performance for DB"},{"line_number":66,"context_line":"        queries, backend calls, etc."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        return [self.get_port_from_device(device) for device in devices]"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def create_security_group_rule(self, context, security_group_rule):"},{"line_number":71,"context_line":"        bulk_rule \u003d {\u0027security_group_rules\u0027: [security_group_rule]}"}],"source_content_type":"text/x-python","patch_set":3,"id":"dac4157f_00305100","line":68,"range":{"start_line":68,"start_character":21,"end_line":68,"end_character":41},"updated":"2015-04-21 04:52:56.000000000","message":"leaving get_port_from_device while changing get_ports_from_devices seems a bit ad-hoc.","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"d27db5788eb48274c502a71bc1e7581753a3a820","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        Subclasses may override this to provide better performance for DB"},{"line_number":66,"context_line":"        queries, backend calls, etc."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        return [self.get_port_from_device(device) for device in devices]"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def create_security_group_rule(self, context, security_group_rule):"},{"line_number":71,"context_line":"        bulk_rule \u003d {\u0027security_group_rules\u0027: [security_group_rule]}"}],"source_content_type":"text/x-python","patch_set":3,"id":"dac4157f_8cec7390","line":68,"in_reply_to":"dac4157f_00305100","updated":"2015-04-21 22:53:42.000000000","message":"I\u0027ll change get_port_from_device to make it consistent with get_ports_from_devices.","commit_id":"e2377b3680e4afa1f2103a0a402d99d631547ecd"}],"neutron/plugins/oneconvergence/plugin.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6a6e10fce0bad3fc2fdd29b3c6c5cad836d00887","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    @staticmethod"},{"line_number":59,"context_line":"    def get_port_from_device(context, device):"},{"line_number":60,"context_line":"        port \u003d nvsd_db.get_port_from_device(device)"},{"line_number":61,"context_line":"        if port:"},{"line_number":62,"context_line":"            port[\u0027device\u0027] \u003d device"},{"line_number":63,"context_line":"        return port"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aca2d91_6e6fd32b","line":60,"updated":"2015-05-08 02:11:01.000000000","message":"This could be updated too, although the patch is geared towards ML2 only. The code is here:\nhttps://github.com/openstack/neutron/blob/master/neutron/plugins/oneconvergence/lib/nvsd_db.py#L23","commit_id":"47dd65cf986d712e9c6ca5dcf4420dfc44900b66"}]}
