)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Since the list of enabled cells are stored as a global cache on the"},{"line_number":13,"context_line":"host_manager, a reset() handler for the SIG-HUP signal has also been"},{"line_number":14,"context_line":"added in the scheduler. Hence upon every create-cell/enable-cell/"},{"line_number":15,"context_line":"disable-cell operation the scheduler has to be signaled so that the"},{"line_number":16,"context_line":"cache is refreshed."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Implements blueprint cell-disable"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"df7087c5_3a9bb4c9","line":16,"range":{"start_line":14,"start_character":24,"end_line":16,"end_character":19},"updated":"2018-03-21 19:38:14.000000000","message":"The nova-manage CLI should be updated as a result, yes?\n\nWe might also want to consider adding something to https://docs.openstack.org/nova/latest/user/cells.html#faqs about disabling cells and what needs to happen if you do so (SIGHUP the nova-scheduler service(s)).\n\nThis section for the scheduler also seems related:\n\nhttps://docs.openstack.org/nova/latest/admin/configuration/schedulers.html?highlight\u003dmaintenance#command-line-interface","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"29330255819cb10b029fe465537985a38d99cd66","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Since the list of enabled cells are stored as a global cache on the"},{"line_number":13,"context_line":"host_manager, a reset() handler for the SIG-HUP signal has also been"},{"line_number":14,"context_line":"added in the scheduler. Hence upon every create-cell/enable-cell/"},{"line_number":15,"context_line":"disable-cell operation the scheduler has to be signaled so that the"},{"line_number":16,"context_line":"cache is refreshed."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Implements blueprint cell-disable"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"df7087c5_7a028cb6","line":16,"range":{"start_line":14,"start_character":24,"end_line":16,"end_character":19},"in_reply_to":"df7087c5_3a9bb4c9","updated":"2018-03-21 19:39:50.000000000","message":"For the latter two, if you ended up documenting something in https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html?highlight\u003dmaintenance#command-line-interface then the cells FAQs page could probably just link to that for details. I\u0027m thinking a FAQs entry could be something like, \"Can I create a cell but have it disabled from scheduling?\" and/or \"How can I disable a cell so new server create requests don\u0027t go it while I perform maintenance?\".","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"599594bd497ff711706fa8097d8599db8fe8d8a3","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Since the list of enabled cells are stored as a global cache on the"},{"line_number":13,"context_line":"host_manager, a reset() handler for the SIG-HUP signal has also been"},{"line_number":14,"context_line":"added in the scheduler. Hence upon every create-cell/enable-cell/"},{"line_number":15,"context_line":"disable-cell operation the scheduler has to be signaled so that the"},{"line_number":16,"context_line":"cache is refreshed."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Implements blueprint cell-disable"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"df7087c5_d5084b2e","line":16,"range":{"start_line":14,"start_character":24,"end_line":16,"end_character":19},"in_reply_to":"df7087c5_7a028cb6","updated":"2018-03-21 20:51:51.000000000","message":"Just thought about this - me pushing for docs is maybe premature at this point? Since we don\u0027t yet have an interface to actually disable a cell, I guess the docs should probably wait (and maybe that\u0027s why you were waiting) until the nova-manage patches are up? If so, I\u0027m OK with that.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Since the list of enabled cells are stored as a global cache on the"},{"line_number":13,"context_line":"host_manager, a reset() handler for the SIG-HUP signal has also been"},{"line_number":14,"context_line":"added in the scheduler. Hence upon every create-cell/enable-cell/"},{"line_number":15,"context_line":"disable-cell operation the scheduler has to be signaled so that the"},{"line_number":16,"context_line":"cache is refreshed."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Implements blueprint cell-disable"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"df7087c5_d064d9e7","line":16,"range":{"start_line":14,"start_character":24,"end_line":16,"end_character":19},"in_reply_to":"df7087c5_d5084b2e","updated":"2018-03-21 21:01:37.000000000","message":"Yes, I will have the nova-manage patches also up and at the end I could change the docs for scheduler interface and cellv2 pages. Since the wordings I put there would be important, I guess its also better for the reviewers to have the whole picture; but nevertheless thank you for telling which which docs need to be updated. I will make a note to do these doc changes as my last patch so that they don\u0027t get buried in the code.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"756fa4e3a86d5653a389f47be9c862c446d79e1d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Surya Seetharaman \u003csuryaseetharaman.9@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-03-23 21:36:51 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow scheduling only to enabled cells (Filter Scheduler)"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This by default would be scheduling to all cells since all cells"},{"line_number":10,"context_line":"would be enabled at the time of creation unless specified otherwise."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"bf659307_4d8739d9","line":7,"updated":"2018-03-23 20:47:33.000000000","message":"Needs a release note,","commit_id":"3c209bf094c10bf149f66719cb18b76bcde9d140"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5cc2f60f69f1a4f4b03ef151e7256056f27b51e2","unresolved":false,"context_lines":[{"line_number":669,"context_line":"            only_cell \u003d spec_obj.requested_destination.cell"},{"line_number":670,"context_line":"            if only_cell not in self.enabled_cells:"},{"line_number":671,"context_line":"                raise Exception(\u0027The requested destination cell with uuid %s \u0027"},{"line_number":672,"context_line":"                                \u0027is currently disabled.\u0027, only_cell.uuid)"},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            only_cell \u003d None"},{"line_number":675,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_5b183774","line":672,"updated":"2018-03-19 14:45:30.000000000","message":"You might actually want to allow that... for things like live-migrate inside a disabled cell. Although I get the logic that you don\u0027t want a user building on a disabled cell by passing in a scheduler filer.","commit_id":"f7f48e74e6da9601fb9fd765c41f4f186dcc1432"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"523369d76d97a33439af0a5707ff6df333423de0","unresolved":false,"context_lines":[{"line_number":669,"context_line":"            only_cell \u003d spec_obj.requested_destination.cell"},{"line_number":670,"context_line":"            if only_cell not in self.enabled_cells:"},{"line_number":671,"context_line":"                raise Exception(\u0027The requested destination cell with uuid %s \u0027"},{"line_number":672,"context_line":"                                \u0027is currently disabled.\u0027, only_cell.uuid)"},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            only_cell \u003d None"},{"line_number":675,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_3b01c3b0","line":672,"in_reply_to":"df7087c5_5b183774","updated":"2018-03-19 14:58:20.000000000","message":"Yes, I am going to allow this, this was just put up as a POC for the spec :) . I have to update this particular patch set.","commit_id":"f7f48e74e6da9601fb9fd765c41f4f186dcc1432"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"83d341570b371b1b207195d2fd70dbd87508f652","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.cells \u003d None"},{"line_number":344,"context_line":"        self.enabled_cells \u003d None"},{"line_number":345,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":346,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":347,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_52899ac7","line":344,"updated":"2018-03-22 14:05:13.000000000","message":"You\u0027re keeping this separate because you see that we use the list of all cells in _init_instance_info(), right?\n\nI think it\u0027s incorrect to refresh just the list of enabled cells here, for a couple of reasons:\n\n1. On startup, we\u0027ll have to make two calls to populate the list of all cells and enabled cells\n2. On HUP, we may find new enabled cells that didn\u0027t exist when we started up and thus the two lists will be out of sync\n\nI think it would be much cleaner to just fetch the full list of cells for self.cells and filter them for self.enabled_cells. One trip to the DB, and consistent lists.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"20a6a91c940bcd51a79266cb795d5aabf52006c9","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.cells \u003d None"},{"line_number":344,"context_line":"        self.enabled_cells \u003d None"},{"line_number":345,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":346,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":347,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_9db42384","line":344,"in_reply_to":"df7087c5_52899ac7","updated":"2018-03-22 14:56:48.000000000","message":"\u003e You\u0027re keeping this separate because you see that we use the list\n \u003e of all cells in _init_instance_info(), right?\n\nCorrect and we are also using the self.cells for the Caching Scheduler in get_all_host_states function (though it will be deprecated soon, code still remains). I did not want to mix the two for these reasons.\n\n \u003e \n \u003e I think it\u0027s incorrect to refresh just the list of enabled cells\n \u003e here, for a couple of reasons:\n \u003e \n \u003e 1. On startup, we\u0027ll have to make two calls to populate the list of\n \u003e all cells and enabled cells\n\nright the load_cells and load_enabled_cells\n\n \u003e 2. On HUP, we may find new enabled cells that didn\u0027t exist when we\n \u003e started up and thus the two lists will be out of sync\n \u003e \n\nWhen you say two lists -\u003e you mean self.cells and self.enabled cells right ? Why do we have to keep the self.cells and self.enabled_cells synced, I mean they are two different things...?\n\n \u003e I think it would be much cleaner to just fetch the full list of\n \u003e cells for self.cells and filter them for self.enabled_cells. One\n \u003e trip to the DB, and consistent lists.\n\nokay,, doable, except I am confused about this context now.\n\nSo are you now talking about loading these two lists upon startup (point1) or are you talking about the refresh at HUP (point2) ?","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"17f678c5916b1b74eac7f89a51095a99a7c9f5f5","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.cells \u003d None"},{"line_number":344,"context_line":"        self.enabled_cells \u003d None"},{"line_number":345,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":346,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":347,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_d8be2964","line":344,"in_reply_to":"df7087c5_9db42384","updated":"2018-03-22 15:15:47.000000000","message":"\u003e When you say two lists -\u003e you mean self.cells and self.enabled\n \u003e cells right ? Why do we have to keep the self.cells and\n \u003e self.enabled_cells synced, I mean they are two different things...?\n\nLet\u0027s say you start up the scheduler, then you create a new cell, and then HUP your scheduler. You expect it to now notice those new cells. That will happen if you\u0027re running FilterScheduler because we refreshed the list that it looks at, but not if you\u0027re running CachingScheduler or the ironic host manager.\n\n \u003e So are you now talking about loading these two lists upon startup\n \u003e (point1) or are you talking about the refresh at HUP (point2) ?\n\nIt should be the same code, no matter where it\u0027s called from. Just do this:\n\n self.cells \u003d objects.CellMappingList.get_all()\n self.enabled_cells \u003d [c for c in self.cells if c.enabled]\n\nIf we\u0027re going to keep two lists, it\u0027s much more efficient to pull from the database once and filter the list to get the enabled set than to make two separate calls just to filter on a column.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b0079641484f92d1331c5f52156a86a75010675d","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.cells \u003d None"},{"line_number":344,"context_line":"        self.enabled_cells \u003d None"},{"line_number":345,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":346,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":347,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_833922d8","line":344,"in_reply_to":"df7087c5_d8be2964","updated":"2018-03-22 15:24:33.000000000","message":"\u003e \u003e When you say two lists -\u003e you mean self.cells and self.enabled\n \u003e \u003e cells right ? Why do we have to keep the self.cells and\n \u003e \u003e self.enabled_cells synced, I mean they are two different\n \u003e things...?\n \u003e \n \u003e Let\u0027s say you start up the scheduler, then you create a new cell,\n \u003e and then HUP your scheduler. You expect it to now notice those new\n \u003e cells. That will happen if you\u0027re running FilterScheduler because\n \u003e we refreshed the list that it looks at, but not if you\u0027re running\n \u003e CachingScheduler or the ironic host manager.\n \u003e \n\nack! (was totally focused on the enable disable cell and forgot about new cells being created)\n\n \u003e \u003e So are you now talking about loading these two lists upon startup\n \u003e \u003e (point1) or are you talking about the refresh at HUP (point2) ?\n \u003e \n \u003e It should be the same code, no matter where it\u0027s called from. Just\n \u003e do this:\n \u003e \n \u003e self.cells \u003d objects.CellMappingList.get_all()\n \u003e self.enabled_cells \u003d [c for c in self.cells if c.enabled]\n \u003e \n \u003e If we\u0027re going to keep two lists, it\u0027s much more efficient to pull\n \u003e from the database once and filter the list to get the enabled set\n \u003e than to make two separate calls just to filter on a column.\n\nright! thanks for clarifying. Will update the refreshing part directly inside the handler and merge both these load cells into one then.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":651,"context_line":"            # to the scheduler."},{"line_number":652,"context_line":"            self.enabled_cells \u003d objects.CellMappingList.get_by_disabled("},{"line_number":653,"context_line":"                context, disabled\u003dFalse)"},{"line_number":654,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":655,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_1aa830d9","line":654,"range":{"start_line":654,"start_character":39,"end_line":654,"end_character":44},"updated":"2018-03-21 19:38:14.000000000","message":"enabled cells","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":651,"context_line":"            # to the scheduler."},{"line_number":652,"context_line":"            self.enabled_cells \u003d objects.CellMappingList.get_by_disabled("},{"line_number":653,"context_line":"                context, disabled\u003dFalse)"},{"line_number":654,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":655,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_309bd5c9","line":654,"range":{"start_line":654,"start_character":39,"end_line":654,"end_character":44},"in_reply_to":"df7087c5_1aa830d9","updated":"2018-03-21 21:01:37.000000000","message":"Done","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":655,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_daa958dd","line":657,"range":{"start_line":657,"start_character":26,"end_line":657,"end_character":30},"updated":"2018-03-21 19:38:14.000000000","message":"identity:\n\nhttps://github.com/openstack/nova/blob/2ec8c49f6cb4a0e7dba217e824c20d9c703d2105/nova/objects/cell_mapping.py#L38","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":655,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_d0a939dd","line":657,"range":{"start_line":657,"start_character":26,"end_line":657,"end_character":30},"in_reply_to":"df7087c5_daa958dd","updated":"2018-03-21 21:01:37.000000000","message":"Done","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":661,"context_line":"        # reset signal handler."},{"line_number":662,"context_line":"        self.enabled_cells \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_1af6f0fa","line":659,"updated":"2018-03-21 19:38:14.000000000","message":"Do we need any kind of lock around this and anything that could be currently using it? It would be bad if we were iterating this thing and then a SIGHUP happens.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c6812b26e7d70d46aff822aa2b322a881ea0db52","unresolved":false,"context_lines":[{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":661,"context_line":"        # reset signal handler."},{"line_number":662,"context_line":"        self.enabled_cells \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_d46cc809","line":659,"in_reply_to":"df7087c5_1044911a","updated":"2018-03-22 01:33:37.000000000","message":"Could ask Dan what he thinks, but it seems we should have a lock around using enabled_cells. The lock object would likely just be a sentinel in this host manager class.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":661,"context_line":"        # reset signal handler."},{"line_number":662,"context_line":"        self.enabled_cells \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_1044911a","line":659,"in_reply_to":"df7087c5_1af6f0fa","updated":"2018-03-21 21:01:37.000000000","message":"Hmm, maybe we could lock this operation (I will look this up on how its done in nova) to be on the safe side and future bug-proof, Maybe if in future the scheduler SIGHUP is needed for some other operation, this might screw the scheduling.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"83d341570b371b1b207195d2fd70dbd87508f652","unresolved":false,"context_lines":[{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":661,"context_line":"        # reset signal handler."},{"line_number":662,"context_line":"        self.enabled_cells \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_f27eaee2","line":659,"in_reply_to":"df7087c5_d46cc809","updated":"2018-03-22 14:05:13.000000000","message":"Technically we don\u0027t need a lock here to protect threads that may be in the middle of looking at this list. There aren\u0027t any yieldable statements between checking that the list is populated and using it. However, that could change later, which would mean we\u0027d check and see that the list is populated (below, L667), find it to be okay, and then use it on L678 and find it to be None, if the signal handler happened sneak in between those two lines.\n\nHowever, unlike raw C signal handlers, we can do stuff in our handlers without worrying[0], so what I would do is refresh the list in the handler itself. That way any existing threads running will continue to use the old list until the new one is available. Setting self._enabled_cells once we have it is atomic and should be safe.\n\n[0]: https://github.com/openstack/oslo.service/blob/master/oslo_service/service.py#L175","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"20a6a91c940bcd51a79266cb795d5aabf52006c9","unresolved":false,"context_lines":[{"line_number":656,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":657,"context_line":"                       [c.uuid for c in self.enabled_cells])})"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def flush_enabled_cells_cache(self):"},{"line_number":660,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":661,"context_line":"        # reset signal handler."},{"line_number":662,"context_line":"        self.enabled_cells \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_f8a0cdf6","line":659,"in_reply_to":"df7087c5_f27eaee2","updated":"2018-03-22 14:56:48.000000000","message":"\u003e Technically we don\u0027t need a lock here to protect threads that may\n \u003e be in the middle of looking at this list. There aren\u0027t any\n \u003e yieldable statements between checking that the list is populated\n \u003e and using it. However, that could change later, which would mean\n \u003e we\u0027d check and see that the list is populated (below, L667), find\n \u003e it to be okay, and then use it on L678 and find it to be None, if\n \u003e the signal handler happened sneak in between those two lines.\n\nUnderstood.\n\n \u003e \n \u003e However, unlike raw C signal handlers, we can do stuff in our\n \u003e handlers without worrying[0], so what I would do is refresh the\n \u003e list in the handler itself.\n\nokay!\n\n \u003e That way any existing threads running\n \u003e will continue to use the old list until the new one is available.\n \u003e Setting self._enabled_cells once we have it is atomic and should be\n \u003e safe.\n \u003e \n \u003e [0]: https://github.com/openstack/oslo.service/blob/master/oslo_service/service.py#L175\n\ncool.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360407f80d03ee99a35a79f8fb709f75403592a9","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":"    def _load_cells(self, context):"},{"line_number":638,"context_line":"        if not self.cells:"},{"line_number":639,"context_line":"            # NOTE(danms, tssurya): global list of cells cached forever which"},{"line_number":640,"context_line":"            # will be refreshed every time a SIG-HUP is sent to the scheduler."},{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_61a5e146","line":639,"range":{"start_line":639,"start_character":64,"end_line":639,"end_character":71},"updated":"2018-03-22 20:15:17.000000000","message":"Heh, I said \"cached forever\" because we never refreshed them. Now that you\u0027re refreshing them you could just clean this up :)","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8c0ffe144ad6f3bc9a175980b317ada9e377c85d","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":"    def _load_cells(self, context):"},{"line_number":638,"context_line":"        if not self.cells:"},{"line_number":639,"context_line":"            # NOTE(danms, tssurya): global list of cells cached forever which"},{"line_number":640,"context_line":"            # will be refreshed every time a SIG-HUP is sent to the scheduler."},{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_b731fb5b","line":639,"range":{"start_line":639,"start_character":64,"end_line":639,"end_character":71},"in_reply_to":"df7087c5_61a5e146","updated":"2018-03-23 13:16:00.000000000","message":"Done","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360407f80d03ee99a35a79f8fb709f75403592a9","unresolved":false,"context_lines":[{"line_number":637,"context_line":"    def _load_cells(self, context):"},{"line_number":638,"context_line":"        if not self.cells:"},{"line_number":639,"context_line":"            # NOTE(danms, tssurya): global list of cells cached forever which"},{"line_number":640,"context_line":"            # will be refreshed every time a SIG-HUP is sent to the scheduler."},{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                      {\u0027count\u0027: len(self.cells),"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_c1694df8","line":640,"range":{"start_line":640,"start_character":45,"end_line":640,"end_character":52},"updated":"2018-03-22 20:15:17.000000000","message":"SIGHUP","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8c0ffe144ad6f3bc9a175980b317ada9e377c85d","unresolved":false,"context_lines":[{"line_number":637,"context_line":"    def _load_cells(self, context):"},{"line_number":638,"context_line":"        if not self.cells:"},{"line_number":639,"context_line":"            # NOTE(danms, tssurya): global list of cells cached forever which"},{"line_number":640,"context_line":"            # will be refreshed every time a SIG-HUP is sent to the scheduler."},{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                      {\u0027count\u0027: len(self.cells),"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_773fe324","line":640,"range":{"start_line":640,"start_character":45,"end_line":640,"end_character":52},"in_reply_to":"df7087c5_c1694df8","updated":"2018-03-23 13:16:00.000000000","message":"Done","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360407f80d03ee99a35a79f8fb709f75403592a9","unresolved":false,"context_lines":[{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                      {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join([c.identity for c in self.cells])})"},{"line_number":645,"context_line":"        if not self.enabled_cells:"},{"line_number":646,"context_line":"            # NOTE(tssurya): Global cache of only the enabled cells. This way"},{"line_number":647,"context_line":"            # scheduling is limited only to the enabled cells. However this"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_a15d3948","line":644,"range":{"start_line":644,"start_character":45,"end_line":644,"end_character":53},"updated":"2018-03-22 20:15:17.000000000","message":"This is a good change, but normally I\u0027d try to get someone to put something cleanupy like that in a separate patch so that we don\u0027t see that line as changed if we\u0027re ever bisecting an obscure bug. Keeping functional/important changes distinct from \"gardening\" maintenance patches helps make history a lot clearer in the long run.\n\nThere are probably other places where we log cell.uuid that we should be using identity, but never cleaned up (like this one) after adding the identity thing. One patch to fix all those would be cool.","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8c0ffe144ad6f3bc9a175980b317ada9e377c85d","unresolved":false,"context_lines":[{"line_number":641,"context_line":"            self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"            LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                      {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join([c.identity for c in self.cells])})"},{"line_number":645,"context_line":"        if not self.enabled_cells:"},{"line_number":646,"context_line":"            # NOTE(tssurya): Global cache of only the enabled cells. This way"},{"line_number":647,"context_line":"            # scheduling is limited only to the enabled cells. However this"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_973cf72f","line":644,"range":{"start_line":644,"start_character":45,"end_line":644,"end_character":53},"in_reply_to":"df7087c5_a15d3948","updated":"2018-03-23 13:16:00.000000000","message":"Okay, I will put this back as it was :)","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fc00d4a964808c1c79454f5da332a349a083f1a9","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            # or when a new cell is created as long as a SIGHUP signal is sent"},{"line_number":650,"context_line":"            # to the scheduler."},{"line_number":651,"context_line":"            self.enabled_cells \u003d [c for c in self.cells if not c.disabled]"},{"line_number":652,"context_line":"            LOG.debug(\u0027Found %(count)i enabled cells: %(cells)s\u0027,"},{"line_number":653,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":654,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":655,"context_line":"                       [c.identity for c in self.enabled_cells])})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf659307_bba89377","line":652,"updated":"2018-03-23 16:43:35.000000000","message":"I also kinda think that two rapid-fire log messages each time we do this is too much. I think just emitting the count of total and disabled cells is enough. Also, CERN has like 80 cells, right? That\u0027s a long debug message to include the .identity of all of them. Do you see that as being important? Maybe we just emit the identity of disabled ones or something?","commit_id":"c6255a1c2ce10be0ee37771dd289fc0d15b2d2a6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"cbbbed4cc2e1de1dd05804a43fdf29d46ba1e10c","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            # or when a new cell is created as long as a SIGHUP signal is sent"},{"line_number":650,"context_line":"            # to the scheduler."},{"line_number":651,"context_line":"            self.enabled_cells \u003d [c for c in self.cells if not c.disabled]"},{"line_number":652,"context_line":"            LOG.debug(\u0027Found %(count)i enabled cells: %(cells)s\u0027,"},{"line_number":653,"context_line":"                      {\u0027count\u0027: len(self.enabled_cells),"},{"line_number":654,"context_line":"                       \u0027cells\u0027: \u0027, \u0027.join("},{"line_number":655,"context_line":"                       [c.identity for c in self.enabled_cells])})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf659307_67949ce9","line":652,"in_reply_to":"bf659307_bba89377","updated":"2018-03-23 19:29:14.000000000","message":"Yes, emitting the identity of the disabled one seems better,","commit_id":"c6255a1c2ce10be0ee37771dd289fc0d15b2d2a6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fc00d4a964808c1c79454f5da332a349a083f1a9","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":659,"context_line":"        # reset signal handler."},{"line_number":660,"context_line":"        self.cells \u003d None"},{"line_number":661,"context_line":"        self.enabled_cells \u003d None"},{"line_number":662,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":663,"context_line":"        self._load_cells(context)"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf659307_fb936ba2","line":661,"updated":"2018-03-23 16:43:35.000000000","message":"As soon as you do this, another scheduler thread will come along and try to load the cells list when it hits its _load_cells() call. And you will also trigger a load_cells on L663. Both will run simultaneously and both will end up writing the new set of cells. All scheduler threads doing actual work will block on waiting for the new set of cells to come along.\n\nJust to short-circuit this cycle a bit, here is what I am proposing:\n\n http://termbin.com/af01","commit_id":"c6255a1c2ce10be0ee37771dd289fc0d15b2d2a6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"cbbbed4cc2e1de1dd05804a43fdf29d46ba1e10c","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        # NOTE(tssurya): This function is called from the scheduler manager\u0027s"},{"line_number":659,"context_line":"        # reset signal handler."},{"line_number":660,"context_line":"        self.cells \u003d None"},{"line_number":661,"context_line":"        self.enabled_cells \u003d None"},{"line_number":662,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":663,"context_line":"        self._load_cells(context)"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf659307_e7b0ac8e","line":661,"in_reply_to":"bf659307_fb936ba2","updated":"2018-03-23 19:29:14.000000000","message":"Thanks for the patient reviewing, I will incorporate it and modify the patch.","commit_id":"c6255a1c2ce10be0ee37771dd289fc0d15b2d2a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe93c36cd1c78caabfb87c1b880f88869be12128","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        return HostState(host, node, cell)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.refresh_cells_caches()"},{"line_number":344,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":345,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":346,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_6c1f756c","line":343,"updated":"2018-03-26 15:47:40.000000000","message":"This is a bit of a new behavior isn\u0027t it? Before this, I could start my nova-scheduler service, and then setup my cell mappings. Then on the first request to the scheduler, it would load up the cells cache. But now, I have to be sure that my cell mappings are created *before* starting nova-scheduler, else I have to SIGHUP nova-scheduler to reload the cache, right?","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1220ba3a4e64331928b71bdb52823160263f5c38","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        return HostState(host, node, cell)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def __init__(self):"},{"line_number":343,"context_line":"        self.refresh_cells_caches()"},{"line_number":344,"context_line":"        self.filter_handler \u003d filters.HostFilterHandler()"},{"line_number":345,"context_line":"        filter_classes \u003d self.filter_handler.get_matching_classes("},{"line_number":346,"context_line":"                CONF.filter_scheduler.available_filters)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_4fa143a9","line":343,"in_reply_to":"bf659307_6c1f756c","updated":"2018-03-26 15:55:55.000000000","message":"Yes...","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe93c36cd1c78caabfb87c1b880f88869be12128","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_2f272fb6","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"updated":"2018-03-26 15:47:40.000000000","message":"Per my concern about startup order above, if we get no cells back here, or at least no non-cell0 cells here, could we not set the cache and then refresh in get_host_states_by_uuids if self.cells is empty (or some other kind of flag)?\n\nDoing so might require a separate change (before this one in the series) that we filter out cell0 from this list, since we\u0027ll never schedule to cell0 anyway since it shouldn\u0027t have computes in it - that\u0027s something I\u0027ve thought we should do for awhile now as an optimization.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8fdfce541ff1722d55da72d07ef27a3e65d77e84","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_59700aa6","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_16f109ce","updated":"2018-03-27 16:45:00.000000000","message":"I would rather not overthink the beginning-of-time situation here. If you start the scheduler before any of the config required to do anything, a SIGHUP or restart will fix it.\n\nThe previous behavior we had, which loaded cells on demand suffered from the problem of all threads storming the castle to update that cache, which we bring back in part if we don\u0027t set self.cells here. It\u0027s just not important enough for me to care, IMHO.\n\nA warning to make it clear we found no cells though is good.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1220ba3a4e64331928b71bdb52823160263f5c38","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_af733f15","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_2f272fb6","updated":"2018-03-26 15:55:55.000000000","message":"\u003e Per my concern about startup order above, if we get no cells back\n \u003e here, or at least no non-cell0 cells here, could we not set the\n \u003e cache and then refresh in get_host_states_by_uuids if self.cells is\n \u003e empty (or some other kind of flag)?\n \u003e \n\nThis is doable, setting self.cells to None if non-cell0 cells are not found, let\u0027s see what Dan says.\n\n \u003e Doing so might require a separate change (before this one in the\n \u003e series) that we filter out cell0 from this list, since we\u0027ll never\n \u003e schedule to cell0 anyway since it shouldn\u0027t have computes in it -\n \u003e that\u0027s something I\u0027ve thought we should do for awhile now as an\n \u003e optimization.\n\nWell can\u0027t I just do this in this same patch ?","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1dcf1ad57faadcbb4a796a11659ace4494dddbb9","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_63e45bad","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_3a9bebc9","updated":"2018-03-26 20:37:12.000000000","message":"\u003e \u003e\n \u003e \u003e Well can\u0027t I just do this in this same patch ?\n \u003e \n \u003e Ah, backporting the optimization.. got it.\n\nThat and it just makes the patches smaller and therefore easier to review on their own.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"775b6e34b6f0676d3fe8625d10261343ae440068","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_6d92d6a0","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_59700aa6","updated":"2018-03-28 08:44:42.000000000","message":"Ack, I will update the patch with a warning and state this situation explicitly in the documentation as well.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1dcf1ad57faadcbb4a796a11659ace4494dddbb9","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_a3035342","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_804f28bc","updated":"2018-03-26 20:37:12.000000000","message":"\u003e On second thoughts could calling refresh_cells_caches() from within a particular scheduler thread bring back the race condition of scheduler/handler threads running refresh at the same time ?\n\nYes, maybe.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"f7b52ac697c4cdb8de941b1638509889bbd83fe8","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_16f109ce","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_a3035342","updated":"2018-03-27 10:57:48.000000000","message":"So is it fine if we just print a debug message saying no cells were found, and anyways we add in the documentation that every new cell_mapping added needs a SIGHUP/startup for the new changes to be reflected, so this falls under the same category right ? If they create cell_mappings after the service is ON, they have to restart to get the changes into the system, just like we have for update_cell command updating db connection or similar. What do you feel?","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"0d9616034785ea19b08e3f053fa2dd6fe4851bf3","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_3a9bebc9","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_af733f15","updated":"2018-03-26 16:07:20.000000000","message":"\u003e \n \u003e Well can\u0027t I just do this in this same patch ?\n\nAh, backporting the optimization.. got it.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"72de9eda2c683a5c158174dde1800156079a879c","unresolved":false,"context_lines":[{"line_number":638,"context_line":"        context \u003d context_module.RequestContext()"},{"line_number":639,"context_line":"        # NOTE(danms, tssurya): global list of cells cached which"},{"line_number":640,"context_line":"        # will be refreshed every time a SIGHUP is sent to the scheduler."},{"line_number":641,"context_line":"        self.cells \u003d objects.CellMappingList.get_all(context)"},{"line_number":642,"context_line":"        LOG.debug(\u0027Found %(count)i cells: %(cells)s\u0027,"},{"line_number":643,"context_line":"                  {\u0027count\u0027: len(self.cells),"},{"line_number":644,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join([c.uuid for c in self.cells])})"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_804f28bc","line":641,"range":{"start_line":641,"start_character":8,"end_line":641,"end_character":61},"in_reply_to":"bf659307_af733f15","updated":"2018-03-26 17:38:23.000000000","message":"\u003e \u003e Per my concern about startup order above, if we get no cells back\n \u003e \u003e here, or at least no non-cell0 cells here, could we not set the\n \u003e \u003e cache and then refresh in get_host_states_by_uuids if self.cells\n \u003e is\n \u003e \u003e empty (or some other kind of flag)?\n \u003e \u003e\n \u003e \n \u003e This is doable, setting self.cells to None if non-cell0 cells are\n \u003e not found, let\u0027s see what Dan says.\n \u003e \n\nOn second thoughts could calling refresh_cells_caches() from within a particular scheduler thread bring back the race condition of scheduler/handler threads running refresh at the same time ?","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec189403cda412f737eec43fba727958e7fd39fb","unresolved":false,"context_lines":[{"line_number":657,"context_line":"        # to the scheduler."},{"line_number":658,"context_line":"        self.enabled_cells \u003d [c for c in self.cells if not c.disabled]"},{"line_number":659,"context_line":"        # Filtering the disabled cells only for logging purposes."},{"line_number":660,"context_line":"        self.disabled_cells \u003d [c for c in self.cells if c.disabled]"},{"line_number":661,"context_line":"        LOG.debug(\u0027Found %(count)i disabled cells: %(cells)s\u0027,"},{"line_number":662,"context_line":"                  {\u0027count\u0027: len(self.disabled_cells),"},{"line_number":663,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join("}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_88607103","line":660,"range":{"start_line":660,"start_character":8,"end_line":660,"end_character":27},"updated":"2018-03-28 13:39:08.000000000","message":"We don\u0027t need to store this as a class member variable if it\u0027s just being used for logging in this method, right?","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ba99f803eaf8f5576104a9f620434da7e7cb35c7","unresolved":false,"context_lines":[{"line_number":657,"context_line":"        # to the scheduler."},{"line_number":658,"context_line":"        self.enabled_cells \u003d [c for c in self.cells if not c.disabled]"},{"line_number":659,"context_line":"        # Filtering the disabled cells only for logging purposes."},{"line_number":660,"context_line":"        self.disabled_cells \u003d [c for c in self.cells if c.disabled]"},{"line_number":661,"context_line":"        LOG.debug(\u0027Found %(count)i disabled cells: %(cells)s\u0027,"},{"line_number":662,"context_line":"                  {\u0027count\u0027: len(self.disabled_cells),"},{"line_number":663,"context_line":"                   \u0027cells\u0027: \u0027, \u0027.join("}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_4808192b","line":660,"range":{"start_line":660,"start_character":8,"end_line":660,"end_character":27},"in_reply_to":"bf659307_88607103","updated":"2018-03-28 13:42:41.000000000","message":"yes, you are right. will change this to a normal variable.","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"38ecde52930f6c960f65ef9e0fc750c760afda1b","unresolved":false,"context_lines":[{"line_number":666,"context_line":"    def get_host_states_by_uuids(self, context, compute_uuids, spec_obj):"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"        if not self.cells:"},{"line_number":669,"context_line":"            LOG.debug(\"No cells were found\")"},{"line_number":670,"context_line":"        if (spec_obj and \u0027requested_destination\u0027 in spec_obj and"},{"line_number":671,"context_line":"                spec_obj.requested_destination and"},{"line_number":672,"context_line":"                \u0027cell\u0027 in spec_obj.requested_destination):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_28e125d1","line":669,"range":{"start_line":669,"start_character":16,"end_line":669,"end_character":21},"updated":"2018-03-28 13:33:04.000000000","message":"I thought you were going to make this a warning?\n\n\u003e Surya Seetharaman 3:44 AM\n\u003e Ack, I will update the patch with a warning and state this situation explicitly in the documentation as well.","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ba99f803eaf8f5576104a9f620434da7e7cb35c7","unresolved":false,"context_lines":[{"line_number":666,"context_line":"    def get_host_states_by_uuids(self, context, compute_uuids, spec_obj):"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"        if not self.cells:"},{"line_number":669,"context_line":"            LOG.debug(\"No cells were found\")"},{"line_number":670,"context_line":"        if (spec_obj and \u0027requested_destination\u0027 in spec_obj and"},{"line_number":671,"context_line":"                spec_obj.requested_destination and"},{"line_number":672,"context_line":"                \u0027cell\u0027 in spec_obj.requested_destination):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_c3096a2c","line":669,"range":{"start_line":669,"start_character":16,"end_line":669,"end_character":21},"in_reply_to":"bf659307_28e125d1","updated":"2018-03-28 13:42:41.000000000","message":"dammmmit, mybad! will fix this","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"}],"nova/scheduler/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"83d341570b371b1b207195d2fd70dbd87508f652","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        # enabled cells cache in the host manager. So every time an existing"},{"line_number":87,"context_line":"        # cell is disabled or enabled or a new cell is created, a SIGHUP signal"},{"line_number":88,"context_line":"        # has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the Enabled Cells cache.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.flush_enabled_cells_cache()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_926972f9","line":89,"range":{"start_line":89,"start_character":34,"end_line":89,"end_character":47},"updated":"2018-03-22 14:05:13.000000000","message":"nit: \"enabled cells\"","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"20a6a91c940bcd51a79266cb795d5aabf52006c9","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        # enabled cells cache in the host manager. So every time an existing"},{"line_number":87,"context_line":"        # cell is disabled or enabled or a new cell is created, a SIGHUP signal"},{"line_number":88,"context_line":"        # has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the Enabled Cells cache.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.flush_enabled_cells_cache()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_9d6983f9","line":89,"range":{"start_line":89,"start_character":34,"end_line":89,"end_character":47},"in_reply_to":"df7087c5_926972f9","updated":"2018-03-22 14:56:48.000000000","message":"Done","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360407f80d03ee99a35a79f8fb709f75403592a9","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # SIGHUP signal has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.cells \u003d None"},{"line_number":91,"context_line":"        self.driver.host_manager.enabled_cells \u003d None"},{"line_number":92,"context_line":"        LOG.debug(\"The cells and enabled_cells caches have been refreshed.\")"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_41045d18","line":91,"updated":"2018-03-22 20:15:17.000000000","message":"FWIW, I think you should have kept the method that did this. Even though they\u0027re not obscured, those are kinda internals of the host manager and thus delegating to a flush method is a nice way of keeping it that way.","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8c0ffe144ad6f3bc9a175980b317ada9e377c85d","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # SIGHUP signal has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.cells \u003d None"},{"line_number":91,"context_line":"        self.driver.host_manager.enabled_cells \u003d None"},{"line_number":92,"context_line":"        LOG.debug(\"The cells and enabled_cells caches have been refreshed.\")"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_971f1707","line":91,"in_reply_to":"df7087c5_41045d18","updated":"2018-03-23 13:16:00.000000000","message":"Yup, will revert this to the old way. Totally misunderstood your wordings of \"do the refresh inside the handler\" earlier on; my bad.","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360407f80d03ee99a35a79f8fb709f75403592a9","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.cells \u003d None"},{"line_number":91,"context_line":"        self.driver.host_manager.enabled_cells \u003d None"},{"line_number":92,"context_line":"        LOG.debug(\"The cells and enabled_cells caches have been refreshed.\")"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":95,"context_line":"    def select_destinations(self, ctxt, request_spec\u003dNone,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_fcff58fe","line":92,"updated":"2018-03-22 20:15:17.000000000","message":"This isn\u0027t actually true, you\u0027ve just dumped the cache, and it will just log about a millisecond after the one on L89.\n\nNote that this isn\u0027t what I was suggesting. Here, you\u0027re invalidating the cache, which will cause the next scheduler thread that wants to look at cells to have to block on doing the refresh, and it opens you up to the potential race that Matt was pointing out earlier.\n\nInstead, if you just call a refresh method on host manager, and do the actual refreshing there, then you\u0027ll allow existing threads to continue doing their jobs with the cached cell list until you have received the new results from the database, after which time you\u0027ll do swap of the new info into the cache and be good.\n\nWhat you have here ensures that we don\u0027t use any stale cached data after the signal is received (and the handler thread is scheduled), but it will impact the throughput of the scheduler, and multiple threads in the scheduler may try to refresh the cells list at the same time.","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8c0ffe144ad6f3bc9a175980b317ada9e377c85d","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.cells \u003d None"},{"line_number":91,"context_line":"        self.driver.host_manager.enabled_cells \u003d None"},{"line_number":92,"context_line":"        LOG.debug(\"The cells and enabled_cells caches have been refreshed.\")"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"},{"line_number":95,"context_line":"    def select_destinations(self, ctxt, request_spec\u003dNone,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_b79b3b15","line":92,"in_reply_to":"df7087c5_fcff58fe","updated":"2018-03-23 13:16:00.000000000","message":"\u003e This isn\u0027t actually true, you\u0027ve just dumped the cache, and it will\n \u003e just log about a millisecond after the one on L89.\n \u003e \n\nWill remove this update.\n\n \u003e Note that this isn\u0027t what I was suggesting. Here, you\u0027re\n \u003e invalidating the cache, which will cause the next scheduler thread\n \u003e that wants to look at cells to have to block on doing the refresh,\n \u003e and it opens you up to the potential race that Matt was pointing\n \u003e out earlier.\n \u003e \n\nI see your point,..\n\n \u003e Instead, if you just call a refresh method on host manager, and do\n \u003e the actual refreshing there, then you\u0027ll allow existing threads to\n \u003e continue doing their jobs with the cached cell list until you have\n \u003e received the new results from the database, after which time you\u0027ll\n \u003e do swap of the new info into the cache and be good.\n\nOh I actually went and read my OS text books last night.\n\nnote to self: As Dan said, if I do the refresh inside my handler, then until this handler thread fetches the list from the DB and creates a new memory for the cache, the existing scheduler threads can now use the old memory cache, note that python threading is super safe.\n\n \u003e What you have here ensures that we don\u0027t use any stale cached data\n \u003e after the signal is received (and the handler thread is scheduled),\n\nyes, exactly,\n\n \u003e but it will impact the throughput of the scheduler, and multiple\n \u003e threads in the scheduler may try to refresh the cells list at the\n \u003e same time.\n\nUh-Oh,, got it!","commit_id":"59cc6fda54abb840e6c534257848ac0e1e6b582c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"02770a6793cdbeb435aefd2e2af77adb6071e06c","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        # and enabled cells caches in the host manager. So every time an"},{"line_number":87,"context_line":"        # existing cell is disabled or enabled or a new cell is created, a"},{"line_number":88,"context_line":"        # SIGHUP signal has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.refresh_cells_caches()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_c6f8e0c5","line":89,"updated":"2018-03-26 14:19:08.000000000","message":"This is a nit, but SIGHUP will emit three log messages, which I think is excessive. That\u0027s \"Refreshing..\", \"Found %i enabled...\", \"Found %i disabled...\".\n\nIt\u0027s only on SIGHUP so not a huge deal, but I just feel compelled to say something :)","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"933b5f39f2cf9d576276b6e725f90dfbbb8f2def","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        # and enabled cells caches in the host manager. So every time an"},{"line_number":87,"context_line":"        # existing cell is disabled or enabled or a new cell is created, a"},{"line_number":88,"context_line":"        # SIGHUP signal has to be sent to the scheduler for proper scheduling."},{"line_number":89,"context_line":"        LOG.debug(\u0027Refreshing the cells and enabled_cells caches.\u0027)"},{"line_number":90,"context_line":"        self.driver.host_manager.refresh_cells_caches()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @messaging.expected_exceptions(exception.NoValidHost)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_468a901b","line":89,"in_reply_to":"bf659307_c6f8e0c5","updated":"2018-03-26 14:23:21.000000000","message":"Hehe, well I guess the Found %i all_cells and Found %i disabled cells are info plenty ? I can remove this \"Refreshing..\" debug from the reset function.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"}],"nova/tests/unit/scheduler/test_host_manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    def test_flush_enabled_cells_cache(self):"},{"line_number":82,"context_line":"        context \u003d nova_context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":83,"context_line":"        self.host_manager._load_enabled_cells(context)"},{"line_number":84,"context_line":"        self.assertEqual(len(self.host_manager.enabled_cells), 1)"},{"line_number":85,"context_line":"        self.host_manager.flush_enabled_cells_cache()"},{"line_number":86,"context_line":"        self.assertIsNone(self.host_manager.enabled_cells)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_ba4a6424","line":84,"range":{"start_line":84,"start_character":25,"end_line":84,"end_character":64},"updated":"2018-03-21 19:38:14.000000000","message":"swap the args here as assertEqual is (expected, actual), it only matters if the assertion fails, the MismatchError message is based on this order.","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    def test_flush_enabled_cells_cache(self):"},{"line_number":82,"context_line":"        context \u003d nova_context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":83,"context_line":"        self.host_manager._load_enabled_cells(context)"},{"line_number":84,"context_line":"        self.assertEqual(len(self.host_manager.enabled_cells), 1)"},{"line_number":85,"context_line":"        self.host_manager.flush_enabled_cells_cache()"},{"line_number":86,"context_line":"        self.assertIsNone(self.host_manager.enabled_cells)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_904f4133","line":84,"range":{"start_line":84,"start_character":25,"end_line":84,"end_character":64},"in_reply_to":"df7087c5_ba4a6424","updated":"2018-03-21 21:01:37.000000000","message":"Done","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2efb5abe5ede5dc73320f56b1a2aa005a3ac3e13","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        mock_ecm.return_value \u003d cells"},{"line_number":98,"context_line":"        self.host_manager._load_enabled_cells(ctxt)"},{"line_number":99,"context_line":"        mock_ecm.assert_called_once()"},{"line_number":100,"context_line":"        self.assertEqual(len(self.host_manager.enabled_cells), 1)"},{"line_number":101,"context_line":"        self.assertEqual(self.host_manager.enabled_cells[0].uuid, cell_uuid1)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    @mock.patch.object(nova.objects.InstanceList, \u0027get_by_filters\u0027)"},{"line_number":104,"context_line":"    @mock.patch.object(nova.objects.ComputeNodeList, \u0027get_all\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_7a54ccca","line":101,"range":{"start_line":100,"start_character":8,"end_line":101,"end_character":77},"updated":"2018-03-21 19:38:14.000000000","message":"same as above","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"435beca50f32377d2687291eccb1c10b15a33bf8","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        mock_ecm.return_value \u003d cells"},{"line_number":98,"context_line":"        self.host_manager._load_enabled_cells(ctxt)"},{"line_number":99,"context_line":"        mock_ecm.assert_called_once()"},{"line_number":100,"context_line":"        self.assertEqual(len(self.host_manager.enabled_cells), 1)"},{"line_number":101,"context_line":"        self.assertEqual(self.host_manager.enabled_cells[0].uuid, cell_uuid1)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    @mock.patch.object(nova.objects.InstanceList, \u0027get_by_filters\u0027)"},{"line_number":104,"context_line":"    @mock.patch.object(nova.objects.ComputeNodeList, \u0027get_all\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df7087c5_3060b5a5","line":101,"range":{"start_line":100,"start_character":8,"end_line":101,"end_character":77},"in_reply_to":"df7087c5_7a54ccca","updated":"2018-03-21 21:01:37.000000000","message":"Done","commit_id":"934070f824143be72e7857237e9e0a613ac12177"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe93c36cd1c78caabfb87c1b880f88869be12128","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_refresh_cells_caches(self):"},{"line_number":82,"context_line":"        ctxt \u003d nova_context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":83,"context_line":"        self.assertEqual(1, len(self.host_manager.enabled_cells))"},{"line_number":84,"context_line":"        self.assertEqual(1, len(self.host_manager.cells))"},{"line_number":85,"context_line":"        cell_uuid1 \u003d uuids.cell1"},{"line_number":86,"context_line":"        cell_mapping1 \u003d objects.CellMapping(context\u003dctxt,"},{"line_number":87,"context_line":"                                            uuid\u003dcell_uuid1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_cfaeb3e6","line":84,"range":{"start_line":83,"start_character":8,"end_line":84,"end_character":57},"updated":"2018-03-26 15:47:40.000000000","message":"OK these are here because they are loaded on startup and the base test case creates a single cell using SingleCellSimple. Would be nice to have a comment about that.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1220ba3a4e64331928b71bdb52823160263f5c38","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_refresh_cells_caches(self):"},{"line_number":82,"context_line":"        ctxt \u003d nova_context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":83,"context_line":"        self.assertEqual(1, len(self.host_manager.enabled_cells))"},{"line_number":84,"context_line":"        self.assertEqual(1, len(self.host_manager.cells))"},{"line_number":85,"context_line":"        cell_uuid1 \u003d uuids.cell1"},{"line_number":86,"context_line":"        cell_mapping1 \u003d objects.CellMapping(context\u003dctxt,"},{"line_number":87,"context_line":"                                            uuid\u003dcell_uuid1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_2f896f22","line":84,"range":{"start_line":83,"start_character":8,"end_line":84,"end_character":57},"in_reply_to":"bf659307_cfaeb3e6","updated":"2018-03-26 15:55:55.000000000","message":"Done","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe93c36cd1c78caabfb87c1b880f88869be12128","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                                            database_connection\u003d\u0027fake:///db3\u0027,"},{"line_number":101,"context_line":"                                            transport_url\u003d\u0027fake:///mq3\u0027,"},{"line_number":102,"context_line":"                                            disabled\u003dFalse)"},{"line_number":103,"context_line":"        cells \u003d [cell_mapping1, cell_mapping2, cell_mapping3]"},{"line_number":104,"context_line":"        with mock.patch(\u0027nova.objects.CellMappingList.get_all\u0027,"},{"line_number":105,"context_line":"                        return_value\u003dcells) as mock_cm:"},{"line_number":106,"context_line":"            self.host_manager.refresh_cells_caches()"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_ef705775","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":61},"updated":"2018-03-26 15:47:40.000000000","message":"nit: this kind of ignores the fact that you already had a single cell because of the SingleCellSimple usage in the base test class. But this is what we get with mocks in unit tests.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1220ba3a4e64331928b71bdb52823160263f5c38","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                                            database_connection\u003d\u0027fake:///db3\u0027,"},{"line_number":101,"context_line":"                                            transport_url\u003d\u0027fake:///mq3\u0027,"},{"line_number":102,"context_line":"                                            disabled\u003dFalse)"},{"line_number":103,"context_line":"        cells \u003d [cell_mapping1, cell_mapping2, cell_mapping3]"},{"line_number":104,"context_line":"        with mock.patch(\u0027nova.objects.CellMappingList.get_all\u0027,"},{"line_number":105,"context_line":"                        return_value\u003dcells) as mock_cm:"},{"line_number":106,"context_line":"            self.host_manager.refresh_cells_caches()"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_6fe207d8","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":61},"in_reply_to":"bf659307_ef705775","updated":"2018-03-26 15:55:55.000000000","message":"Yea, I will add it in the comments to make it clear that it was intentional.","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"}],"releasenotes/notes/scheduler-startup-behaviour-change-cd8a5e230f56f758.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6c52e750f655f9d6226f8739c199ba63020558a4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    If the scheduler service is started before the cell mappings are created or"},{"line_number":5,"context_line":"    setup, nova-scheduler needs to be restarted or SIGHUP-ed for the newly"}],"source_content_type":"text/x-yaml","patch_set":17,"id":"bf659307_626baab6","line":2,"updated":"2018-03-28 22:13:56.000000000","message":"See my note in the last patch set, let\u0027s just merge these into a single release note.","commit_id":"1ef837f8285b2ad2bad66dbe3c55f7161ad83747"}],"releasenotes/notes/stop-scheduling-to-disabled-cells-eadbfe30d1f6be65.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe93c36cd1c78caabfb87c1b880f88869be12128","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support for stopping new schedulings to disabled cells has been added."},{"line_number":5,"context_line":"    Firstly the concept of disabled cells has been introduced which means such"},{"line_number":6,"context_line":"    disabled cells will not be candidates for the scheduler. Secondly changes"},{"line_number":7,"context_line":"    have been made to the filter scheduler to ensure that it chooses only the"}],"source_content_type":"text/x-yaml","patch_set":13,"id":"bf659307_6fd8c752","line":4,"range":{"start_line":4,"start_character":25,"end_line":4,"end_character":40},"updated":"2018-03-26 15:47:40.000000000","message":"This is a bit ambiguous. The entire first sentence is a bit hard to parse actually.\n\nMaybe re-word as, \"Support for filtering out disabled cells during scheduling for server create or unshelve requests has been added.\"\n\nThe point being, we need to be clear that this only applies to server create and unshelve, not other operations on existing instances already in a cell, like move operations (cold/live migrate and evacuate).","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1220ba3a4e64331928b71bdb52823160263f5c38","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support for stopping new schedulings to disabled cells has been added."},{"line_number":5,"context_line":"    Firstly the concept of disabled cells has been introduced which means such"},{"line_number":6,"context_line":"    disabled cells will not be candidates for the scheduler. Secondly changes"},{"line_number":7,"context_line":"    have been made to the filter scheduler to ensure that it chooses only the"}],"source_content_type":"text/x-yaml","patch_set":13,"id":"bf659307_0fb26be7","line":4,"range":{"start_line":4,"start_character":25,"end_line":4,"end_character":40},"in_reply_to":"bf659307_6fd8c752","updated":"2018-03-26 15:55:55.000000000","message":"Done","commit_id":"0dba3b909c419567844e385f9b0e25cfa301dcf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"38ecde52930f6c960f65ef9e0fc750c760afda1b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"bf659307_28f4c55a","line":12,"updated":"2018-03-28 13:33:04.000000000","message":"We should have a separate \u0027upgrade\u0027 impact release note about the nova-scheduler startup behavior change such that if you start nova-scheduler before creating your cell mappings, you will need to SIGHUP or restart the service to register the cells in the scheduler cache.\n\nFortunately, the install guide tells you to restart the controller services after getting cells setup:\n\nhttps://docs.openstack.org/nova/latest/install/controller-install-ubuntu.html#finalize-installation","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"bf532eab94c4522b40ee639e2ec2c606c44db8b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"bf659307_2f3eeb8b","line":12,"in_reply_to":"bf659307_28f4c55a","updated":"2018-03-29 14:21:50.000000000","message":"\u003e We should have a separate \u0027upgrade\u0027 impact release note \n\nI misunderstood this separate reno part for two of them, what you wanted was just another upgrade section...","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ba99f803eaf8f5576104a9f620434da7e7cb35c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"bf659307_431d5a64","line":12,"in_reply_to":"bf659307_28f4c55a","updated":"2018-03-28 13:42:41.000000000","message":"sure, I will create a new upgrade release note then.","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"65a687b71817ee919c1e14b211e1b05913f909d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"bf659307_a26d42cd","line":12,"in_reply_to":"bf659307_431d5a64","updated":"2018-03-28 22:13:28.000000000","message":"Note that you can add the upgrade section in the same release note, it\u0027s just:\n\nfeatures:\n  - |\n    Support for foobars are great @ CERN LOL?!\nupgrade:\n  - |\n    BTW BROKE ALL YOUR TOOLS","commit_id":"2b4a4a8b0a7f041f8dee8ae96076896a89ca1fa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a70c189c6efd7baae2e25d52e45d70bb81438dac","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support for filtering out disabled cells during scheduling for server"},{"line_number":5,"context_line":"    create or unshelve requests has been added. Firstly the concept of disabled"},{"line_number":6,"context_line":"    cells has been introduced which means such disabled cells will not be"},{"line_number":7,"context_line":"    candidates for the scheduler. Secondly changes have been made to the filter"},{"line_number":8,"context_line":"    scheduler to ensure that it chooses only the enabled cells for scheduling"}],"source_content_type":"text/x-yaml","patch_set":18,"id":"bf659307_6deaa10f","line":5,"range":{"start_line":5,"start_character":11,"end_line":5,"end_character":22},"updated":"2018-03-29 22:55:11.000000000","message":"Sorry about this since I brought it up, but I just realized that unshelve goes to the same cell that the instance was already in, because of the instance mapping - we don\u0027t delete that when we shelve offload an instance:\n\nhttps://github.com/openstack/nova/blob/9ea731a652d0f59b2b7b0d17f0be73b62739f7c2/nova/conductor/manager.py#L778","commit_id":"a77718d7bd8302090b8bf86351a9376d07247bd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3e769e8f20b18e7f33da15cef93574ea4d403e1e","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support for filtering out disabled cells during scheduling for server"},{"line_number":5,"context_line":"    create or unshelve requests has been added. Firstly the concept of disabled"},{"line_number":6,"context_line":"    cells has been introduced which means such disabled cells will not be"},{"line_number":7,"context_line":"    candidates for the scheduler. Secondly changes have been made to the filter"},{"line_number":8,"context_line":"    scheduler to ensure that it chooses only the enabled cells for scheduling"}],"source_content_type":"text/x-yaml","patch_set":18,"id":"bf659307_d0efae1b","line":5,"range":{"start_line":5,"start_character":11,"end_line":5,"end_character":22},"in_reply_to":"bf659307_6deaa10f","updated":"2018-03-30 23:16:25.000000000","message":"Done","commit_id":"a77718d7bd8302090b8bf86351a9376d07247bd7"}]}