)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ecc266665a1190b053af80531c4cb30411b89c8c","unresolved":false,"context_lines":[{"line_number":12,"context_line":"resource provider."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This spec proposes to use this parameter for optimization when we move"},{"line_number":15,"context_line":"instances and the target host is already picked up before asking to"},{"line_number":16,"context_line":"the scheduler."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I99064bf9497c0ede6c9bada2fbc783204364f6f2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_e0ba5a6c","line":15,"range":{"start_line":15,"start_character":41,"end_line":15,"end_character":50},"updated":"2019-03-24 12:09:09.000000000","message":"picked (or \"selected\")\n\nThroughout.","commit_id":"e727a6660c40a1a20f03849b89a428fa8ccbf0db"}],"specs/train/approved/use-placement-in-tree.rst":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ecc266665a1190b053af80531c4cb30411b89c8c","unresolved":false,"context_lines":[{"line_number":16,"context_line":"(See the `Filter Allocation Candidates by Provider Tree`_ spec for details)"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This spec proposes to use this parameter for optimization when we create or"},{"line_number":19,"context_line":"move instances and the target host is already picked up before asking to the"},{"line_number":20,"context_line":"scheduler."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Problem description"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_80b11692","line":19,"range":{"start_line":19,"start_character":46,"end_line":19,"end_character":55},"updated":"2019-03-24 12:09:09.000000000","message":"picked/known/selected (throughout)","commit_id":"e727a6660c40a1a20f03849b89a428fa8ccbf0db"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ecc266665a1190b053af80531c4cb30411b89c8c","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_004c469c","line":59,"updated":"2019-03-24 12:09:09.000000000","message":"And we should be careful to add the value of this field only to groups requesting resources we know live on the compute node, like VCPU/MEMORY_MB, to protect against messing up requests with sharing providers.\n\n(Implementation detail, no need to put that in the spec, just thinking aloud.)","commit_id":"e727a6660c40a1a20f03849b89a428fa8ccbf0db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":25,"context_line":"In create and move instance operations, there are cases where the target host"},{"line_number":26,"context_line":"is already picked before calling the scheduler. Even in such cases, nova"},{"line_number":27,"context_line":"retrieves all the possible candidates from placement. This is inefficient and"},{"line_number":28,"context_line":"can cause, for example, `Bug#1777591`_ filtering out the pre-determined target"},{"line_number":29,"context_line":"resource provider by `Limiting Allocation Candidates`_ feature in placement."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_a60c6a94","line":28,"updated":"2019-04-04 22:31:52.000000000","message":"Right, so we workaround this by removing the limit parameter from the GET /a_c call, but that\u0027s obviously inefficient since placement could return 10K candidates that the nova scheduler then needs to filter through just to find the exact one it wants.\n\n(later)\n\nOK I see you called this out in the Alternatives section.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"37a26a8b7aafe9dc2ec1507fc1e29ccad6b5915c","unresolved":false,"context_lines":[{"line_number":41,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Instead of issuing the inefficient request to placement, we will use"},{"line_number":44,"context_line":"``in_tree`` query with the pre-determined target host resource provider"},{"line_number":45,"context_line":"uuid calling the ``GET /allocation_candidates`` API."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Alternatives"},{"line_number":48,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_a7bc916c","line":45,"range":{"start_line":44,"start_character":42,"end_line":45,"end_character":4},"updated":"2019-04-03 12:52:22.000000000","message":"note-for-self: okay so this will be stored in the in_tree field that you are adding and then passed into the query.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"37a26a8b7aafe9dc2ec1507fc1e29ccad6b5915c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"------------"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Disable the `Limiting Allocation Candidates`_ feature calling placement."},{"line_number":51,"context_line":"This is actually what we have now as workaround, but not efficient."},{"line_number":52,"context_line":"(See the `unlimiting allocation candidates`_ patch for details)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"Data model impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_e757f925","line":51,"range":{"start_line":51,"start_character":49,"end_line":51,"end_character":66},"updated":"2019-04-03 12:52:22.000000000","message":"++","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"37a26a8b7aafe9dc2ec1507fc1e29ccad6b5915c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_07f7a55b","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"updated":"2019-04-03 12:52:22.000000000","message":"It could be me because I am not fully aware of the discussions around this, but for a first time reader, it would be nice to have more information about this field.\n\nFor instance I saw this patch: https://review.openstack.org/#/c/649532 where you are adding a new node_uuid field to request_spec.destination and I can see you have this listed in your work items, but if this is the agreed approach we should mention this in the spec. Although I am not sure why we should add the uuid field when we can determine that from the requested_destination.node field right ? using the hypervisor_hostname unless I am missing something obvious here.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b33e9e8c42b8e709fb28f09b720154a94b249df6","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_f461b7a9","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_06645e48","updated":"2019-04-05 09:23:31.000000000","message":"\u003e One other thing to note is that to figure out which cell a given\n \u003e compute node is in, we generally look up the host mapping in the\n \u003e nova API DB based on the given host, \n\nI wish we did, but we don\u0027t :( at least not for scheduling after placement gives back the allocation_candidates which is why we have https://bugs.launchpad.net/nova/+bug/1767303.\n\nAs for the host_states building using the host_mapping of the host to get the cell, its completely unnecessary since we already gather the per cell hosts. jaypipes started working on this here : https://review.openstack.org/#/c/623558/ , to improve this.\n\n \u003e but that might not work in the\n \u003e case of something like creating a server with availability_zone\u003dnova::nodename\n \u003e where the host isn\u0027t provided but only the node - in that case we\u0027d\n \u003e have to iterate cells looking for a compute node with the same\n \u003e nodename (which sucks but should be rare - I can only think of\n \u003e ironic baremetal uses for that use case).\n\nYeah exactly, I have a POC https://review.openstack.org/#/c/635532/ which we use downstream to make scheduling faster, but then we abuse the concept of host_mappings and map ironic nodes too :D to avoid this host/node problem. But then I don\u0027t expect this to be merged upstream since we only have the \"host_mapping\" concept and adding a \"node_mapping\" (or extending the host_mapping to include this) just for the baremetal use case would be an overkill maybe.\n\nSo in today\u0027s nova world I think we are safe since we still iterate through all the cells for determining which cell a compute is in. Nevertheless with this spec at least downstream I am expecting a major improvement in schedule time at least we can cut the 2 to 3 seconds that placement takes to scan and return all the 10,000 candidates only for the scheduler to narrow it down to the one we know from the beginning.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d2025da8078e255fa8dff3a1b9c1f6df8686f62c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_1cfef804","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_07f7a55b","updated":"2019-04-04 16:15:09.000000000","message":"Yes, since the the Destination object has both host and node, I would think we ought to be able to determine the in_tree value without an additional field.\n\nGood catch.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_06645e48","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_1cfef804","updated":"2019-04-04 22:31:52.000000000","message":"Agree, we shoudn\u0027t need a new field since the RequestSpec has both the force_hosts/force_nodes (server create scenario) and requested_destination (move ops and rebuild scenario) and we can pull the ComputeNode from the cell database using the host/node values. I think the only question is where does that happen - in the API code? In some placement pre-request filter code? Or maybe in the nova.scheduler.utils.resources_from_request_spec code?\n\nOne other thing to note is that to figure out which cell a given compute node is in, we generally look up the host mapping in the nova API DB based on the given host, but that might not work in the case of something like creating a server with availability_zone\u003dnova::nodename where the host isn\u0027t provided but only the node - in that case we\u0027d have to iterate cells looking for a compute node with the same nodename (which sucks but should be rare - I can only think of ironic baremetal uses for that use case).","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a0f2e8099d1ae38aaca8f40674dd6e85f8acb0a4","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_a6d8dc51","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_a3ee8e1e","updated":"2019-04-05 16:29:51.000000000","message":"https://review.openstack.org/650376","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4b25f7967ec923de8221396f42910b009dcfee98","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_7f403236","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_a6d8dc51","updated":"2019-04-06 11:16:48.000000000","message":"\u003e I think we should definitely centralize that code somewhere in the scheduler if possible\n\nAck. Make sense.\n\n\u003e so we\u0027d have to iterate the cells looking for the compute node if only force_nodes is given\n\nNow I got your point. We have Hostmapping but we don’t have Nodemapping.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"7a9c9404bcb82ef8ab7262753b89b4ee4537a934","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_056c4b9a","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_c54ba35c","updated":"2019-04-05 12:53:02.000000000","message":"\u003e I thougt we couldn\u0027t have duplicate node names at all, because\n \u003e placement returns 400 when you try to create 2 RPs with the same\n \u003e name and nova uses node names for RP names?\n\nyea you are right, in today\u0027s nova world we cannot have duplicate node/host names. What Matt is talking about here is the ironic use case where one nova host can have several compute nodes under it in which case, the compute_node.host for those ironic computes would be the same. Hope that clears your doubt.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff303e02758ff29d86dfb0ecf2c4c1c0bd79e7df","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_a3ee8e1e","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_c54ba35c","updated":"2019-04-05 15:38:46.000000000","message":"\u003e This might be a nit, but if I’m not wrong, rebuild in the same host with a different image also uses force_hosts/force_nodes rather than requested_destination going through the scheduler?\n\nYes that\u0027s true:\n\nhttps://github.com/openstack/nova/blob/7bf27511ac3ae3a4615b2334e971238bc681ab2e/nova/compute/api.py#L3433\n\nAnd there is a hack in the scheduler code to treat it like requested_destination so it still runs through some filters:\n\nhttps://github.com/openstack/nova/blob/7bf27511ac3ae3a4615b2334e971238bc681ab2e/nova/scheduler/host_manager.py#L583\n\nI don\u0027t know why we did force_hosts/nodes for rebuild, we should have just used requested_destination - and still probably can, it just means re-working that code. I\u0027m not saying you\u0027d need to do that as part of this blueprint though, it\u0027s really more of a bug fix for code maintenance.\n\n\u003e If I’m still minority here, I would say okay to issue that DB query again in the scheduler, since we should add to ask at least once to DB somewhere anyway for the create scenario (and I guess, rebuild in the same host case scenario, too). In that case I assume we do so in nova.scheduler.utils.resources_from_request_spec code checking if any of force_host, force_node or requested_distination has value and just translate it to uuid?\n\nAs I noted in your code series, I see you\u0027re setting the value for the move ops that already lookup the compute node, as you said, but not handling the force_hosts case, so I think we should definitely centralize that code somewhere in the scheduler if possible, either resources_from_request_spec or a new placement pre-request filter, like in here:\n\nhttps://github.com/openstack/nova/blob/7bf27511ac3ae3a4615b2334e971238bc681ab2e/nova/scheduler/request_filter.py\n\nFor a move operation, the Destination object already has the cell UUID set, so we should be able to get the CellMapping via the cell UUID and then target the context to query the COmputeNode out of the cell DB to get the node UUID. As I mentioned before, force_hosts/nodes in the server create case won\u0027t have that though since we don\u0027t know which cell the scheduler is going to pick - so we\u0027d have to iterate the cells looking for the compute node if only force_nodes is given, otherwise if we have force_hosts we should be able to lookup the HostMapping to get the cell and from there we can query the cell DB for the compute node record.\n\nI also tend to think the extra DB queries in the scheduler before calling placement to find the node uuid is acceptable given the performance gain it should give us by not having to pull hosts from all cells to match to the requested host and get all of those allocation candidates back.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff303e02758ff29d86dfb0ecf2c4c1c0bd79e7df","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_03a0c2f3","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_f461b7a9","updated":"2019-04-05 15:38:46.000000000","message":"\u003e I wish we did, but we don\u0027t :( at least not for scheduling after placement gives back the allocation_candidates which is why we have https://bugs.launchpad.net/nova/+bug/1767303.\n\nI was referring to figuring out a compute node in a cell given a host name in general, not the scheduler specifically.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"e1e383b2226faf707cf31a79eb3451e27af4c945","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Data model impact"},{"line_number":55,"context_line":"-----------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The ``RequestGroup`` object will have a new field, ``in_tree`` in the new"},{"line_number":58,"context_line":"version."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"REST API impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_c54ba35c","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":8},"in_reply_to":"5fc1f717_f461b7a9","updated":"2019-04-05 12:44:06.000000000","message":"\u003e RequestSpec has both the force_hosts/force_nodes (server create scenario) and …\n\nThis might be a nit, but if I’m not wrong, rebuild in the same host with a different image also uses force_hosts/force_nodes rather than requested_destination going through the scheduler?\n\n\u003e the only question is where does that happen - in the API code? In some placement pre-request filter code? Or maybe in the nova.scheduler.utils.resources_from_request_spec code?\n\nI used the Destination object in the PoC branch because in scenarios we use requested_destination field (move/evacuate operation), in the API layer we have already looked up the DB to get the ComputeNode object [1] and I was reluctant to do the same DB look up again in the scheduler.\n\n[1] https://review.openstack.org/#/c/649533/3/nova/compute/api.py@3598\n\nIf I’m still minority here, I would say okay to issue that DB query again in the scheduler, since we should add to ask at least once to DB somewhere anyway for the create scenario (and I guess, rebuild in the same host case scenario, too). In that case I assume we do so in nova.scheduler.utils.resources_from_request_spec code checking if any of force_host, force_node or requested_distination has value and just translate it to uuid?\n\n\u003e in that case we\u0027d have to iterate cells looking for a compute node with the same nodename\n\nForgive me asking a stupid question, but I\u0027m not catching up with you two. In nova world today, is it acceptable to have\n\n1. the same service host name within a cell\n2. the same service host name globally\n3. the same node name within a cell if the host is different\n4. the same node name globally if the host is different\n\nI thougt we couldn\u0027t have duplicate node names at all, because placement returns 400 when you try to create 2 RPs with the same name and nova uses node names for RP names?","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":80,"context_line":"Performance Impact"},{"line_number":81,"context_line":"------------------"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"The performance would be improved slightly because we don\u0027t need to get all"},{"line_number":84,"context_line":"the candidates."},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"Other deployer impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_264e3ac1","line":83,"range":{"start_line":83,"start_character":34,"end_line":83,"end_character":42},"updated":"2019-04-04 22:31:52.000000000","message":"It could be a great deal if you have 10K+ compute nodes / resource providers in the deployment.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":99,"context_line":"Upgrade impact"},{"line_number":100,"context_line":"--------------"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"N/A"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Implementation"},{"line_number":105,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_66d9a2f2","line":102,"updated":"2019-04-04 22:31:52.000000000","message":"Based on your \u0027other deployer impact\u0027 section, we do have a minimum required placement API check in the \u0027nova-status upgrade checks\u0027 command so we could just bump MIN_PLACEMENT_MICROVERSION in there.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":113,"context_line":"Work Items"},{"line_number":114,"context_line":"----------"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* Changes to support the new feature"},{"line_number":117,"context_line":"    * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":118,"context_line":"      the translation to the query parameter"},{"line_number":119,"context_line":"    * Take out and put in the pre-determined destination host from"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_46dee6da","line":116,"updated":"2019-04-04 22:31:52.000000000","message":"Yeah see above, this isn\u0027t clear about how you plan on doing the translation and where. As noted, I don\u0027t think we probably need a new field but I could be wrong. We definitely shouldn\u0027t persist it in the RequestSpec if we do put something in there.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"e1e383b2226faf707cf31a79eb3451e27af4c945","unresolved":false,"context_lines":[{"line_number":113,"context_line":"Work Items"},{"line_number":114,"context_line":"----------"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* Changes to support the new feature"},{"line_number":117,"context_line":"    * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":118,"context_line":"      the translation to the query parameter"},{"line_number":119,"context_line":"    * Take out and put in the pre-determined destination host from"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_45db337c","line":116,"in_reply_to":"5fc1f717_46dee6da","updated":"2019-04-05 12:44:06.000000000","message":"\u003e We definitely shouldn\u0027t persist it in the RequestSpec if we do put something in there.\n\nBut we persist both requested_destination and forced_hosts/forced_nodes in the RequestSpec, don’t we? I’m not sure the reason though. It is nice to know that reason here though.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63ae9ad476189324bb2ab17bc6a1f45dd7d18332","unresolved":false,"context_lines":[{"line_number":114,"context_line":"----------"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* Changes to support the new feature"},{"line_number":117,"context_line":"    * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":118,"context_line":"      the translation to the query parameter"},{"line_number":119,"context_line":"    * Take out and put in the pre-determined destination host from"},{"line_number":120,"context_line":"      ``RequestSpec`` object to the new ``RequestGroup.in_tree``"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_86d4cebc","line":117,"updated":"2019-04-04 22:31:52.000000000","message":"This sub-list doesn\u0027t format properly in rst.","commit_id":"5636049e7a7025791d95e97497f1592d843cb8b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5406ddcb42d2f58cd6b7d5536c1116c6a85d8cfd","unresolved":false,"context_lines":[{"line_number":35,"context_line":"* Migrating an instance to a host specified by operator"},{"line_number":36,"context_line":"* Live-migrating an instance to a host specified by operator without forcing"},{"line_number":37,"context_line":"* Evacuating an instance to a host specified by operator without forcing"},{"line_number":38,"context_line":"* Rebuilding an instance in the same host with a new image."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Proposed change"},{"line_number":41,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_0f388e9d","line":38,"updated":"2019-04-10 13:22:24.000000000","message":"Nice and thorough list. The only other move operation is unshelve but that doesn\u0027t have a way to specify a target host from the API.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"105a519e39558e0579476e3a756c1590fd7e1f8a","unresolved":false,"context_lines":[{"line_number":100,"context_line":"--------------"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"We will have a minimum required placement API check in the"},{"line_number":103,"context_line":"`nova-status upgrade checks` command."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"Implementation"},{"line_number":106,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fce034c_47564d09","line":103,"range":{"start_line":103,"start_character":29,"end_line":103,"end_character":36},"updated":"2019-04-11 10:29:30.000000000","message":"We always do.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5406ddcb42d2f58cd6b7d5536c1116c6a85d8cfd","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_cf24c662","line":121,"updated":"2019-04-10 13:22:24.000000000","message":"This is sufficiently generic that we can figure out the best place to do this in the scheduling flow, be that the scheduler utils resources_from_request_spec method or a new pre-request filter https://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/scheduler/request_filter.py - I tend the think the latter would be nice and self-contained and thus cleaner. I also don\u0027t think we\u0027d need a config option like the other request filters to make it optional, it should always be on.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"425310e19300e7aa7d70d9dc6c20a9225a6238ea","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_42f0ad91","line":121,"in_reply_to":"5fc1f717_8f7ebe16","updated":"2019-04-10 14:14:32.000000000","message":"I just went through the code series and see how you\u0027re integrating this into resources_from_request_spec - that makes sense so you can ignore most of my rambling above.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40a4c94358fd4e172febb88afe5c5005d610b112","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_8f7ebe16","line":121,"in_reply_to":"5fc1f717_cf24c662","updated":"2019-04-10 13:27:32.000000000","message":"Another note (since I wanted to make sure a request filter would work): the request filters are run before scheduler_utils.resources_from_request_spec is called:\n\nhttps://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/scheduler/manager.py#L132\n\nSo if we have the case below where an admin is forcing to a baremetal compute host but not specifying a specific node, and the request filter queries the compute nodes for that host and finds more than one (so it can\u0027t decide which to use for in_tree), then simply not modifying the requestspec/group should mean the resources_from_request_spec would handle that case like it does today - but that probably wouldn\u0027t work if you revert https://review.openstack.org/#/c/576693/ so maybe a request filter won\u0027t work and you\u0027ll have to bake the new in_tree logic into the resources_from_request_spec flow after all and *not* revert https://review.openstack.org/#/c/576693/? I guess we\u0027ll see when it comes to implementation and we should have sufficient functional test coverage for this case to see what happens.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"18549283d1ee0e4dde7b30ad8f9f8dd3f8e3ad2a","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* Revert the workaround in `unlimiting allocation candidates`_ patch"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_4530fc56","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":56},"updated":"2019-04-08 02:20:56.000000000","message":"Wait, what to do (what\u0027s the current behavior of ironic and other host:node \u003d 1:N drivers) if only the `force_hosts` is provided and multiple nodes are found?\n\nIf this is not \"NoValidHost\" and if we are picking up one of the node under the same host service, we will need to modify the placement REST API to accept the *list* of uuids in the `in_tree` parameter, or we will just use this kind of optimization when only one entry is found in the DB.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"111477e04c86446fa6139b69ec4744e07736e6fb","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* Revert the workaround in `unlimiting allocation candidates`_ patch"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_783c7fbe","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":56},"in_reply_to":"5fc1f717_4530fc56","updated":"2019-04-10 11:26:23.000000000","message":"I don\u0027t think this needs to delay the implementation of this spec, though, does it? I mean, we could implement this spec with a hard \"if force_hosts resolves to a single compute node UUID, then use that for the in_tree\u003d\u003cUUID\u003e query parameter, otherwise, disable the limit and suffer the performance problems\"","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5406ddcb42d2f58cd6b7d5536c1116c6a85d8cfd","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* Revert the workaround in `unlimiting allocation candidates`_ patch"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_8fc6be99","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":56},"in_reply_to":"5fc1f717_783c7fbe","updated":"2019-04-10 13:22:24.000000000","message":"I agree that we don\u0027t need to handle that case. Forcing hosts/nodes during server create is not meant to be used extensively and there is a spec to avoid the force nature of it (https://review.openstack.org/#/c/645458/). If someone is forcing to a specific baremetal node they\u0027ll have to deal with the existing limitation as Jay pointed out (@Surya - do CERN operators do this for baremetal servers?).\n\nFor non-ironic compute nodes, I agree with Jay\u0027s logic which will cover libvirt and similar 1:1 virt driver modeling of the compute node just fine.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"f6444f18e989b486b86efc93bd6b3ed8762f423d","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"  * Add ``in_tree`` field in ``RequestGroup`` object implementing"},{"line_number":120,"context_line":"    the translation into the placement query parameter"},{"line_number":121,"context_line":"  * Add a database query in the scheduler to translate"},{"line_number":122,"context_line":"    ``RequestSpec.force_hosts/force_nodes`` and"},{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* Revert the workaround in `unlimiting allocation candidates`_ patch"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_a2515164","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":56},"in_reply_to":"5fc1f717_8fc6be99","updated":"2019-04-10 14:21:04.000000000","message":"\u003e I agree that we don\u0027t need to handle that case. Forcing hosts/nodes\n \u003e during server create is not meant to be used extensively and there\n \u003e is a spec to avoid the force nature of it (https://review.openstack.org/#/c/645458/).\n \u003e If someone is forcing to a specific baremetal node they\u0027ll have to\n \u003e deal with the existing limitation as Jay pointed out (@Surya - do\n \u003e CERN operators do this for baremetal servers?).\n\nNo we don\u0027t generally force to a particular node, we group them based on hardware/ip/rack and allow users to schedule based on the properties. So we are cool with this.\n\n \u003e \n \u003e For non-ironic compute nodes, I agree with Jay\u0027s logic which will\n \u003e cover libvirt and similar 1:1 virt driver modeling of the compute\n \u003e node just fine.\n\n+1","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"425310e19300e7aa7d70d9dc6c20a9225a6238ea","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    ``RequestSpec.requested_destination`` to the compute node uuid"},{"line_number":124,"context_line":"    and set it to the new ``RequestGroup.in_tree`` field"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* Revert the workaround in `unlimiting allocation candidates`_ patch"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Dependencies"},{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_e234394b","line":126,"updated":"2019-04-10 14:14:32.000000000","message":"As noted in the code series and above, this would be a partial revert since we still need it for the baremetal case where a host is specified but not a node and there are multiple nodes from to select.","commit_id":"085653eb569623fe587612743a4fbfe9887dc7d4"}]}