Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-14
15:53:40 bauzas stephenfin: https://tools.ietf.org/html/rfc4122#page-5
15:53:50 bauzas oops https://tools.ietf.org/html/rfc4122#section-4.1
15:54:06 dansmith stephenfin: it does't change the fact that we're rotating one uuid in a given namespace, and storing it in the DB, just to get past a uuid format warning
15:54:08 mriedem dansmith: i always try to explain why i'm reverting something
15:54:14 mriedem because the later patch might never come
15:54:33 dansmith mriedem: okay, I just didn't want to argue over the rationale and delay the revert, but fair enough
15:54:42 stephenfin dansmith: It seemed no worse than stripping out the spaces. It was still very much a reversible operation
15:55:14 stephenfin However, you're suggesting that it's not actually valid thing to do. I didn't know that and that makes a revert the right thing to do, if so
15:55:20 dansmith stephenfin: it's completely worse because (a) it could create a conflict, (b) it breaks the format that people in the middle of this operation may have already stored and (c) it's much harder to manually inspect if there is a problem
15:55:37 cfriesen cdent: for the VCPU vs NUMA_CORES thing, also bear in mind the proposed specs for shared/dedicated instances on the same host, and even on the same instance, as well as sahid's spec to run the emulator threads on a separate pool of host pCPUs
15:55:56 stephenfin Are UUID conflicts a thing? Aren't there, like, a bajillion permutations?
15:56:03 cdent cfriesen: yeah, it makes my head swim
15:56:06 cfriesen cdent: make that shared/dedicated vCPUs within the same instance
15:56:08 bauzas okay, so I marked the last Requestspec I checked by using a NULL UUID for the instance https://github.com/openstack/nova/commit/09f2d4d5ec3a699176d70c2407ced0ce7cd58197#diff-cbbdc4d7c140314a7e0b2d97ebcd1f9c
15:56:28 stephenfin That was noted in the commit message and I agreed with it https://review.openstack.org/#/c/539323/5//COMMIT_MSG@30
15:56:34 dansmith stephenfin: but it's wrong
15:56:36 bauzas now, I need to understand why the marker is different for instance_mappings
15:56:38 stephenfin :)
15:56:45 cfriesen cdent: if we could figure out an efficient generic way to do dynamic allocation of pCPUs for shared vs dedicated it would simplify things, but it seems too complicated to do generically
15:57:41 bauzas cfriesen: context is https://review.openstack.org/#/c/552924/
15:58:16 openstackgerrit Dan Smith proposed openstack/nova master: Revert "Make the InstanceMapping marker UUID-like" https://review.openstack.org/552937
15:58:24 dansmith bauzas: mriedem stephenfin ^
15:58:38 bauzas cfriesen: cdent: what I'd like is to keep the root RP responsible for the general resource classes for upgrade and consistency purposes, and just add NUMA-specific resources to the children
15:59:17 bauzas the fact that the operator has to handle flavors accordingly (ie. having a NUMA_CORES value that matches with the flavor VCPU) is something important to note, but not a big deal
16:02:49 bauzas stephenfin: see https://docs.openstack.org/nova/latest/contributor/policies.html#reverts-for-retrospective-vetos
16:03:38 stephenfin bauzas: Good to know :)
16:04:45 cfriesen bauzas: so what would the extra-specs look like? would we drop the existing "dedicated" cpu policy?
16:04:52 dansmith stephenfin: wanna discuss why #1 is an issue?
16:05:20 cfriesen bauzas: and what about the "isolate" hyperthreading policy, how would we handle the sibling LCPUs?
16:05:26 dansmith or are you saying it's so unlikely that you don't care?
16:05:33 bauzas cfriesen: I imagine some time being where two things would continue to work
16:05:56 bauzas cfriesen: I'm not trying to boil the ocean and have all the NUMA policies be done in Placement
16:06:22 stephenfin dansmith: Sure. I'd been going under the assumption that UUID conflicts were night-on impossible. Shuffling bits around on an existing UUID wouldn't change that
16:07:00 cfriesen bauzas: agreed, I just don't have a good picture of what the extra specs would look like when asking for dedicated cpus
16:07:07 dansmith stephenfin: https://en.wikipedia.org/wiki/Universally_unique_identifier#Collisions
16:07:20 dansmith stephenfin: "collisions can occur only when an implementation varies from the standards, either inadvertently or intentionally"
16:07:44 dansmith stephenfin: mutating one uuid (even predictably) within a given namespace of others that are not so mutated would amount to a different implementation right?
16:07:48 bauzas dansmith: right, like I said, nothing in the RFC tells "-" is mandatory AFAIK
16:07:56 dansmith bauzas: well, there's also that
16:08:06 dansmith stephenfin: uuids boil down to a very large number space
16:08:10 edleafe dansmith: so is your main concern the possibility of collisions?
16:08:30 dansmith stephenfin: if you reduce that to a human-conceivable range and then populate it with some values, rotating one within the space, you can easily cause a conflict
16:08:56 dansmith edleafe: my main concerns are in the revert.. collisions are one aspect
16:09:12 bauzas cfriesen: from https://docs.openstack.org/nova/pike/admin/flavors.html#extra-specs-numa-topology
16:09:13 dansmith edleafe: totally agree it's very unlikely but it's fundamentally a wrong thing to do to mutate one like that
16:10:25 arvindn05 mriedem: wanted to discuss your review on https://review.openstack.org/#/c/541507/. Are you going to be online an hr from now?
16:10:37 edleafe dansmith: just to be clear: it wasn't to pretty up the unit tests. When o.vo switches from warning to raising, it will break this code
16:10:49 stephenfin dansmith: I imagine it would, yes
16:11:01 edleafe And adding spaces to a UUID is worse munging, IMO
16:11:09 stephenfin and I certainly can't prove it wouldn't
16:11:15 mriedem arvindn05: yes
16:11:17 stephenfin Yeah, that was my thinking ^
16:11:24 stephenfin If we're going to munge, do it right
16:11:28 dansmith edleafe: which may never happen, if you look at the discussion around doing the uuid format checking a couple years ago..
16:11:51 stephenfin bauzas: What was this about using a null UUID as the marker?
16:11:54 arvindn05 mriedem: thanks!
16:11:57 dansmith stephenfin: edleafe: but changing dashes to spaces doesn't change the unique elements of the uuid, it only changes the uniqueness of the string as a SQL hack
16:12:10 bauzas stephenfin: I was walking over the RequestSpec tablre
16:12:21 dansmith using the null uuid is a completely legit way to do this, as bauzas did for reqspec
16:12:34 dansmith but it doesn't help as much in this case because of where the constraints lie
16:13:44 cfriesen bauzas: I understand the current extra specs, just wasn't sure what it would look like with the placement stuff added to it.
16:14:11 openstackgerrit Chris Dent proposed openstack/nova-specs master: Spec for isolating configuration of placement database https://review.openstack.org/552927
16:15:03 bauzas dansmith: yeah, I'm trying to load again the instance mapping context in mind
16:15:29 bauzas and why it has to be different from my walkthrough over reqspec
16:15:32 dansmith bauzas: instance mapping is storing the uuids of the instances, so it can't store a duplicate one since there is a unique constraint on that column
16:16:07 dansmith there's no room inside it to stash the uuid elsewhere and use the null uuid as the marker key
16:16:29 bauzas disclaimer: I have goblins at home that make noise, I need to take my Bose headset
16:20:16 bauzas dansmith: mmmm, I was just considering the Nul UUID for storing the last RequestSpec record I was walking thru
16:20:42 bauzas now, I'm looking at instance_mappings
16:22:37 dansmith bauzas: you were storing the instance uuid inside a blob field, which was not required to be unique (nor would be) and thus it didn't conflict with the actual reqspec for the instance
16:22:47 dansmith bauzas: and using the nil uuid to find the marker each time
16:22:58 bauzas yeah, also because I was wrong 5 mins ago
16:23:12 dansmith in instance mapping, we can't store the instance's uuid exactly again because it's a UC field
16:23:21 bauzas I wasn't walking over reqspec, but rather over the instances table
16:23:27 bauzas right
16:24:04 bauzas dansmith: I wasn't storing the instance field, I was storing the request spec of the last instance I checkedc
16:24:12 dansmith right
16:24:40 bauzas now, I'm looking at the instance mappings migration script
16:24:59 dansmith we could have used the nil uuid here and then stashed the instance's real uuid in the project field or something like that, but that would be rather nasty as well and could have affected runtime in other ways
16:25:04 dansmith and, this is done and in people's systems
16:26:00 bauzas right
16:26:07 bauzas now the thing is done
16:26:35 cdent edleafe: is member_of is a state of ready for review?
16:26:39 cdent s/is/in/
16:28:12 bauzas gibi: so, I just saw https://github.com/openstack/oslo.versionedobjects/commit/0e3526710f67b3b4ebab60864ea060fa9caf9537
16:28:46 bauzas gibi: like I said earlier, I bet that valid UUIDs with spaces instead of hypens are valid per-RFC
16:28:54 bauzas hyphens
16:29:26 mriedem i believe the gibster is currently celebrating the national holiday of the rubik's cube
16:29:42 edleafe cdent: yep
16:29:43 dansmith ugh
16:29:51 melwitt dansmith, mriedem, tssurya: I could use your eyeballs on this cells session recap to add/correct anything I might have missed before I send it to the dev ML https://etherpad.openstack.org/p/nova-ptg-rocky-cells-summary
16:29:54 dansmith bauzas: that's really unfortunate
16:30:11 cdent thanks edleafe
16:30:53 bauzas dansmith: I need some tests on my box
16:31:05 bauzas dansmith: but I think o.vo badly coerces, that's it
16:31:16 dansmith bauzas: hmm?
16:31:35 dansmith bauzas: it should only be emitting a warning (now error) if the format doesn't match
16:31:49 dansmith bauzas: making it required formatting would be breaking our RPC API
16:32:35 bauzas dansmith: I'm just saying that '52ec5cae 1654 42ad bc38 ebab73fbb161' is a valid UUID
16:32:46 bauzas dansmith: so o.vo shouldn't complain at all

Earlier   Later