)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"9dd1a4203840cf9827543fd5dce3f055895af4cf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8cbf230c_3991827e","updated":"2022-10-19 11:40:30.000000000","message":"\u003e Patch Set 1: Code-Review-2\n\u003e \n\u003e This needs to be seriously discussed, because we have to take into consideration many things.\n\u003e Currently we can do multiple parallel backups, and we can have multiple processes in a single Backup service, and we have the \"backup_max_operations\" configuration option to limit the concurrency so we don\u0027t run out of memory or go overboard with the CPU.\n\u003e \n\u003e Adding this without careful consideration would be a regression for many people, as their backup services may blow up.\n\nWhile I agree that all cinder services must not choke the machine to death even with a full queue of stuff to do (as in: many backups being requested) and do so over a longer period of time munching away on the queue.\n\nBut having more parallelism / threads working on the chunks of a single backup is \"just\" another factor for the potentially total threads running.\nAnd it\u0027s less about having all those threads consume CPU cycles but to fight the latency of talking to (remote) (object) storage as well, right?\n\nIn short: I\u0027d rather have that single backup be done quicker (using more cores and parallelism) and do less concurrent backups and the performance numbers with a single thread are just unbearable.\n\n","commit_id":"1f484852d7ea8107b4bece125436e6ca78be7563"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"380b8961c1287f67fcd0fd9e3a3fdf299a793049","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"37b24b82_99f44ded","in_reply_to":"8cbf230c_3991827e","updated":"2022-10-20 14:02:07.000000000","message":"Thanks for review. With default value of 1, we are not change current behaviour. Consider scenario where large volume needs to backup instead of multiple small volumes.","commit_id":"1f484852d7ea8107b4bece125436e6ca78be7563"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b208e8be60ef36abf04cba59629c34e8c59ee323","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6ca72a38_6c3be027","updated":"2022-10-25 12:09:35.000000000","message":"Sorry, but I believe the threading you are using in the patch would give you greenthreads, not native threads, and in any case we already have support for what this patch is trying to do.\n\nPlease look at the \"backup_native_threads_pool_size\" and \"backup_max_operations\" configuration options.","commit_id":"d3b728dab882252ab1f474e39ca42917238f566c"}],"cinder/backup/chunkeddriver.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"2227b5a944d2a37cb519b25f061faae6e978f985","unresolved":true,"context_lines":[{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        counter \u003d 0"},{"line_number":562,"context_line":"        total_block_sent_num \u003d 0"},{"line_number":563,"context_line":"        workers \u003d os.cpu_count()"},{"line_number":564,"context_line":"        pool \u003d concurrent.futures.ThreadPoolExecutor(max_workers\u003dworkers)"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        # There are two mechanisms to send the progress notification."}],"source_content_type":"text/x-python","patch_set":1,"id":"283a9483_89c2a7ad","line":563,"range":{"start_line":563,"start_character":18,"end_line":563,"end_character":32},"updated":"2021-03-10 14:41:07.000000000","message":"I think we\u0027ll want the number of workers to be configurable (maybe cpu count is a reasonable default for that options).","commit_id":"fbfa9df73c21abdd7acd12cc67ecbdf87f78ef4f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"cd0c364428a9bed473f0b65a0fe219a407db3c7c","unresolved":true,"context_lines":[{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        counter \u003d 0"},{"line_number":562,"context_line":"        total_block_sent_num \u003d 0"},{"line_number":563,"context_line":"        workers \u003d os.cpu_count()"},{"line_number":564,"context_line":"        pool \u003d concurrent.futures.ThreadPoolExecutor(max_workers\u003dworkers)"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        # There are two mechanisms to send the progress notification."}],"source_content_type":"text/x-python","patch_set":1,"id":"42a91fc2_a2ffa6e2","line":563,"range":{"start_line":563,"start_character":18,"end_line":563,"end_character":32},"in_reply_to":"283a9483_89c2a7ad","updated":"2021-03-10 14:44:36.000000000","message":"This pool seems to be per backup -- why would we make a pool per-backup based on the number of cpus?  Are we trying to optimize the performance of a single backup?  The case where a handful run at once is probably more interesting.","commit_id":"fbfa9df73c21abdd7acd12cc67ecbdf87f78ef4f"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"64897bae138745464c34fe37af43c4f02ac95965","unresolved":false,"context_lines":[{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        counter \u003d 0"},{"line_number":562,"context_line":"        total_block_sent_num \u003d 0"},{"line_number":563,"context_line":"        workers \u003d os.cpu_count()"},{"line_number":564,"context_line":"        pool \u003d concurrent.futures.ThreadPoolExecutor(max_workers\u003dworkers)"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        # There are two mechanisms to send the progress notification."}],"source_content_type":"text/x-python","patch_set":1,"id":"270c2b4c_12e8195e","line":563,"range":{"start_line":563,"start_character":18,"end_line":563,"end_character":32},"in_reply_to":"42a91fc2_a2ffa6e2","updated":"2021-03-20 09:56:27.000000000","message":"Done","commit_id":"fbfa9df73c21abdd7acd12cc67ecbdf87f78ef4f"}]}