| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-14 | |||
| 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. | |
| 16:37:43 | dansmith | bauzas: note it allows removing the dashes :) | |
| 16:37:44 | dansmith | bauzas: because it thinks that's okay :) | |
| 16:37:49 | dansmith | which we could also have done | |
| 16:39:33 | bauzas | we could have done many things | |
| 16:39:34 | stephenfin | bauzas: It didn't matter much before anyway since we were undoing it. This will only start to bite us if/when o.vo decides to make that warning an error https://github.com/openstack/nova/blob/fd59fbd4d1914d2adf35a85435ba4aa433f082cd/nova/cmd/manage.py#L1171 | |
| 16:40:06 | bauzas | but I'm trying to see how we could better coerce in o.vo so that would make both not changing the DB, and make edleafe and stephenfin happy | |
| 16:40:19 | edleafe | bauzas: I really don't care | |
| 16:40:29 | dansmith | stephenfin: that would (a) be a change to our (and others') RPC APIs, but also (b) we could easily handle this in _from_db_obj(), or by doing the migration process with the low-level routines instead of objects | |
| 16:40:33 | edleafe | I was just trying to fix a potential issue | |
| 16:40:37 | bauzas | \o/ | |
| 16:40:55 | bauzas | I'm litterally 20 mins away from a long holiday period | |
| 16:41:07 | edleafe | but spaces in a UUID are not valid. Removing the dashes is fine | |
| 16:41:07 | bauzas | would those 20 mins well spent in fixing that then ? | |
| 16:41:26 | bauzas | c'on | |
| 16:41:32 | bauzas | it's a *string* | |
| 16:41:34 | dansmith | a UUID is a number | |
| 16:41:42 | dansmith | the string representation of it can be many things | |
| 16:41:49 | dansmith | microsoft encloses them in {} to make them stand out | |