| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-10 | |||
| 16:29:36 | sean-k-mooney | i think if we were to do the double query as i said we need the compute capableity trait to protect against lading on a new host when old hsot are available and we ask for vcpus | |
| 16:29:41 | mriedem | you'd only do that fallback for VCPU if the flavor in the request spec (in the scheduler) has resources:PCPU=x right? | |
| 16:30:12 | stephenfin | sean-k-mooney: not gonna happen - the NUMATopologyFilter or libvirt driver protect us | |
| 16:30:15 | dansmith | stephenfin: I was going to suggest we also always expose the inventory, even if we have to synthesize it from the older config, but I'm guessing you're going to say we'd potentially expose as shared or dedicated and be wrong for the intention of the operator right? | |
| 16:30:17 | sean-k-mooney | mriedem: no we are translating hw:cpu_policy=dedicated into PCPU requests | |
| 16:30:38 | dansmith | stephenfin: require them to convert their configs before U, but expose the new inventory right away | |
| 16:30:44 | mriedem | ok but my point is, just make sure we're not unconditionally doing that fallback re-query | |
| 16:30:50 | stephenfin | sean-k-mooney: because if you land on a Train compute node, that'll have explicitly set 'NUMATopology.pcpuset' to None | |
| 16:31:11 | dansmith | mriedem: yes, only do the fallback query for PCPU things | |
| 16:31:19 | sean-k-mooney | ok we could have issue with the limit paramater on placement | |
| 16:31:26 | stephenfin | and if that's None, we've got nothing to pin to | |
| 16:31:33 | stephenfin | so placement will pass but the filter will fail | |
| 16:31:40 | sean-k-mooney | but if we do the second query with out a limit then ya the numa toplogy filter would prevent that | |
| 16:31:49 | sean-k-mooney | so we dont need the trait | |
| 16:32:11 | stephenfin | dansmith: Yeah, exactly | |
| 16:32:17 | dansmith | I don't think the limit is a problem | |
| 16:32:31 | dansmith | stephenfin: so they express their intent right now how? | |
| 16:32:33 | sean-k-mooney | dansmith: doesnt cern set it to like 15 | |
| 16:32:43 | stephenfin | dansmith: My previous solution had been to expose CPU inventory on hosts without the new configuration as both PCPU and VCPU | |
| 16:33:09 | dansmith | sean-k-mooney: we're asking placement for a query that will return things with PCPU resources.. if placement returns nothing then there's nothing that will fit | |
| 16:33:10 | sean-k-mooney | the default limit is 1000 so that should not be a proablem but if its really low then i expect it could be | |
| 16:33:13 | dansmith | sean-k-mooney: their problem is different | |
| 16:33:31 | sean-k-mooney | dansmith: im not talking baout the first query for pcpus | |
| 16:33:32 | dansmith | stephenfin: yeah, I don't think that's a good plan | |
| 16:33:42 | sean-k-mooney | dansmith: i ment the second query for VCPUS | |
| 16:33:58 | stephenfin | Yeah, it's really not. I detailed why in that ML post | |
| 16:34:00 | dansmith | sean-k-mooney: I don't see what the problem is | |
| 16:34:09 | sean-k-mooney | the new host can have invetories of both | |
| 16:34:19 | sean-k-mooney | we could get 15 new hosts and no old hosts | |
| 16:34:26 | dansmith | stephenfin: I'm asking about today.. they use this ambiguous config thing.. how do they control which type land where? | |
| 16:34:28 | sean-k-mooney | then the numa toplogy filter would elinate all hosts | |
| 16:35:06 | stephenfin | the scheduler option and the NUMATopologyFilter | |
| 16:35:44 | dansmith | stephenfin: meaning static config to determine how to treat pinned cpu requests? | |
| 16:36:04 | stephenfin | If the scheduler option is set to False (so it's not translating 'hw:cpu_policy=dedicated' to 'resources:PCPU') then we'll keep requesting VCPU from placement | |
| 16:36:09 | dansmith | no, | |
| 16:36:12 | dansmith | I'm asking about TODAY | |
| 16:36:19 | stephenfin | oh, today | |
| 16:36:21 | dansmith | not the mythical future where your patches are landed | |
| 16:36:23 | stephenfin | gotcha | |
| 16:36:25 | stephenfin | host aggregates | |
| 16:36:28 | dansmith | right, okay | |
| 16:36:31 | stephenfin | and metadata | |
| 16:36:36 | dansmith | that's what I thought | |
| 16:36:46 | stephenfin | that's what you're _supposed_ to use, of course. We never enforced it | |
| 16:37:01 | dansmith | so the problem is that computes literally don't have access to the information they need to know what kind of thing to expose, because they have config, but no access to the aggregate info | |
| 16:37:08 | sean-k-mooney | and windrieve developed a host agent to allow you to mix | |
| 16:37:18 | sean-k-mooney | so they exploited that we did nto enforece it | |
| 16:37:19 | stephenfin | correct | |
| 16:37:29 | dansmith | stephenfin: so, if we defaulted to the looser interpretation of the inventory, | |
| 16:37:32 | stephenfin | the aggregate metadata isn't standardized | |
| 16:37:37 | stephenfin | and it doesn't even need to be set | |
| 16:37:42 | dansmith | then the aggregate configs would still be in place and we could fall back appropriately maybe? | |
| 16:37:51 | efried_afk | stephenfin: Not having looked thoroughly, if I addressed your -1s on the pmem series, a) would I still be able to +2 in your opinion; b) would it do any good over waiting for luyao to hit it overnight, in terms of you being able to get back to it today? | |
| 16:38:35 | dansmith | anyway, we're getting off on a tangent a bit I think, so let me summarize: | |
| 16:39:01 | dansmith | I think we can do away with the scheduler knob by doing the query-nay-requery approach to prioritize upgraded and converted computes | |
| 16:39:17 | dansmith | it would be nice if we could make the compute side smarter and/or default to a closer runtime scenario, | |
| 16:39:32 | stephenfin | efried_afk: So if the comments were addressed could you +2 in my absence? I guess, but I'm happy to review again tomorrow too | |
| 16:39:39 | dansmith | but I'm less concerned about that if they can be reconfigured and restarted in isolation to be picked up by the scheduler by default | |
| 16:39:58 | efried | stephenfin: I meant me +2ing on myself, not assuming your +2. | |
| 16:40:06 | stephenfin | ohh | |
| 16:40:39 | stephenfin | Yeah, sure. There's no major rework necessary in anything I've reviewed so far | |
| 16:41:01 | stephenfin | this will need a good bit of modification though, I suspect https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py | |
| 16:41:19 | stephenfin | dansmith: Yup, that all makes sense to me | |
| 16:43:11 | efried | I ain't touching that patch | |
| 16:43:36 | stephenfin | good call :) | |
| 16:43:44 | gibi | mriedem: replied in https://review.opendev.org/#/c/676140 with some questions regarding the private helper you suggested | |
| 16:48:46 | mriedem | gibi: replied | |
| 17:11:17 | sean-k-mooney | i need to take a break for a bit to clear my head so im going to have something to eat. | |
| 17:11:30 | sean-k-mooney | i just kicked of stacking with the latest version of artoms code | |
| 17:11:39 | sean-k-mooney | ill start testing it when i get back | |
| 17:21:18 | openstackgerrit | Merged openstack/os-resource-classes master: Update api-ref link to canonical location https://review.opendev.org/681235 | |
| 17:23:31 | efried | dansmith: bottom two vpmems on your radar for today? https://review.opendev.org/#/c/678447/ | |
| 17:24:01 | efried | hopefully easy, minor updates per your prior comments | |
| 17:24:42 | dansmith | efried: no, I gotta give a talk in a bit and I spent too much time this morning on reviews already | |
| 17:24:56 | dansmith | if comments were addressed it's probably easy for other people to confirm, | |
| 17:25:08 | efried | if you're okay with that, then sure. | |
| 17:25:21 | dansmith | but for the record, I'm scared of both pcpu and vpmems at this point | |
| 17:25:24 | efried | imo the changes are simple and in line with what you requested | |
| 17:25:48 | efried | I don't blame you. But hey, it's just FF. We have *weeks* to fix bugs :P | |
| 17:43:02 | mriedem | i'll call it, the first major bugs from pmem and pcpu regressions will be after vexxhost upgrades to train, and then in about 18-24 months when other deployments start upgrading to train :) | |
| 17:43:16 | mriedem | maybe cern in a year | |
| 17:43:41 | dansmith | heh, yeah, we won't find any of the bugs in the FF->release window | |
| 17:45:39 | mriedem | is there any way to test pcpu in a gate job if we ran tempest smoke tests in serial or something? like wouldn't that just be a matter of creating a flavor with PCPU, single node devstack? | |
| 17:45:52 | mriedem | and configuring n-cpu for the dedicated CPUs on the host? | |
| 17:45:56 | mriedem | basically all of them | |
| 17:46:53 | dansmith | I'm less concerned about if it actually works in a contrived scenario, and more about an existing deployment trying to get through the upgrade and/or being able to use it without regressions or other issues | |
| 17:47:18 | mriedem | sure, and functional tests are good, they just aren't a replacement for the real thing | |
| 17:47:23 | dansmith | presumably vpmem testing is pretty much impossive | |
| 17:47:27 | dansmith | *impossible | |
| 17:47:43 | mriedem | i assume so, hence the 3rd party ci | |
| 17:57:11 | artom | Which, btw, came up super fast (though I dunno if it had been in the works for a long time before) | |
| 17:57:25 | artom | RH can learn a thing or 2 | |
| 18:02:25 | dansmith | mriedem: so numa LM should be ready for you, if I understand the current state right? | |
| 18:02:50 | dansmith | I hit the object patch again a bit ago | |
| 18:03:30 | dansmith | I think the third patch should be good too, but you wanted some verification from the NUMA boyz which sent it off into that cpu pinning tangent, so not sure if you're still waiting for something there | |
| 18:10:07 | mriedem | artom: came up super fast? you mean the 3rd party CI did? | |
| 18:10:13 | artom | mriedem, yeah | |
| 18:10:20 | artom | Intel_Zuul appeared a few days ago | |
| 18:10:36 | mriedem | dansmith: yeah it's sitting in a tab, was going through gibi's series until i hit a stopping point, which i just did | |
| 18:10:58 | mriedem | gibi: https://review.opendev.org/#/c/676972/ re-introduces the race you just fixed, so i'll rebase the series and fix that and +W once your other change is merged | |