| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-14 | |||
| 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 | |
| 16:32:58 | dansmith | bauzas: ah, about that I seee | |
| 16:33:15 | dansmith | bauzas: well, I was against enforcing any one of the many ways to write a uuid in the first place | |
| 16:33:19 | dansmith | the warning was the compromise | |
| 16:33:33 | dansmith | melwitt: looks okay to me | |
| 16:33:53 | melwitt | cool, thanks | |
| 16:33:56 | bauzas | dansmith: right, I remember the early and shiny days of nova/objects/fields.py and the UUID() field type :) | |
| 16:34:03 | dansmith | yeah | |
| 16:34:35 | tssurya | melwitt: did a small change, otherwise looks good to me | |
| 16:35:34 | melwitt | sweet, thanks | |
| 16:36:14 | bauzas | dansmith: stephenfin: gibi: so I'm wrapping my head around http://paste.openstack.org/show/700970/ | |
| 16:36:28 | bauzas | because this is wrong | |
| 16:36:47 | dansmith | it's opinionated | |
| 16:37:00 | stephenfin | bauzas: I think the RFC is wrong | |
| 16:37:01 | stephenfin | :) | |
| 16:37:38 | bauzas | it says 16 octets, period. | |