)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"236fe165786c77074eb814fe9e388c694ddd3110","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6889fccc_b634cc74","updated":"2021-12-14 16:45:58.000000000","message":"Good catch, thanks.","commit_id":"7f4714ca3daf2cab26b68bcf4d4202f3969a9119"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"35f9b52b3cdac30dade458b6ceaa1ca459a71370","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"b44da23a_2d809208","updated":"2021-12-15 16:55:57.000000000","message":"I\u0027m going to get this out of the chain, I got a deadlock with this thing and I cannot figure out how or why.","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"6537fb65958eee20466b03f17f967bf58993c608","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"128fc8fd_0d896120","in_reply_to":"111500fb_c843ffc4","updated":"2021-12-15 18:52:30.000000000","message":"Yup, it\u0027s better to be safe.\nDid you notice if those hanging Pods had a kuryrPorts CR already filled with active vif? I sometimes noticed the CR was already filled, but CNI took a while to handle those. Or the Pods were in Pending state","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ca672f413848d163d81e6f0899f1c580228fba24","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cd95461f_4ffd42a3","in_reply_to":"128fc8fd_0d896120","updated":"2021-12-16 08:31:01.000000000","message":"These were in ContainerCreating, I haven\u0027t checked if KuryrPorts were there. I can try to reproduce it later today.","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1c84d8de140ae204db19c7da1e4d6e0d5bdd9e37","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"111500fb_c843ffc4","in_reply_to":"8dcc8239_938519f9","updated":"2021-12-15 18:38:23.000000000","message":"I just saw last 16 pods out of 500 hanging for several minutes and they were grouped by node and namespace. Eventually it could time out but I\u0027m not sure it\u0027s guaranteed as I couldn\u0027t find the case when a deadlock might happen. I just want to play this safe, pools lock + semaphore is pretty good on it\u0027s own, I\u0027m creating 500 pods in 7-8 minutes easily.","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"419432d6d02bccfb45d482d98741b47321828f1a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8dcc8239_938519f9","in_reply_to":"b44da23a_2d809208","updated":"2021-12-15 18:22:08.000000000","message":"okay, can you share a bit what you noticed? was some other request_vifs not handled for really long time?","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"}],"kuryr_kubernetes/controller/drivers/vif_pool.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"76e686a3a2c165154e39ce26d48e8242a87c5169","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        lock \u003d self._get_pool_lock(pool_key, self._port_lock,"},{"line_number":213,"context_line":"                                   self._get_port_lock)"},{"line_number":214,"context_line":"        try:"},{"line_number":215,"context_line":"            if lock.acquire(timeout\u003d300):"},{"line_number":216,"context_line":"                sgs \u003d tuple(sorted(security_groups))"},{"line_number":217,"context_line":"                try:"},{"line_number":218,"context_line":"                    return self._get_port_from_pool(pool_key, pod, subnets,"}],"source_content_type":"text/x-python","patch_set":3,"id":"06aafb48_f04cf7bb","line":215,"range":{"start_line":215,"start_character":28,"end_line":215,"end_character":39},"updated":"2021-12-13 11:42:52.000000000","message":"should this timeout be related to any other timeout value? like VIF activation timeout or similar one?","commit_id":"a3946476a4a4c4f7e4caf51e740ed3923d3472ce"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"97b36b731a08ef2438e3d9d54d64da9f36e53e05","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        lock \u003d self._get_pool_lock(pool_key, self._port_lock,"},{"line_number":213,"context_line":"                                   self._get_port_lock)"},{"line_number":214,"context_line":"        try:"},{"line_number":215,"context_line":"            if lock.acquire(timeout\u003d300):"},{"line_number":216,"context_line":"                sgs \u003d tuple(sorted(security_groups))"},{"line_number":217,"context_line":"                try:"},{"line_number":218,"context_line":"                    return self._get_port_from_pool(pool_key, pod, subnets,"}],"source_content_type":"text/x-python","patch_set":3,"id":"77118b3e_e7586f75","line":215,"range":{"start_line":215,"start_character":28,"end_line":215,"end_character":39},"in_reply_to":"06aafb48_f04cf7bb","updated":"2021-12-13 12:11:29.000000000","message":"Right, maybe we can use vif_annotation_timeout, it\u0027s currently configured with 500 in the cluster network operator and in Kuryr[1] it defaults to 60 seconds. The default in Kuryr might be too small given that we need to create ports in bulk, tag and add to the trunk. I will try to increase it. wdyt?\nBTW The only activation timeout I saw was this one[2] which might be too small for this scenario.\n\n[1] https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/config.py#L43-L47\n[2] https://github.com/openstack/kuryr-kubernetes/blob/47caf5089217537421f2226f0d3a732825a2dcdb/kuryr_kubernetes/controller/drivers/nested_vlan_vif.py#L132","commit_id":"a3946476a4a4c4f7e4caf51e740ed3923d3472ce"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"df5d75bf5df381e5959c941fd042d26244dd75a5","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        lock \u003d self._get_pool_lock(pool_key, self._port_lock,"},{"line_number":213,"context_line":"                                   self._get_port_lock)"},{"line_number":214,"context_line":"        try:"},{"line_number":215,"context_line":"            if lock.acquire(timeout\u003d300):"},{"line_number":216,"context_line":"                sgs \u003d tuple(sorted(security_groups))"},{"line_number":217,"context_line":"                try:"},{"line_number":218,"context_line":"                    return self._get_port_from_pool(pool_key, pod, subnets,"}],"source_content_type":"text/x-python","patch_set":3,"id":"a75e949c_817f6e9c","line":215,"range":{"start_line":215,"start_character":28,"end_line":215,"end_character":39},"in_reply_to":"77118b3e_e7586f75","updated":"2021-12-13 13:57:34.000000000","message":"yep, I was thinking about that vif_annotation_timeout, but if it does not make sense to reuse, then just create a new one with the appropriate value","commit_id":"a3946476a4a4c4f7e4caf51e740ed3923d3472ce"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9ffaa470c91478f2aecfd22aca3990d2be74aefb","unresolved":true,"context_lines":[{"line_number":924,"context_line":"                # since the port is already acquired"},{"line_number":925,"context_line":"                if lock:"},{"line_number":926,"context_line":"                    lock.release()"},{"line_number":927,"context_line":"            os_net.update_port(port_id, security_groups\u003dlist(security_groups))"},{"line_number":928,"context_line":"        if config.CONF.kubernetes.port_debug:"},{"line_number":929,"context_line":"            os_net.update_port(port_id, name\u003dc_utils.get_port_name(pod))"},{"line_number":930,"context_line":"        eventlet.spawn(self._populate_pool, pool_key, pod, subnets,"}],"source_content_type":"text/x-python","patch_set":6,"id":"aa527060_4c42ba0d","line":927,"range":{"start_line":927,"start_character":0,"end_line":927,"end_character":78},"updated":"2021-12-14 16:10:11.000000000","message":"I just realized that on error from this update_port we should put the port back into the pool. Anyway that\u0027s a thing for another patch.","commit_id":"f2f7c1046904ec1c063f076ddac2ee9451c3ea72"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5587b8c1ea75f8f50933761a1ddebadea446cdf3","unresolved":true,"context_lines":[{"line_number":924,"context_line":"                # since the port is already acquired"},{"line_number":925,"context_line":"                if lock:"},{"line_number":926,"context_line":"                    lock.release()"},{"line_number":927,"context_line":"            os_net.update_port(port_id, security_groups\u003dlist(security_groups))"},{"line_number":928,"context_line":"        if config.CONF.kubernetes.port_debug:"},{"line_number":929,"context_line":"            os_net.update_port(port_id, name\u003dc_utils.get_port_name(pod))"},{"line_number":930,"context_line":"        eventlet.spawn(self._populate_pool, pool_key, pod, subnets,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3b0defb1_461a382f","line":927,"range":{"start_line":927,"start_character":0,"end_line":927,"end_character":78},"in_reply_to":"aa527060_4c42ba0d","updated":"2021-12-14 16:37:52.000000000","message":"and I just noticed the lock.release should be outside the else block, going to fix it","commit_id":"f2f7c1046904ec1c063f076ddac2ee9451c3ea72"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"50f68e2eab4a7d2f46b94fefdd92776b223cf317","unresolved":true,"context_lines":[{"line_number":229,"context_line":"                            pool_key, pod, subnets, sgs)"},{"line_number":230,"context_line":"            raise exceptions.ResourceNotReady(pod)"},{"line_number":231,"context_line":"        finally:"},{"line_number":232,"context_line":"            if lock.locked():"},{"line_number":233,"context_line":"                lock.release()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _get_port_from_pool(self, pool_key, pod, subnets, security_groups):"},{"line_number":236,"context_line":"        raise NotImplementedError()"},{"line_number":237,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"169aadee_827db239","line":234,"range":{"start_line":232,"start_character":0,"end_line":234,"end_character":0},"updated":"2021-12-15 15:06:21.000000000","message":"You can\u0027t really do it that way, you might release lock taken by other thread if acquire() happens between locked() and release().","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"419432d6d02bccfb45d482d98741b47321828f1a","unresolved":true,"context_lines":[{"line_number":229,"context_line":"                            pool_key, pod, subnets, sgs)"},{"line_number":230,"context_line":"            raise exceptions.ResourceNotReady(pod)"},{"line_number":231,"context_line":"        finally:"},{"line_number":232,"context_line":"            if lock.locked():"},{"line_number":233,"context_line":"                lock.release()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _get_port_from_pool(self, pool_key, pod, subnets, security_groups):"},{"line_number":236,"context_line":"        raise NotImplementedError()"},{"line_number":237,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e9401599_01e3bee8","line":234,"range":{"start_line":232,"start_character":0,"end_line":234,"end_character":0},"in_reply_to":"169aadee_827db239","updated":"2021-12-15 18:22:08.000000000","message":"That\u0027s true, thanks for the catch.","commit_id":"2d315d6839926c32b309879e52d8af30d42b17bb"}]}
