Earlier  
Posted Nick Remark
#openstack-nova - 2019-09-10
16:23:11 artom mriedem, I'm not saying remove it, I'm saying leave it "legacy CPU resource thing", and come up with new resources to mean shared CPU and dedicated CPU
16:23:19 dansmith artom: -3
16:23:58 artom Anyways, I have 0 context and func tests to fix
16:24:29 sean-k-mooney artom: we talked about SCPU and PCPU in the past but we said no we want to keep VCPU resouces
16:24:37 artom dansmith, hey man, dinner first
16:24:42 sean-k-mooney so way to late to go back that route
16:24:42 stephenfin dansmith: so tl;dr: kill the static scheduler-only config option and instead do "give me PCPU, but if you can't give me PCPU then search for VCPU" instead
16:25:14 dansmith stephenfin: yes, but only the last part of there are old computes around. once you do that one time and find everything is upgraded, stop even doing that check
16:25:27 dansmith stephenfin: remove that compat step in U, no deprecation cycle needed for that
16:25:46 stephenfin What do you mean by old computes?
16:25:51 sean-k-mooney dansmith: if we did that we would need a trait to make the scond query safe
16:26:21 mriedem stephenfin: older than train computes
16:26:25 mriedem based on the nova-compute service rpc api version
16:26:26 dansmith stephenfin: service version will tell you if all computes have been upgraded.. so actually, maybe just always do that in T if no candidates, because they could do the upgrade and the config tweak separately
16:26:26 sean-k-mooney we would need a support_PCPU capablity trait and need to add it as a forbiden trait for the second query
16:26:43 stephenfin that won't work though
16:26:49 dansmith stephenfin: so yeah, what you said
16:26:51 stephenfin because by default we don't report PCPU on Train
16:27:08 stephenfin doing so would force people to set new config options as soon as they upgrade
16:27:14 stephenfin which we can't do
16:27:29 sean-k-mooney stephenfin: if you look at artom migratio ncode we ignore the "can live migrate with numa" config if everythin is upgraded
16:27:40 dansmith stephenfin: that's why I backed away from the version check there
16:28:30 sean-k-mooney that would still require everything to be restated however.
16:28:33 stephenfin sean-k-mooney: right, but that's because the "can live migrate with numa" check only needs to know that code is new enough. it doesn't need the operator to tweak some config first
16:28:38 stephenfin which this does
16:28:58 stephenfin dansmith: Ah, yeah, I missed the "so actually"
16:29:35 dansmith stephenfin: the reason I was heading in that direction, is because:
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

Earlier   Later