)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":25903,"name":"Vu Cong Tuan","email":"tuanvc@vn.fujitsu.com","username":"tuanvu"},"change_message_id":"722248adab38b4d392a91c9c9f736d60cae8d6e8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Migrate to using Subnet and SubnetPool OVO objects in the IPAM subnet"},{"line_number":10,"context_line":"allocation code."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I0f0a3482a5a91881927cfe39fc40e042d6ac10cd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f7c97a3_1d8317ab","line":11,"updated":"2018-07-05 13:51:47.000000000","message":"Partially-Implements: bp/adopt-oslo-versioned-objects-for-db\nfor consistency with other patches?","commit_id":"b48912c71954ce2c8807602f3121ef3fc234be40"}],"neutron/ipam/subnet_alloc.py":[{"author":{"_account_id":28581,"name":"Lujin Luo","email":"lujinluo@gmail.com","username":"lujin"},"change_message_id":"57349628843989e03cbab8a59c17af3ff31283d4","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_0c48762a","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":28},"updated":"2018-08-16 09:45:30.000000000","message":"I did some experiments in my local environment. What I did is to change this line to \n\n        count \u003d subnet_pool.update()\n        if not count:\n            raise db_exc.RetryRequest(lib_exc.SubnetPoolInUse(\n                                      subnet_pool_id\u003dself._subnetpool[\u0027id\u0027]))\n\nThe reason I think this would work is that, .update() is also guarded by transaction. If the transaction is rolled back, it would return None, but if it succeeds, it would return 0. This is the same as query.update()\n\nBy making this change, the failed UT is passed. But I have not checked if this will cause any regression.","commit_id":"d76340ea48d888052c3921361ab8720a0e6722ca"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"4c589c190b507e56918a25ab711ccae737c272c9","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_c91ca1c3","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":28},"in_reply_to":"3f79a3b5_0c48762a","updated":"2018-08-16 19:43:53.000000000","message":"Giving it a whirl.  How do you think it would be best to check for regressions in this particular spot?","commit_id":"d76340ea48d888052c3921361ab8720a0e6722ca"},{"author":{"_account_id":28581,"name":"Lujin Luo","email":"lujinluo@gmail.com","username":"lujin"},"change_message_id":"8eb8ab8756d3e599150223c203406af161ca03a6","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_6a03f874","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":28},"in_reply_to":"3f79a3b5_c91ca1c3","updated":"2018-08-17 01:25:58.000000000","message":"I simply meant I have not checked if this change will cause other UT/FTs to fail. I just respinned to fix pep8 failure. Let\u0027s see how Zuul responds.","commit_id":"d76340ea48d888052c3921361ab8720a0e6722ca"},{"author":{"_account_id":28581,"name":"Lujin Luo","email":"lujinluo@gmail.com","username":"lujin"},"change_message_id":"57a28c4f4c115bc3faf9e788d872403570e5296d","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":60,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":61,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":62,"context_line":"        count \u003d subnet_pool.update()"},{"line_number":63,"context_line":"        if not count:"},{"line_number":64,"context_line":"            raise db_exc.RetryRequest(lib_exc.SubnetPoolInUse("},{"line_number":65,"context_line":"                                      subnet_pool_id\u003dself._subnetpool[\u0027id\u0027]))"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":68,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_46d8747e","line":65,"range":{"start_line":62,"start_character":8,"end_line":65,"end_character":77},"updated":"2018-08-17 03:45:39.000000000","message":"I might be wrong. I need to dig more.","commit_id":"6ec327619c328408407a64888caef2f5f7cbf2b5"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"f107973ba3fea1ee34d36521512038b10e8e6869","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":60,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":61,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":62,"context_line":"        count \u003d subnet_pool.update()"},{"line_number":63,"context_line":"        if not count:"},{"line_number":64,"context_line":"            raise db_exc.RetryRequest(lib_exc.SubnetPoolInUse("},{"line_number":65,"context_line":"                                      subnet_pool_id\u003dself._subnetpool[\u0027id\u0027]))"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":68,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_73d85aee","line":65,"range":{"start_line":62,"start_character":8,"end_line":65,"end_character":77},"in_reply_to":"3f79a3b5_46d8747e","updated":"2018-08-22 15:22:36.000000000","message":"Based on the extensive unit test failures, I think you are probably right about there being regression issues.","commit_id":"6ec327619c328408407a64888caef2f5f7cbf2b5"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fb2fe5823ae9d3e278230199a906a01abdd6039c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":50,"context_line":"        subnetpool."},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        subnet_pool \u003d subnetpool_obj.SubnetPool.get_objects(self._context,"},{"line_number":53,"context_line":"                id\u003dself._subnetpool[\u0027id\u0027]).pop()"},{"line_number":54,"context_line":"        current_hash \u003d subnet_pool.hash if subnet_pool else None"},{"line_number":55,"context_line":"        if current_hash is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_0205cb0f","line":52,"updated":"2018-09-19 20:45:34.000000000","message":"get_object() here instead of get_objects()? The subnetpool is being locked by ID, so we can pull the specific subnetpool directly without using pop().","commit_id":"b8e9292921d2ba7d9a4fa2de419128df3be03b0d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0142b5a97cac396794c70d56c6fb5841421d6fb9","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":50,"context_line":"        subnetpool."},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        subnet_pool \u003d subnetpool_obj.SubnetPool.get_objects(self._context,"},{"line_number":53,"context_line":"                id\u003dself._subnetpool[\u0027id\u0027]).pop()"},{"line_number":54,"context_line":"        current_hash \u003d subnet_pool.hash if subnet_pool else None"},{"line_number":55,"context_line":"        if current_hash is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_3c93fca6","line":52,"in_reply_to":"3f79a3b5_0205cb0f","updated":"2018-09-25 08:39:49.000000000","message":"+1","commit_id":"b8e9292921d2ba7d9a4fa2de419128df3be03b0d"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"8fb8f4c7794e96323bfbde39f7d07e6992cf7d53","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":50,"context_line":"        subnetpool."},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        subnet_pool \u003d subnetpool_obj.SubnetPool.get_objects(self._context,"},{"line_number":53,"context_line":"                id\u003dself._subnetpool[\u0027id\u0027]).pop()"},{"line_number":54,"context_line":"        current_hash \u003d subnet_pool.hash if subnet_pool else None"},{"line_number":55,"context_line":"        if current_hash is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_13ecd826","line":52,"in_reply_to":"3f79a3b5_0205cb0f","updated":"2018-11-01 14:44:46.000000000","message":"Done","commit_id":"b8e9292921d2ba7d9a4fa2de419128df3be03b0d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0142b5a97cac396794c70d56c6fb5841421d6fb9","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_352ea38c","line":60,"updated":"2018-09-25 08:39:49.000000000","message":"Test \"test_subnetpool_concurrent_allocation_exception\" is failing because you are still mocking \"sqlalchemy.orm.query.Query.update\", in [1]\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/tests/unit/ipam/test_subnet_alloc.py#L199","commit_id":"b8e9292921d2ba7d9a4fa2de419128df3be03b0d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1d9f268f97f645fa846b91948c95ed29e3175a27","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_1e3fff86","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":28},"updated":"2018-11-01 14:07:15.000000000","message":"You are going to have the same problems as in PS10","commit_id":"9d71fea7807134a447e237c6e4062c4f29b892c9"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"8fb8f4c7794e96323bfbde39f7d07e6992cf7d53","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            raise n_exc.SubnetPoolNotFound("},{"line_number":58,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":59,"context_line":"        subnet_pool.hash \u003d uuidutils.generate_uuid()"},{"line_number":60,"context_line":"        subnet_pool.update()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def _get_allocated_cidrs(self):"},{"line_number":63,"context_line":"        subnets \u003d subnet_obj.Subnet.get_objects(self._context,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_3930d5b7","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":28},"in_reply_to":"3f79a3b5_1e3fff86","updated":"2018-11-01 14:44:46.000000000","message":"I actually removed the test because the exception it is checking for, and the scenario it is intended to check for,  does not appear to exist in the OVO codepath.","commit_id":"9d71fea7807134a447e237c6e4062c4f29b892c9"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"8e27a5efb6e0b25675e18cc29ba87d0a54158b5b","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":61,"context_line":"        new_hash \u003d uuidutils.generate_uuid()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        # NOTE(cbrandily): the update disallows 2 concurrent subnet allocation"},{"line_number":64,"context_line":"        # to succeed: at most 1 transaction will succeed, others will be"},{"line_number":65,"context_line":"        # rolled back and be caught in neutron.db.v2.base"},{"line_number":66,"context_line":"        with db_api.CONTEXT_WRITER.using(self._context):"},{"line_number":67,"context_line":"            query \u003d ("},{"line_number":68,"context_line":"                self._context.session.query(models_v2.SubnetPool).filter_by("}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_aaaa4218","side":"PARENT","line":65,"range":{"start_line":63,"start_character":0,"end_line":65,"end_character":57},"updated":"2018-11-01 19:58:59.000000000","message":"The original code is for ensuring only one caller can lock the subnetpool if there are multiple callers tried to do it at the same time. In particular, what the code below does is a compare-and-swap operation (not a normal OVO update). It compares if the hash equals to current hash and update to new hash only if this condition satisfies.\n\nHowever, the OVO conversion in this patch seems to replace a compare-and-swap operation with an simple DB update, which doesn\u0027t seem to be right.","commit_id":"4a4fa0db7f4c4275faa3b6528ce74d3b91506ca4"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"eab3c1be597656f8bd1e91002b1b5b0cf623bcbf","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                subnetpool_id\u003dself._subnetpool[\u0027id\u0027])"},{"line_number":61,"context_line":"        new_hash \u003d uuidutils.generate_uuid()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        # NOTE(cbrandily): the update disallows 2 concurrent subnet allocation"},{"line_number":64,"context_line":"        # to succeed: at most 1 transaction will succeed, others will be"},{"line_number":65,"context_line":"        # rolled back and be caught in neutron.db.v2.base"},{"line_number":66,"context_line":"        with db_api.CONTEXT_WRITER.using(self._context):"},{"line_number":67,"context_line":"            query \u003d ("},{"line_number":68,"context_line":"                self._context.session.query(models_v2.SubnetPool).filter_by("}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_a56273d9","side":"PARENT","line":65,"range":{"start_line":63,"start_character":0,"end_line":65,"end_character":57},"in_reply_to":"3f79a3b5_aaaa4218","updated":"2018-11-01 20:05:16.000000000","message":"Yes, I missed this in earlier reviews. Good catch, this indeed fails to perform a compare and swap.","commit_id":"4a4fa0db7f4c4275faa3b6528ce74d3b91506ca4"}]}