)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0f08a9012ef3548ee744bb9443b7d693d06b85e6","unresolved":false,"context_lines":[{"line_number":14,"context_line":"Provide a configuration option and spawn a sync manager thread to"},{"line_number":15,"context_line":"rate limit the number of parallel sync activities happening during"},{"line_number":16,"context_line":"the _sync_power_states period task."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I3d518249e6513d60b61d3877e7f647e82df60ed0"},{"line_number":19,"context_line":"Close-bug: #1388077"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5a890539_454a8584","line":17,"updated":"2014-11-10 09:09:39.000000000","message":"Can you please add a DocImpact flag","commit_id":"b802d241c8cafe119ecfbb71bf730c30054cc3b2"}],"nova/compute/manager.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"0ec894814b03d1eb626e833ec674d08df5feb79e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    cfg.IntOpt(\u0027sync_power_state_concurrency\u0027,"},{"line_number":142,"context_line":"               default\u003d1,"},{"line_number":143,"context_line":"               help\u003d\u0027The level of concurrency that will be used during sync \u0027"},{"line_number":144,"context_line":"                    \u0027of power states between the database and the hypervisor.\u0027),"},{"line_number":145,"context_line":"    cfg.IntOpt(\"heal_instance_info_cache_interval\","},{"line_number":146,"context_line":"               default\u003d60,"},{"line_number":147,"context_line":"               help\u003d\"Number of seconds between instance info_cache self \""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_0903c842","line":144,"updated":"2014-11-07 12:35:05.000000000","message":"E501 line too long (80 \u003e 79 characters)","commit_id":"b802d241c8cafe119ecfbb71bf730c30054cc3b2"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"09a48e1fec23d581e4c2afac454c37912cb8c428","unresolved":false,"context_lines":[{"line_number":620,"context_line":"        self._resource_tracker_dict \u003d {}"},{"line_number":621,"context_line":"        self.instance_events \u003d InstanceEvents()"},{"line_number":622,"context_line":"        self._sync_power_pool \u003d ("},{"line_number":623,"context_line":"            eventlet.GreenPool(CONF.sync_power_state_concurrency + 1))"},{"line_number":624,"context_line":"        self._syncs_in_progress \u003d {}"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"        super(ComputeManager, self).__init__(service_name\u003d\"compute\","}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_3c646a21","line":623,"updated":"2014-11-12 17:44:15.000000000","message":"Why are you adding 1 to the config parameter value here ? IIUC, this is basically setting the thread count for the greenpool, so I don\u0027t see why we shouldn\u0027t just use the value directly.\n\nAlso, previously we\u0027d default to 1000 threads in the pool, but as declared above there will only be 1 thread in the pool out of the box.\n\nI get that we want to limit parallelism, but going from one extreme right to the other feels wrong. I\u0027d expect we should default to something with some level concurrency. To pick a number out of the hat, 20.","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":935,"name":"James Page","email":"ringo.page@gmail.com","username":"james-page"},"change_message_id":"17429d264f92069827b29fee041094cfd8803225","unresolved":false,"context_lines":[{"line_number":620,"context_line":"        self._resource_tracker_dict \u003d {}"},{"line_number":621,"context_line":"        self.instance_events \u003d InstanceEvents()"},{"line_number":622,"context_line":"        self._sync_power_pool \u003d ("},{"line_number":623,"context_line":"            eventlet.GreenPool(CONF.sync_power_state_concurrency + 1))"},{"line_number":624,"context_line":"        self._syncs_in_progress \u003d {}"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"        super(ComputeManager, self).__init__(service_name\u003d\"compute\","}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_35b71dda","line":623,"in_reply_to":"5a890539_3c646a21","updated":"2014-11-12 19:00:09.000000000","message":"the +1 to accommodate the _sync_manager thread in addition to the threads performing the power sync tasks.\n\nSwitching to something other than 1 is a good idea (I guess I was driven to 1 by a very instance dense cloud I was testing on) - I\u0027ll put that in my next update","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"09a48e1fec23d581e4c2afac454c37912cb8c428","unresolved":false,"context_lines":[{"line_number":5739,"context_line":"                    self._syncs_in_progress[uuid] \u003d True"},{"line_number":5740,"context_line":"                    self._sync_power_pool.spawn_n(_sync, db_instance)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5743,"context_line":""},{"line_number":5744,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5745,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_5f78488f","line":5742,"updated":"2014-11-12 17:44:15.000000000","message":"I\u0027m not sure why you\u0027re introducing this extra spawn. The actual sync work is already being pushed into the thread pool on line 5740. So why do we need the actual for loop itself, to run in another thread too. This feels unrelated to the bug which was about  limiting the size of the thread pool","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"899bd846527031cc5f0dc5d8c4a816b44b3be68e","unresolved":false,"context_lines":[{"line_number":5739,"context_line":"                    self._syncs_in_progress[uuid] \u003d True"},{"line_number":5740,"context_line":"                    self._sync_power_pool.spawn_n(_sync, db_instance)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5743,"context_line":""},{"line_number":5744,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5745,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_7a6dfb97","line":5742,"in_reply_to":"5a890539_15edc1ef","updated":"2014-11-13 12:16:08.000000000","message":"Ok, I understand now - the GreenPool.spawn_n method is blocking when the thread count limit is hit.\n\nI don\u0027t think your code is safe against blocking though, even with this.\n\nIf the number of instances to sync is very large compared to the number of threads in the pool, it may take a non-trivial amount of time before \"_sync_manager\" function completes and its thread exits. If the _sync_power_states periodic job triggers again, before the previous _sync_manager thread has completed, we will not have any free threads. So you can still risk blocking the entire periodic task.\n\nIMHO to properly solve this we should not directly spawn threads for every single instance sync job. Instead we should have an eventlet.Queue object and place all instances to be synced onto the queue, and have each thread process multiple jobs.\n\nie spawn enough threads to ensure there are min(queue length, thread limit)  threads running (taking account of fact some threads may already be running). Each thread should do \"while not queue.empty() ...process a queue entry\" and exit once the queue is finished.","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":935,"name":"James Page","email":"ringo.page@gmail.com","username":"james-page"},"change_message_id":"17429d264f92069827b29fee041094cfd8803225","unresolved":false,"context_lines":[{"line_number":5739,"context_line":"                    self._syncs_in_progress[uuid] \u003d True"},{"line_number":5740,"context_line":"                    self._sync_power_pool.spawn_n(_sync, db_instance)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5743,"context_line":""},{"line_number":5744,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5745,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_15edc1ef","line":5742,"in_reply_to":"5a890539_5f78488f","updated":"2014-11-12 19:00:09.000000000","message":"I\u0027ve maintained the switch to move the power sync out of the periodic task thread (L5723 pre change) so that it does not get blocked; the sync manager feeds sync tasks into the pool, which will block when full.\n\nI probably need to pull the comment down to this line to make that clear.","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":935,"name":"James Page","email":"ringo.page@gmail.com","username":"james-page"},"change_message_id":"6653008436ed5946d6e10204c21f4f8cda777587","unresolved":false,"context_lines":[{"line_number":5739,"context_line":"                    self._syncs_in_progress[uuid] \u003d True"},{"line_number":5740,"context_line":"                    self._sync_power_pool.spawn_n(_sync, db_instance)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5743,"context_line":""},{"line_number":5744,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5745,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_dd8c2938","line":5742,"in_reply_to":"5a890539_7a6dfb97","updated":"2014-11-13 12:49:45.000000000","message":"Ah - good point - I\u0027d not considered that scenario.\n\nMoving to a queue does sound preferable but right now I probably don\u0027t have enough time to refactor to that approach - would spawning the _sync_manager outside of the pool be acceptable for now?\n\nTaking that would avoid the blocking issue, as the _syncs_in_process dict deals with overlapping _sync_power_states periodic tasks already.","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"523d9733f220880bdc3734c18747135d1f325f37","unresolved":false,"context_lines":[{"line_number":5739,"context_line":"                    self._syncs_in_progress[uuid] \u003d True"},{"line_number":5740,"context_line":"                    self._sync_power_pool.spawn_n(_sync, db_instance)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5743,"context_line":""},{"line_number":5744,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5745,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_3647071f","line":5742,"in_reply_to":"5a890539_dd8c2938","updated":"2014-12-05 12:51:03.000000000","message":"I\u0027d rather prefer to see this fixed properly now. Spawning outside the pool is still leaving the bug in place, just obscuring it from view.","commit_id":"11482b8b917f5d7dea741b909ac979093333659e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ec171c25a5f962bb23232a99da8bbccbe6ce63e7","unresolved":false,"context_lines":[{"line_number":5742,"context_line":"        # process syncs asynchronously via the sync manager as we"},{"line_number":5743,"context_line":"        # don\u0027t want instance locking to block the entire periodic"},{"line_number":5744,"context_line":"        # task thread"},{"line_number":5745,"context_line":"        self._sync_power_pool.spawn_n(_sync_manager)"},{"line_number":5746,"context_line":""},{"line_number":5747,"context_line":"    def _query_driver_power_state_and_sync(self, context, db_instance):"},{"line_number":5748,"context_line":"        if db_instance.task_state is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_3ae8d3e1","line":5745,"updated":"2014-11-13 12:17:05.000000000","message":"See my reply to you in patchset 4, for why I think this is not safe.","commit_id":"fee887d13e0e39589aed322b7a873ed366c5af1a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"47c165912b3b563097c1d1b237aff9663ab5de3e","unresolved":false,"context_lines":[{"line_number":5799,"context_line":"                self._sync_power_pool.waitall()"},{"line_number":5800,"context_line":"                if self._sync_power_queue.empty():"},{"line_number":5801,"context_line":"                    self._sync_manager \u003d None"},{"line_number":5802,"context_line":"                    break"},{"line_number":5803,"context_line":""},{"line_number":5804,"context_line":"        for db_instance in db_instances:"},{"line_number":5805,"context_line":"            uuid \u003d db_instance.uuid"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a930d6b_1cb91815","line":5802,"updated":"2015-01-26 14:16:23.000000000","message":"I think you have a race condition here.\n\n 1. Thread B sees  self._sync_power_queue.empty() \u003d\u003d TRUE\n 2. Thread A adds an entry self._sync_power_queue.put(db_instance)\n 3. Thread A sees \u0027not self._sync_manager\u0027 \u003d\u003d False, so assumes _sync_manager is already running\n 4. Thread B exits so _sync_manager is not running\n\nSo the job added to the queue at 2 never gets processed.","commit_id":"ed8e35acea29bd67d83a16fbe6b3ece50b76c2ac"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"87638aa8d7e5027021f5a4f31099e504bce32afa","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                    \u0027the hypervisor. Set to -1 to disable. \u0027"},{"line_number":146,"context_line":"                    \u0027Setting this to 0 will run at the default rate.\u0027),"},{"line_number":147,"context_line":"    cfg.IntOpt(\u0027sync_power_state_concurrency\u0027,"},{"line_number":148,"context_line":"               default\u003d20,"},{"line_number":149,"context_line":"               help\u003d\u0027The level of concurrency that will be used during sync \u0027"},{"line_number":150,"context_line":"                    \u0027of power states between the database and the \u0027"},{"line_number":151,"context_line":"                    \u0027hypervisor.\u0027),"}],"source_content_type":"text/x-python","patch_set":17,"id":"da9b358b_1b1c1711","line":148,"updated":"2015-03-31 17:08:41.000000000","message":"usually there are less than 20 instances on one compute host\njust curious :)","commit_id":"8acedefe1ad26e7346fbffd042fec7bcb95601be"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"7ab4c4b159c7c51b76ea756ed7260d3775821228","unresolved":false,"context_lines":[{"line_number":5972,"context_line":"                self._syncs_in_progress[uuid] \u003d True"},{"line_number":5973,"context_line":"                self._sync_power_queue.put(db_instance)"},{"line_number":5974,"context_line":""},{"line_number":5975,"context_line":"        if not self._sync_manager:"},{"line_number":5976,"context_line":"            # process syncs asynchronously via the sync manager as we"},{"line_number":5977,"context_line":"            # don\u0027t want instance locking to block the entire periodic"},{"line_number":5978,"context_line":"            # task thread"}],"source_content_type":"text/x-python","patch_set":17,"id":"9aa53dc9_8d3d44df","line":5975,"updated":"2015-04-09 23:30:36.000000000","message":"Maybe we should also absorb/revive\n\nhttps://review.openstack.org/#/c/21380/4/glance/common/utils.py\n\nThe \u0027speed_limit_iter\u0027 there could be useful here...","commit_id":"8acedefe1ad26e7346fbffd042fec7bcb95601be"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"1acfec6c7ebfe8b27ae11826e1c5a8debf89f0b2","unresolved":false,"context_lines":[{"line_number":5972,"context_line":"                self._syncs_in_progress[uuid] \u003d True"},{"line_number":5973,"context_line":"                self._sync_power_queue.put(db_instance)"},{"line_number":5974,"context_line":""},{"line_number":5975,"context_line":"        if not self._sync_manager:"},{"line_number":5976,"context_line":"            # process syncs asynchronously via the sync manager as we"},{"line_number":5977,"context_line":"            # don\u0027t want instance locking to block the entire periodic"},{"line_number":5978,"context_line":"            # task thread"}],"source_content_type":"text/x-python","patch_set":17,"id":"9aa53dc9_839875c1","line":5975,"in_reply_to":"9aa53dc9_8d3d44df","updated":"2015-04-10 00:43:36.000000000","message":"Or https://review.openstack.org/#/c/172246/ (which is this moved into oslo.utils).","commit_id":"8acedefe1ad26e7346fbffd042fec7bcb95601be"}]}