)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"28a4bcdd081505db378380a09871af9b46264ed9","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In \"DbQuotaNoLockDriver\", when a new reservation is being made,"},{"line_number":10,"context_line":"first the expired reservations are removed. That guarantees the"},{"line_number":11,"context_line":"fresness of the existing reservations."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In systems with high concurrency of operations, the"},{"line_number":14,"context_line":"\"DbQuotaNoLockDriver.make_reservation\" method will be called in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f2242d53_71fa52ee","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":8},"updated":"2021-12-14 11:28:55.000000000","message":"nit: freshness","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"91bb03304a5016a56f0d1f345eb3e4c7b94ff9c0","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In \"DbQuotaNoLockDriver\", when a new reservation is being made,"},{"line_number":10,"context_line":"first the expired reservations are removed. That guarantees the"},{"line_number":11,"context_line":"fresness of the existing reservations."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In systems with high concurrency of operations, the"},{"line_number":14,"context_line":"\"DbQuotaNoLockDriver.make_reservation\" method will be called in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"86ecad90_cbf38544","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":8},"in_reply_to":"f2242d53_71fa52ee","updated":"2021-12-14 13:53:17.000000000","message":"Done","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"28a4bcdd081505db378380a09871af9b46264ed9","unresolved":true,"context_lines":[{"line_number":15,"context_line":"parallel. The expired reservations removal implies a deletion"},{"line_number":16,"context_line":"on the \"reservation\" table that could be executed by several"},{"line_number":17,"context_line":"workers at the same time (in the same controller or not). That"},{"line_number":18,"context_line":"could lead to a \"DBDeadlock\" exception if multiple workers want"},{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9659390b_a0d7c6ef","line":19,"range":{"start_line":18,"start_character":16,"end_line":19,"end_character":29},"updated":"2021-12-14 11:28:55.000000000","message":"AFAIK DBDeadLock could happen even if multiple workers want\nto delete different registers, it depends on DBMS - those that are using table locks.","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bd2bb49c1cf55d7c0553930726c007d861ae0d19","unresolved":true,"context_lines":[{"line_number":15,"context_line":"parallel. The expired reservations removal implies a deletion"},{"line_number":16,"context_line":"on the \"reservation\" table that could be executed by several"},{"line_number":17,"context_line":"workers at the same time (in the same controller or not). That"},{"line_number":18,"context_line":"could lead to a \"DBDeadlock\" exception if multiple workers want"},{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d23e90b2_f3ef16bc","line":19,"range":{"start_line":18,"start_character":16,"end_line":19,"end_character":29},"in_reply_to":"59e975ee_c486a5a7","updated":"2021-12-14 14:30:43.000000000","message":"you mean we use in upstream CI?","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"91bb03304a5016a56f0d1f345eb3e4c7b94ff9c0","unresolved":true,"context_lines":[{"line_number":15,"context_line":"parallel. The expired reservations removal implies a deletion"},{"line_number":16,"context_line":"on the \"reservation\" table that could be executed by several"},{"line_number":17,"context_line":"workers at the same time (in the same controller or not). That"},{"line_number":18,"context_line":"could lead to a \"DBDeadlock\" exception if multiple workers want"},{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"59e975ee_c486a5a7","line":19,"range":{"start_line":18,"start_character":16,"end_line":19,"end_character":29},"in_reply_to":"9659390b_a0d7c6ef","updated":"2021-12-14 13:53:17.000000000","message":"Right but by default the DB backends we use will raise this exception in this case, as we use backends with row level granularity.","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"321da8e438afad1b04aa9829879591b5c3f07786","unresolved":true,"context_lines":[{"line_number":15,"context_line":"parallel. The expired reservations removal implies a deletion"},{"line_number":16,"context_line":"on the \"reservation\" table that could be executed by several"},{"line_number":17,"context_line":"workers at the same time (in the same controller or not). That"},{"line_number":18,"context_line":"could lead to a \"DBDeadlock\" exception if multiple workers want"},{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1b038f58_b4248c64","line":19,"range":{"start_line":18,"start_character":16,"end_line":19,"end_character":29},"in_reply_to":"d23e90b2_f3ef16bc","updated":"2021-12-14 15:01:11.000000000","message":"Yes and in any deployment using MySQL, MariaDB and PostgreSQL with InnoDB tables.","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"28a4bcdd081505db378380a09871af9b46264ed9","unresolved":true,"context_lines":[{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"},{"line_number":23,"context_line":"should not retry this operation."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The default reservation expiration timeout is set to 120 seconds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6fb21532_affb10de","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":61},"updated":"2021-12-14 11:28:55.000000000","message":"This assumption may not be true if other worker deleted expired reservations of different tenant/project","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"321da8e438afad1b04aa9829879591b5c3f07786","unresolved":true,"context_lines":[{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"},{"line_number":23,"context_line":"should not retry this operation."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The default reservation expiration timeout is set to 120 seconds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"73ab190b_cd1f3f03","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":61},"in_reply_to":"30c9ae62_52f642ce","updated":"2021-12-14 15:01:11.000000000","message":"Yes for several reasons (and I\u0027ll this to the commit message to be more explicit, thanks for pointing out this):\n1) Those expired reservations will be eventually deleted by any worker that succeeds with the deletion command.\n2) When calculating the available resources we (a) count the number of used resources (that calculation will depend on the type of driver) and (b) the current number of reservations.\nWe retrieve the reservations in [1][2] filtering out the expired reservations [3]. \n\n[1]https://github.com/openstack/neutron/blob/e99d9a9d0697a21ba7ec84465f415f60041f3767/neutron/quota/resource.py#L340\n[2]https://github.com/openstack/neutron/blob/e99d9a9d0697a21ba7ec84465f415f60041f3767/neutron/db/quota/api.py#L226\n[3]https://github.com/openstack/neutron/blob/e99d9a9d0697a21ba7ec84465f415f60041f3767/neutron/objects/quota.py#L100-L101","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"91bb03304a5016a56f0d1f345eb3e4c7b94ff9c0","unresolved":true,"context_lines":[{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"},{"line_number":23,"context_line":"should not retry this operation."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The default reservation expiration timeout is set to 120 seconds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c4b5496a_0bb94960","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":61},"in_reply_to":"6fb21532_affb10de","updated":"2021-12-14 13:53:17.000000000","message":"Right but that won\u0027t affect to us. Of course that will depend on the lock level granularity. By default we use engines that provide row locks.\n\nIf the lock is at table level, then will have bigger problems than this one, in case of high load.\n\nA system should not have expired reservations in the table. This is just a clean-up method to keep the reservations table healthy.","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bd2bb49c1cf55d7c0553930726c007d861ae0d19","unresolved":true,"context_lines":[{"line_number":19,"context_line":"to delete the same registers."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"In case an API worker receives this exception, it should continue"},{"line_number":22,"context_line":"as the expired reservations have been deleted by other worker. It"},{"line_number":23,"context_line":"should not retry this operation."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The default reservation expiration timeout is set to 120 seconds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"30c9ae62_52f642ce","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":61},"in_reply_to":"c4b5496a_0bb94960","updated":"2021-12-14 14:30:43.000000000","message":"\u003e\u003e If the lock is at table level, then will have bigger problems than this one, in case of high load.\n\nI think I remember such problems with Galera clusters - that time DB retries helped with this exact issue.\n\nSo can these left/not retried expired reservations be an issue for someone who uses table locks?","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"a10f1ed903f2f48772230e37e2eecca4ec4ab09b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"758f9d41_f5951fe5","updated":"2021-12-14 00:35:09.000000000","message":"Don\u0027t we need any sort of testing for this change?","commit_id":"7a4c7a06ac41a042a64954415c6773c619d9ad7f"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"4c7acc4d21ceec438dbc66f28578a0b52d4781b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e6fbb265_a2a2b575","updated":"2021-12-14 00:34:42.000000000","message":"recheck","commit_id":"7a4c7a06ac41a042a64954415c6773c619d9ad7f"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e92eb24ffb852cd228f6fbbcc3ea3b19157f9967","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8f8d648f_bc14f64c","in_reply_to":"758f9d41_f5951fe5","updated":"2021-12-14 10:02:06.000000000","message":"Done","commit_id":"7a4c7a06ac41a042a64954415c6773c619d9ad7f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"28a4bcdd081505db378380a09871af9b46264ed9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"707f9533_0a4ee591","updated":"2021-12-14 11:28:55.000000000","message":"Not sure retry is not needed","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"91bb03304a5016a56f0d1f345eb3e4c7b94ff9c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3d3e4663_950835f4","in_reply_to":"707f9533_0a4ee591","updated":"2021-12-14 13:53:17.000000000","message":"This is what I\u0027m trying to explain in the commit message. This deletion command is a way to keep the \"reservations\" table healthy. By default we should not have expired reservations; this command is just a guard. Any request that issues a reservation will first clean up the table. If we are executing multiple requests at the same time, at least one of them will be able to finish the operation.","commit_id":"dff4b378e92940e509bef396001d4f2467cae2e5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"321da8e438afad1b04aa9829879591b5c3f07786","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a465a19f_c0a8421a","updated":"2021-12-14 15:01:11.000000000","message":"BTW, just in case. For the ","commit_id":"856fe2a4c46d9c76433d1d9a8ff09e9bde8d9c69"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"a1d8d992d8823c9a5be189c1e227093124ef655a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"35567efb_4dbe377e","updated":"2021-12-14 21:11:23.000000000","message":"This looks good to me. I think we need to wait a bit for the dependencies problem to be fixed in CI before merging","commit_id":"2dd3ffa271d68b4e042ff64fcc2657af6990e95f"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4ec5842bbf51035b1874fefe3f4d3de3c9be9c23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"40c0ac14_eb64a9b4","updated":"2021-12-15 08:04:23.000000000","message":"recheck","commit_id":"2dd3ffa271d68b4e042ff64fcc2657af6990e95f"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"bbad091f0027d57d14c8289444b2b297a64e4999","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"96813dea_b6dc75fb","updated":"2021-12-14 21:13:42.000000000","message":"recheck","commit_id":"2dd3ffa271d68b4e042ff64fcc2657af6990e95f"}]}