)]}'
{"ironic/conductor/manager.py":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"98f8e43dca05078350d173ef83ef03b630a047d4","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            LOG.audit(\"Spawning worker for state %s\" % new_state)"},{"line_number":210,"context_line":"            self.worker_pool.spawn("},{"line_number":211,"context_line":"                utils.node_power_action,"},{"line_number":212,"context_line":"                task, task.node, new_state)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    # NOTE(deva): There is a race condition in the RPC API for vendor_passthru."},{"line_number":215,"context_line":"    # Between the validate_vendor_action and do_vendor_action calls, it\u0027s"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F9oFU%3D","line":212,"updated":"2014-02-05 15:14:44.000000000","message":"I\u0027m sure this intentional, just for example, but, still need manual locking here. Once we out of \"WITH\" the task object will be modified (lock released)","commit_id":"6b700fee755897d85815a48346b694ff3f0be7a3"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"2123892fad140cad5c3e4cadc093230499d239de","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            LOG.audit(\"Spawning worker for state %s\" % new_state)"},{"line_number":210,"context_line":"            self.worker_pool.spawn("},{"line_number":211,"context_line":"                utils.node_power_action,"},{"line_number":212,"context_line":"                task, task.node, new_state)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    # NOTE(deva): There is a race condition in the RPC API for vendor_passthru."},{"line_number":215,"context_line":"    # Between the validate_vendor_action and do_vendor_action calls, it\u0027s"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F9n1I%3D","line":212,"in_reply_to":"AAAAUH%2F9oFU%3D","updated":"2014-02-05 15:47:06.000000000","message":"Yep, you\u0027re right.","commit_id":"6b700fee755897d85815a48346b694ff3f0be7a3"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"a59754e212f3a45ed826da159b2859c2fcc0045b","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        LOG.audit(\"Spawning worker for state %s\" % new_state)"},{"line_number":215,"context_line":"        thd \u003d self.worker_pool.spawn("},{"line_number":216,"context_line":"                utils.node_power_action,"},{"line_number":217,"context_line":"                task, task.node, new_state)"},{"line_number":218,"context_line":"        thd.link(_release)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    # NOTE(deva): There is a race condition in the RPC API for vendor_passthru."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9lTg%3D","line":217,"updated":"2014-02-05 22:26:15.000000000","message":"Sorry, I should have been noticed this in previous review..\n\nutils.node_power_action should be reworked similar to my patch too. We need to separate what we want to do async, and what to do sync. Because currently it\u0027s completely async and therefore this patch only resolves race lock problem, but doesn\u0027t solve validation. There are two ways here\n\n1. To make two public methods: utils.validate_node_power_action and utils_node_power_action. Call the first here in usual way, call the second inside a green pool. This may make the conductor code more complex..\n\n2. To make self.worker_pool public (well, it\u0027s already public:)) and call it from inside of utils, return created thread from utils. E.g. move logic from 1 to utils.\n\nIn general green pool approach seems clear and straightforward to me. Thanks for the research!\n\nAlso, we still can use futures here - if our worker_pool will be a ThreadPoolExecutor object instead of GreenPool. Please take a look on the example \n\n    def start(self):\n        self.executor \u003d ThreadPoolExecutor(max_workers\u003dCONF.rpc_pool_size)\n\n    def foo(self):\n        a \u003d self.executor.submit(foo1)\n        b \u003d self.executor.submit(foo2)\n\nThread pool executor have max_workers limitation which is basically the pool size.It seems futures will provide more convenient api, and also we will be able to switch to ProcessPoolExecutor (60 processes per node is not too much right? as a pros - we will gain all the benefits from sub-processes: 1. if something goes wrong in working process the main conductor service remains online, 2. if conductor dies for some reason, all the sub-processes remain their execution until they done - lower probability of data corruption). But processes is another discussion I think.\n\nI\u0027ll compare both approaches (futures.ThreadPoolExecutur vs GreenPool) and share what I have","commit_id":"e441ba38c0b17fd39f211569ec738a9bfeb4738c"}]}