| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-14 | |||
| 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 | bauzas | would those 20 mins well spent in fixing that then ? | |
| 16:41:07 | edleafe | but spaces in a UUID are not valid. Removing the dashes is fine | |
| 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 | |
| 16:41:55 | bauzas | how the string translates to an hex is something fine even with spaces | |
| 16:42:07 | bauzas | yeah | |
| 16:42:48 | edleafe | hey, I wanted to store all uuids are 128-bit integers, but got out-voted by the human-readable people | |
| 16:42:56 | edleafe | s/are/as | |
| 16:43:17 | bauzas | edleafe: it's comprehensive | |
| 16:43:32 | bauzas | edleafe: but the o.vo coercing method shouldn't care at all about the formatting | |
| 16:44:04 | bauzas | it should just assume Good Faith (c) | |
| 16:46:45 | cfriesen | on a totally different topic...does nova wait to ensure vifs are actually plugged when doing a live migration? I see it calling self.virtapi.wait_for_instance_event() on instance spawn and cold migration, but not for live. I assume the flow is somewhat different? | |
| 16:47:22 | stephenfin | bauzas: Before you go, fancy pushing these two patches through? https://review.openstack.org/#/c/385071 | |
| 16:47:39 | stephenfin | Additional shuffling things around/adding docstring patches | |
| 16:47:48 | bauzas | my review stats are poor this week | |
| 16:47:57 | bauzas | thanks to the NUMA spec | |
| 16:48:18 | bauzas | (and the f*** tax-credit document I had to write) | |
| 16:48:38 | stephenfin | bauzas: No better time, in that case | |
| 16:48:53 | bauzas | https://docs.python.org/2/library/uuid.html#uuid.UUID "When a string of hex digits is given, curly braces, hyphens, and a URN prefix are all optional.' | |
| 16:49:13 | bauzas | so, yeah, really the space carries a lot more, but meh | |
| 16:49:26 | dansmith | tssurya: ah, yeah, kudos for spotting it in the initial patch :) | |