)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"a722f777d5415f2332425653fcd7c607aa589633","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f83dff78_f959dbe9","updated":"2022-06-23 13:20:35.000000000","message":"recheck","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"}],"nodepool/driver/aws/adapter.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"d29dcffe466628c855747f62a1f2a97e8e8e0fb6","unresolved":true,"context_lines":[{"line_number":666,"context_line":"    def _deleteThreadInner(self):"},{"line_number":667,"context_line":"        records \u003d []"},{"line_number":668,"context_line":"        try:"},{"line_number":669,"context_line":"            records.append(self.delete_queue.get(block\u003dTrue, timeout\u003d10))"},{"line_number":670,"context_line":"        except queue.Empty:"},{"line_number":671,"context_line":"            return"},{"line_number":672,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":1,"id":"6bddd74e_a1754762","line":669,"updated":"2022-06-22 13:01:07.000000000","message":"It\u0027s probably unlikely to hit but the terminate_instances can take up to 1000 instance ids with the note that\u0027s recommended to break this up into smaller batches. What do you think about limiting this to 50 per batch?","commit_id":"91c878edb1bb9e4dee8a7c231b75ec359c94ece0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"8097f5a186fa386233c78542338ca87302e9432c","unresolved":false,"context_lines":[{"line_number":666,"context_line":"    def _deleteThreadInner(self):"},{"line_number":667,"context_line":"        records \u003d []"},{"line_number":668,"context_line":"        try:"},{"line_number":669,"context_line":"            records.append(self.delete_queue.get(block\u003dTrue, timeout\u003d10))"},{"line_number":670,"context_line":"        except queue.Empty:"},{"line_number":671,"context_line":"            return"},{"line_number":672,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8b5d3152_615d6ffa","line":669,"updated":"2022-06-22 20:26:03.000000000","message":"Sounds good.","commit_id":"91c878edb1bb9e4dee8a7c231b75ec359c94ece0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"8097f5a186fa386233c78542338ca87302e9432c","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        with self.rate_limiter(log.debug, f\"Deleted {count} instances\"):"},{"line_number":683,"context_line":"            self.ec2_client.terminate_instances(InstanceIds\u003dids)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def _deleteInstance(self, external_id, log\u003dNone):"},{"line_number":686,"context_line":"        if log is None:"},{"line_number":687,"context_line":"            log \u003d self.log"},{"line_number":688,"context_line":"        for instance in self._listInstances():"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffda1f4b_34f1482b","line":685,"updated":"2022-06-22 20:26:03.000000000","message":"Also sounds good.","commit_id":"91c878edb1bb9e4dee8a7c231b75ec359c94ece0"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"d29dcffe466628c855747f62a1f2a97e8e8e0fb6","unresolved":true,"context_lines":[{"line_number":682,"context_line":"        with self.rate_limiter(log.debug, f\"Deleted {count} instances\"):"},{"line_number":683,"context_line":"            self.ec2_client.terminate_instances(InstanceIds\u003dids)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def _deleteInstance(self, external_id, log\u003dNone):"},{"line_number":686,"context_line":"        if log is None:"},{"line_number":687,"context_line":"            log \u003d self.log"},{"line_number":688,"context_line":"        for instance in self._listInstances():"}],"source_content_type":"text/x-python","patch_set":1,"id":"23f3586e_2829a4c5","line":685,"updated":"2022-06-22 13:01:07.000000000","message":"Documentation states that when the delete instances call fails no instance is deleted. So I think for leaked resources we should stay with the direct deletion without the background thread. So what do you think about adding a direct_delete flag here and depending on this put it into the delete queue or directly terminate the instance?","commit_id":"91c878edb1bb9e4dee8a7c231b75ec359c94ece0"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e808a6af30b4a224795c6f683547fe737351b211","unresolved":true,"context_lines":[{"line_number":190,"context_line":"        # values, and only less if users slow down the rate."},{"line_number":191,"context_line":"        workers \u003d max(min(int(self.provider.rate * 4), 8), 1)"},{"line_number":192,"context_line":"        self.log.info(\"Create executor with max workers\u003d%s\", workers)"},{"line_number":193,"context_line":"        self.create_executor \u003d ThreadPoolExecutor(max_workers\u003dworkers)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        # We can batch delete instances using the AWS API, so to do"},{"line_number":196,"context_line":"        # that, create a queue for deletes, and a thread to process"}],"source_content_type":"text/x-python","patch_set":2,"id":"7bc6e519_8f12aeb0","line":193,"updated":"2022-06-29 17:37:51.000000000","message":"I guess the other requests are short enough that we don\u0027t need to farm them out to the thread pool too? Otherwise we may just shift the problem from instance create to making a volume or whatever other little bits are involved.\n\nThat said I suspect that is why deletes are now batched and the other tasks are quick enough to not really matter.","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"04cca55187a9cca8e40dae171e0b524308e49e7d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        # values, and only less if users slow down the rate."},{"line_number":191,"context_line":"        workers \u003d max(min(int(self.provider.rate * 4), 8), 1)"},{"line_number":192,"context_line":"        self.log.info(\"Create executor with max workers\u003d%s\", workers)"},{"line_number":193,"context_line":"        self.create_executor \u003d ThreadPoolExecutor(max_workers\u003dworkers)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        # We can batch delete instances using the AWS API, so to do"},{"line_number":196,"context_line":"        # that, create a queue for deletes, and a thread to process"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba986dcf_f5da0f30","line":193,"updated":"2022-06-29 17:47:05.000000000","message":"Yeah: create takes \u003e 1 sec so that\u0027s bad.  Delete is faster so it\u0027s not a problem.  But we batch it because fewer requests are better regardless.  The only other requests that happen during the normal lifecycle are quota checks (which are heavily cached) and instance lists (also cached).","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"04cca55187a9cca8e40dae171e0b524308e49e7d","unresolved":false,"context_lines":[{"line_number":667,"context_line":"        records \u003d []"},{"line_number":668,"context_line":"        try:"},{"line_number":669,"context_line":"            records.append(self.delete_queue.get(block\u003dTrue, timeout\u003d10))"},{"line_number":670,"context_line":"        except queue.Empty:"},{"line_number":671,"context_line":"            return"},{"line_number":672,"context_line":"        while True:"},{"line_number":673,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"42037642_4bb336d7","line":670,"updated":"2022-06-29 17:47:05.000000000","message":"But we set timeout\u003d10 so it does raise Empty in that case.","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e808a6af30b4a224795c6f683547fe737351b211","unresolved":true,"context_lines":[{"line_number":667,"context_line":"        records \u003d []"},{"line_number":668,"context_line":"        try:"},{"line_number":669,"context_line":"            records.append(self.delete_queue.get(block\u003dTrue, timeout\u003d10))"},{"line_number":670,"context_line":"        except queue.Empty:"},{"line_number":671,"context_line":"            return"},{"line_number":672,"context_line":"        while True:"},{"line_number":673,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"240e6cf0_122db8f3","line":670,"updated":"2022-06-29 17:37:51.000000000","message":"The docs say this exception is only raised with non blocking gets. We block above so this try except shouldn\u0027t be necessary.","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e808a6af30b4a224795c6f683547fe737351b211","unresolved":true,"context_lines":[{"line_number":684,"context_line":"            log.debug(f\"Deleting instance {del_id}\")"},{"line_number":685,"context_line":"        count \u003d len(ids)"},{"line_number":686,"context_line":"        with self.rate_limiter(log.debug, f\"Deleted {count} instances\"):"},{"line_number":687,"context_line":"            self.ec2_client.terminate_instances(InstanceIds\u003dids)"},{"line_number":688,"context_line":""},{"line_number":689,"context_line":"    def _deleteInstance(self, external_id, log\u003dNone, immediate\u003dFalse):"},{"line_number":690,"context_line":"        if log is None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0b02a_50e3f7d4","line":687,"updated":"2022-06-29 17:37:51.000000000","message":"It looks like timeouts to delete are handled in the state machine state machine :) But that means we don\u0027t have to worry about extra deletes for a single instance ending up in this queue. Instead we\u0027ll try once until the timeout. Then later nodepool should notice the deletion failed and try again which will put it in this queue again after it was removed and the attempt expired.","commit_id":"d5b0dee6428ccec04ed2826a9b92e4ea76f583bc"}]}