)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cd328c2708149e3d949ea28c4cb639bb23ab301b","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_98a03aa1","line":25,"updated":"2015-06-08 08:49:28.000000000","message":"This is a misconception we continue to propagate. Galera *does* support select for update, and handles it safely. The difference is, it handles it safely by aborting one of the conflicting transactions at or before commit rather than by waiting if the transactions are on different nodes. If the transactions are on the same node, it will wait as normal.\n\nWe should stop saying that Galera doesn\u0027t support select for update, because it is both wrong and confusing.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"63412c4af70bc8c1ba1bf8e4d36bce715dea900a","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_ef77589d","line":25,"in_reply_to":"3afb71cf_3ba6d653","updated":"2015-06-08 14:34:09.000000000","message":"Interestingly, while Galera *does* safely support:\n\n  select foo from bar for update;\n  update bar set foo \u003d foo + 1;\n\nI don\u0027t think this code is safe after all, because there is no update. The safety of the write-intent locks relies on a remote write conflicting with a locally-held write-intent lock during local certification.\n\nFirstly, Galera\u0027s support for write intent locks:\n\nWhen Galera commits a transaction, the first thing it does is check for conflicts with other writesets in the pending queue from other nodes. As this is a local check, it has the context of the write intent lock: it didn\u0027t have to replicate it. If a remote write conflicts with the local lock, the transaction is aborted.\n\nThe same happens when remote writesets are committed locally, btw. If the write causes a conflict with a local lock, the local transaction is aborted.\n\nSo, if you have several transactions all doing:\n\n  select foo from bar for update;\n  update bar set foo \u003d foo + 1;\n\nthe first transaction to commit will be replicate the update to bar to all nodes. This will cause all the other transactions to fail local certification because of the conflict with their locally-held write intent locks.\n\nHowever, in this particular case there is a write-intent lock with no write, so there is nothing to generate a conflict. You could work round this by asserting that all writes on Galera go to a single node. Alternatively, you might do something like add a generation column to the subnet pool table. Bit of a hack, but it would work.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c6a9a5db12b0ee663fc2bccbea077373e2ccc1ac","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_460b72d6","line":25,"in_reply_to":"3afb71cf_8480c247","updated":"2015-06-10 21:42:35.000000000","message":"@Matthew, your point is well taken.  Thanks.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"25c0a48eef2241038c7a7927acf87de0bb311fed","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_8727b611","line":25,"in_reply_to":"3afb71cf_98a03aa1","updated":"2015-06-08 10:29:52.000000000","message":"Agree. it is better to rephrase it.\n\nIf conflicting transactions are sent to one node, it works, but if they are sent to different nodes, SELECT FOR UPDATE leads to \"deadlock\" error and transactions will raise exceptions more than we expect when we use SELECT FOR UPDATE in our code.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"7aaf67bbaad1dab4b80ba3a965cab3eefdf478cc","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_fba4beb9","line":25,"in_reply_to":"3afb71cf_98a03aa1","updated":"2015-06-08 13:23:05.000000000","message":"Some references to a documentation would be appreciated as it does not correspond to jaypipes point of view nor to Galera developers (meet at Vancouver).","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a447c71a713d66b058267e46da8d2c34fcd2a738","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_f073b413","line":25,"in_reply_to":"3afb71cf_eaece62a","updated":"2015-06-08 15:28:37.000000000","message":"@Matthew:  You\u0027re missing a very important piece here.  See my comment here [1].  This is not propagating any misconceptions.  It is true that galera *does not* support select for update!  The write intent locks a simply not propagated across all active writers.\n\nYour example is naive and doesn\u0027t actually illustrate support for it.  It merely illustrates that your contrived example will still fail.  But, it fails with a different failure mode: not because galera supports select for update but because galera will fail the write certification with a deadlock later on in the transaction.  It isn\u0027t the same and it doesn\u0027t work in this case.\n\n[1] https://review.openstack.org/#/c/179955/7/neutron/ipam/subnet_alloc.py","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"8a1f271332836926b286d4c2824bac5b273ea02b","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_eaece62a","line":25,"in_reply_to":"3afb71cf_ef77589d","updated":"2015-06-08 14:54:57.000000000","message":"@Matthew: \"you might do something like add a generation column to the subnet pool table. Bit of a hack, but it would work.\", that\u0027s the aim of the follow-up change (https://review.openstack.org/187985) which you should not be reviewed as it\u0027s more a backup than a \"under review\" code.\n\nbtw, 2 changes are done in order to solve postgres trouble quickly (1st change) and galera trouble correctly (2nd change).","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"371a764c0fa79545b13624d503bb67af2c5f9a7e","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_8480c247","line":25,"in_reply_to":"3afb71cf_f073b413","updated":"2015-06-09 08:28:46.000000000","message":"The documentation sucks in this area. This blog describes the operation of the local certification check:\n\nhttps://www.percona.com/blog/2012/11/20/understanding-multi-node-writing-conflict-metrics-in-percona-xtradb-cluster-and-galera/\n\nThis explains why if you take a row lock and then update that row, Galera will maintain the same ACID semantics albeit by a different mechanism: rollback instead of waiting. It doesn\u0027t need to propagate write intent locks to do this. The failure mode isn\u0027t relevant here, only that it has predictable behaviour.\n\nHowever, as you point out there *is* a semantic difference here because we hold a row lock while updating a different row. I hadn\u0027t considered this last time I gave this significant thought, so I need to row back on my original statement: they\u0027re clearly not the same. Don\u0027t write it off, though, because it can be used. Just not here :)","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"c5980bc57b3e45fe281342ef14004bee4955974e","unresolved":false,"context_lines":[{"line_number":22,"context_line":"the lock ensures the lock is done on a non-empty select."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But this change does not ensure non-overlapping cidrs in subnetpools"},{"line_number":25,"context_line":"with galera because galera doesn\u0027t support SELECT FOR UPDATE locks. A"},{"line_number":26,"context_line":"follow-up change will (try to?) remove locks from subnet allocation[1]"},{"line_number":27,"context_line":"in order to ensure non-overlapping cidrs in subnetpools also with galera."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3afb71cf_3ba6d653","line":25,"in_reply_to":"3afb71cf_fba4beb9","updated":"2015-06-08 13:32:36.000000000","message":"The mailing list discussion: \"MySQL Galera does *not* support SELECT ... FOR UPDATE\"[1]\n\n[1] http://lists.openstack.org/pipermail/openstack-dev/2014-May/035264.html","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"}],"neutron/ipam/subnet_alloc.py":[{"author":{"_account_id":14258,"name":"Ryan Tidwell","email":"rktidwell85@gmail.com","username":"tidwellr"},"change_message_id":"ea97e0272fd9ae4adddc243926695c3951ad9192","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            self._check_subnetpool_tenant_quota(session,"},{"line_number":101,"context_line":"                                                request.tenant_id,"},{"line_number":102,"context_line":"                                                request.prefixlen)"},{"line_number":103,"context_line":"            self._lock_subnetpool(session)"},{"line_number":104,"context_line":"            prefix_pool \u003d self._get_available_prefix_list(session)"},{"line_number":105,"context_line":"            for prefix in prefix_pool:"},{"line_number":106,"context_line":"                if request.prefixlen \u003e\u003d prefix.prefixlen:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_04293be6","line":103,"updated":"2015-05-05 04:50:24.000000000","message":"I think we need to lock the subnetpool before the quota check above.","commit_id":"be3462162d5c9fd3f94543e0197e0deee88135fc"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"729aab96c936db4456b77b8282ea0b6f69a27716","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            self._check_subnetpool_tenant_quota(session,"},{"line_number":101,"context_line":"                                                request.tenant_id,"},{"line_number":102,"context_line":"                                                request.prefixlen)"},{"line_number":103,"context_line":"            self._lock_subnetpool(session)"},{"line_number":104,"context_line":"            prefix_pool \u003d self._get_available_prefix_list(session)"},{"line_number":105,"context_line":"            for prefix in prefix_pool:"},{"line_number":106,"context_line":"                if request.prefixlen \u003e\u003d prefix.prefixlen:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_703305ac","line":103,"in_reply_to":"3acd31a7_04293be6","updated":"2015-05-05 08:35:58.000000000","message":"Done","commit_id":"be3462162d5c9fd3f94543e0197e0deee88135fc"},{"author":{"_account_id":14258,"name":"Ryan Tidwell","email":"rktidwell85@gmail.com","username":"tidwellr"},"change_message_id":"ea97e0272fd9ae4adddc243926695c3951ad9192","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                                request.tenant_id,"},{"line_number":125,"context_line":"                                                request.prefixlen)"},{"line_number":126,"context_line":"            subnet \u003d request.subnet"},{"line_number":127,"context_line":"            self._lock_subnetpool(session)"},{"line_number":128,"context_line":"            available \u003d self._get_available_prefix_list(session)"},{"line_number":129,"context_line":"            matched \u003d netaddr.all_matching_cidrs(subnet, available)"},{"line_number":130,"context_line":"            if len(matched) is 1 and matched[0].prefixlen \u003c\u003d subnet.prefixlen:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_a41ba7c9","line":127,"updated":"2015-05-05 04:50:24.000000000","message":"See comment on line 103","commit_id":"be3462162d5c9fd3f94543e0197e0deee88135fc"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"729aab96c936db4456b77b8282ea0b6f69a27716","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                                request.tenant_id,"},{"line_number":125,"context_line":"                                                request.prefixlen)"},{"line_number":126,"context_line":"            subnet \u003d request.subnet"},{"line_number":127,"context_line":"            self._lock_subnetpool(session)"},{"line_number":128,"context_line":"            available \u003d self._get_available_prefix_list(session)"},{"line_number":129,"context_line":"            matched \u003d netaddr.all_matching_cidrs(subnet, available)"},{"line_number":130,"context_line":"            if len(matched) is 1 and matched[0].prefixlen \u003c\u003d subnet.prefixlen:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_5024895e","line":127,"in_reply_to":"3acd31a7_a41ba7c9","updated":"2015-05-05 08:35:58.000000000","message":"Done","commit_id":"be3462162d5c9fd3f94543e0197e0deee88135fc"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"0af595bdc984618396e3c5efdd5c16cc4120fbde","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def _allocate_any_subnet(self, session, request):"},{"line_number":99,"context_line":"        with session.begin(subtransactions\u003dTrue):"},{"line_number":100,"context_line":"            self._lock_subnetpool(session)"},{"line_number":101,"context_line":"            self._check_subnetpool_tenant_quota(session,"},{"line_number":102,"context_line":"                                                request.tenant_id,"},{"line_number":103,"context_line":"                                                request.prefixlen)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_a40c3b43","line":100,"updated":"2015-05-12 10:59:34.000000000","message":"the lock should be released by parent transaction as the insertion is done in parent transaction","commit_id":"8cbfa31e60d5d202da4fa89f942fb17704f52fea"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"aadddbff7998f543a7b34e38940c8862ce774854","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def _allocate_any_subnet(self, session, request):"},{"line_number":99,"context_line":"        with session.begin(subtransactions\u003dTrue):"},{"line_number":100,"context_line":"            self._lock_subnetpool(session)"},{"line_number":101,"context_line":"            self._check_subnetpool_tenant_quota(session,"},{"line_number":102,"context_line":"                                                request.tenant_id,"},{"line_number":103,"context_line":"                                                request.prefixlen)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_290d7e90","line":100,"in_reply_to":"9a0c5dc1_a40c3b43","updated":"2015-05-15 16:36:01.000000000","message":"plz, forget my comment","commit_id":"8cbfa31e60d5d202da4fa89f942fb17704f52fea"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"3696df7fcddb167543235a2b6fd019361ace0d39","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._subnetpool \u003d subnetpool"},{"line_number":38,"context_line":"        self._sp_helper \u003d SubnetPoolHelper()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def lock_subnetpool(self, session):"},{"line_number":41,"context_line":"        \"\"\"Lock subnetpool associated row."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        This method disallows to allocate concurrently 2 subnets in the same"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_7d492dd0","line":40,"updated":"2015-05-14 12:15:01.000000000","message":"why to make this an allocator api, rather than making allocate_subnet do it internally?","commit_id":"cfef35109e0fa77980a0d005acfad96e9fcc9fa9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"1269bb3cb060c5372e3c61cef08a33ec8a22b260","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._subnetpool \u003d subnetpool"},{"line_number":38,"context_line":"        self._sp_helper \u003d SubnetPoolHelper()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def lock_subnetpool(self, session):"},{"line_number":41,"context_line":"        \"\"\"Lock subnetpool associated row."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        This method disallows to allocate concurrently 2 subnets in the same"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_fd6cfd09","line":40,"in_reply_to":"7a016987_7d492dd0","updated":"2015-05-14 12:26:41.000000000","message":"The subnet is persisted in _create_subnet_from_pool, that s why lock_subnetpool in its transaction not in allocate_subnet one","commit_id":"cfef35109e0fa77980a0d005acfad96e9fcc9fa9"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"a07b53c72a6b435ad85b4fbc0dcad87799119c50","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._subnetpool \u003d subnetpool"},{"line_number":38,"context_line":"        self._sp_helper \u003d SubnetPoolHelper()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def lock_subnetpool(self, session):"},{"line_number":41,"context_line":"        \"\"\"Lock subnetpool associated row."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        This method disallows to allocate concurrently 2 subnets in the same"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_2e6224ae","line":40,"in_reply_to":"7a016987_e3eb0877","updated":"2015-05-15 16:07:52.000000000","message":"let me explain my understanding.\nsqlalchemy\u0027s subtransactions\u003dTrue does not create separate transactions.  where to acquire row locks is not important.\nonce you lock rows, they will be held until the end of the entire transaction.","commit_id":"cfef35109e0fa77980a0d005acfad96e9fcc9fa9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"a28b0aeca0a85ad0bae170deb79219aabafd075d","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._subnetpool \u003d subnetpool"},{"line_number":38,"context_line":"        self._sp_helper \u003d SubnetPoolHelper()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def lock_subnetpool(self, session):"},{"line_number":41,"context_line":"        \"\"\"Lock subnetpool associated row."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        This method disallows to allocate concurrently 2 subnets in the same"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_e3eb0877","line":40,"in_reply_to":"7a016987_fd6cfd09","updated":"2015-05-15 14:55:42.000000000","message":"i could also call it at the beginning of allocate_subnet outside of allocate_subnet transaction. But it implies lock release is only ensured because  _create_subnet_from_pool embeds allocate_subnet call in a parent transaction ... imo, it means assuming too much on how allocated_subnet will be used!","commit_id":"cfef35109e0fa77980a0d005acfad96e9fcc9fa9"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"25c0a48eef2241038c7a7927acf87de0bb311fed","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Lock subnetpool associated row."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        This method disallows to allocate concurrently 2 subnets in the same"},{"line_number":45,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":46,"context_line":"        subnetpool."},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        # FIXME(cbrandily): not working with Galera"}],"source_content_type":"text/x-python","patch_set":7,"id":"5afe65bd_1e69fdbe","line":45,"updated":"2015-06-08 10:29:52.000000000","message":"nit: the comma should be a period.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cd328c2708149e3d949ea28c4cb639bb23ab301b","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":46,"context_line":"        subnetpool."},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        # FIXME(cbrandily): not working with Galera"},{"line_number":49,"context_line":"        (self._context.session.query(models_v2.SubnetPool.id)."},{"line_number":50,"context_line":"         filter_by(id\u003dself._subnetpool[\u0027id\u0027])."},{"line_number":51,"context_line":"         with_lockmode(\u0027update\u0027).first())"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_d8b11291","line":48,"updated":"2015-06-08 08:49:28.000000000","message":"This works on Galera. In fact, in the event that 2 conflicting transactions happen on the same db node, this will behave exactly as you expect.\n\nIn the event that the conflicting transactions happen on different nodes, the losing transaction(s) will be aborted at or before commit. In other words, it\u0027s not dissimilar to optimistic lockless. If you leave it as is, a locking conflict will result in a \u0027deadlock\u0027 being raised. This is obviously not a deadlock, but that\u0027s how it\u0027s reported by the db layer, so there you have it. If you wrap the caller in a retry block you can automatically retry on a lock conflict.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a447c71a713d66b058267e46da8d2c34fcd2a738","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":46,"context_line":"        subnetpool."},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        # FIXME(cbrandily): not working with Galera"},{"line_number":49,"context_line":"        (self._context.session.query(models_v2.SubnetPool.id)."},{"line_number":50,"context_line":"         filter_by(id\u003dself._subnetpool[\u0027id\u0027])."},{"line_number":51,"context_line":"         with_lockmode(\u0027update\u0027).first())"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_f01cd4c2","line":48,"in_reply_to":"3afb71cf_d8b11291","updated":"2015-06-08 15:28:37.000000000","message":"I\u0027m not so sure this is true.  The lock is on the SubnetPool row and the changes are made to the subnet pool in a way that does not violate constraints[*].  So, what is going to conflict?  There is nothing to prevent the write certification from succeeding.\n\n[*] The way subnets are stored doesn\u0027t lend itself to any kind of database level constraint (that I know of) to enforce the invariant that subnets do not overlap.  We could have reorganized the tables a bit and sacrificed storage space to achieve it but we chose to lock around the subnet pool instead.  We a sacrificed a finer grained lock to keep the database structure as it was and prevent it from blowing up.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"76bfa8bad84617ae5ac6bb9f1a6c99c9125caf9f","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        subnetpool, it\u0027s required to ensure non-overlapping cidrs in the same"},{"line_number":46,"context_line":"        subnetpool."},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        # FIXME(cbrandily): not working with Galera"},{"line_number":49,"context_line":"        (self._context.session.query(models_v2.SubnetPool.id)."},{"line_number":50,"context_line":"         filter_by(id\u003dself._subnetpool[\u0027id\u0027])."},{"line_number":51,"context_line":"         with_lockmode(\u0027update\u0027).first())"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_6ddec640","line":48,"in_reply_to":"3afb71cf_f01cd4c2","updated":"2015-06-08 18:26:59.000000000","message":"Minor correct:  \"The lock is on the SubnetPool row and the changes are made to the subnet table in a way ...\"","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"25c0a48eef2241038c7a7927acf87de0bb311fed","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        subnetpool."},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        # FIXME(cbrandily): not working with Galera"},{"line_number":49,"context_line":"        (self._context.session.query(models_v2.SubnetPool.id)."},{"line_number":50,"context_line":"         filter_by(id\u003dself._subnetpool[\u0027id\u0027])."},{"line_number":51,"context_line":"         with_lockmode(\u0027update\u0027).first())"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5afe65bd_fef621d6","line":49,"updated":"2015-06-08 10:29:52.000000000","message":"Nit: I am not sure why SubnetPool.id is queried.\nSELECT for UPDATE locks a selected row, so I think query(models_v2.SubnetPool) is enough. In my understanding there is no difference. I might be wrong.","commit_id":"3682e3391f188845d0c7f382f0ccd4b38db3904e"}]}