| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-14 | |||
| 15:26:30 | efried | bauzas: You're having trouble getting the doc to build? | |
| 15:26:37 | bauzas | yep | |
| 15:27:25 | bauzas | nvm, found the issue | |
| 15:27:27 | bauzas | PEBKAC | |
| 15:27:56 | efried | yep, indent that graphic | |
| 15:28:06 | bauzas | yeah, missed the code directive | |
| 15:28:19 | efried | ...and the next one. | |
| 15:28:23 | efried | Oh, or that. | |
| 15:28:58 | efried | and an extra newline around L138 | |
| 15:29:14 | efried | and 150 | |
| 15:29:28 | efried | and 178 | |
| 15:29:41 | efried | bauzas: With those fixed, it builds for me. | |
| 15:30:08 | efried | bauzas: btw, not sure how you're building locally, but I use this to cut build time down to a sub-second: | |
| 15:30:18 | efried | set -x; | |
| 15:30:18 | efried | ( . .tox/docs/bin/activate || return; | |
| 15:30:18 | efried | rele=${1:-rocky}; | |
| 15:30:18 | efried | { | |
| 15:30:18 | efried | specs () | |
| 15:30:19 | efried | find doc/build/html -type f -name '*.html' | xargs realpath ) | |
| 15:30:19 | efried | python setup.py build_sphinx -s doc/source/specs/$rele -c doc/source || return; | |
| 15:30:20 | efried | } | |
| 15:30:24 | stephenfin | bauzas: Also, single-backticks aren't really valid rST. They mean default role which just happens to be italics in current Sphinx. No reason that won't change going forward though (they do tend to break stuff often) | |
| 15:30:38 | stephenfin | But that's a big nit :) Also placed on my review queue | |
| 15:32:27 | bauzas | thanks both of you folks | |
| 15:32:54 | openstackgerrit | Chris Dent proposed openstack/nova-specs master: Spec for isolating configuration of placement database https://review.openstack.org/552927 | |
| 15:32:58 | bauzas | stephenfin: so, for targeting links, you would recommend double-backticks ? | |
| 15:33:16 | stephenfin | bauzas: What do you mean? | |
| 15:33:24 | bauzas | (16:30:24) stephenfin: bauzas: Also, single-backticks aren't really valid rST. T | |
| 15:33:43 | stephenfin | Something like `xyz`_ or :role:`test` is valid. What's not valid is `xyz` | |
| 15:33:50 | bauzas | I used single backticks for explicit targeting | |
| 15:34:17 | bauzas | stephenfin: yeah, so I thought I did that everywhere, were have you seen a single-backtick without a link ? | |
| 15:35:12 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA topology with RPs https://review.openstack.org/552924 | |
| 15:35:38 | stephenfin | bauzas: Line 131 | |
| 15:35:57 | stephenfin | 257 too | |
| 15:36:10 | dansmith | edleafe: hey, I just noticed this went in and I'm a tad confused by it: https://review.openstack.org/#/c/539323/ | |
| 15:36:23 | stephenfin | that's about it though, actually :) Like I said, it's the nitiest of nits | |
| 15:36:24 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA topology with RPs https://review.openstack.org/552924 | |
| 15:36:35 | edleafe | dansmith: on a call now | |
| 15:36:36 | dansmith | edleafe: the uuid warning from o.vo has been around for a long time now and it seems like this is a bit of a heavy-handed way to resolve a warning | |
| 15:36:39 | dansmith | edleafe: ack | |
| 15:36:50 | dansmith | gibi: stephenfin ^ | |
| 15:37:51 | stephenfin | dansmith: Heavy handed, perhaps, but it did resolve the issue and made sense to me | |
| 15:38:24 | dansmith | stephenfin: it resolves the warning in unit tests by storing an obfuscated uuid in the database which would make it really hard to examine what is going on | |
| 15:38:42 | dansmith | that seems not sensical to me | |
| 15:39:31 | dansmith | and would break anyone who was halfway through a mapping and then walked over this | |
| 15:39:35 | dansmith | like someone doing an FFU | |
| 15:40:11 | dansmith | and.. instance uuid _is_ a uuid, so if there's a warning about the format of it, maybe the warning is wrong or something else needs to change | |
| 15:40:35 | dansmith | but munging the values (which I'm not even sure is legit uuid behavior) seems kinda silly | |
| 15:40:36 | openstackgerrit | Merged openstack/nova master: Updated from global requirements https://review.openstack.org/552774 | |
| 15:42:22 | dansmith | if you're not doing the rotation on all uuids then you're increasing your chances for collision, and since this doesn't (actual instance mappings have unrotated uuids, only the marker is rotated) | |
| 15:43:46 | dansmith | since this is munging data in the database I think I'm going to exercise fast revert | |
| 15:44:12 | stephenfin | dansmith: Gimme a sec. I'm trying to re-review this and see what my reasoning for letting it in was | |
| 15:44:51 | cdent | bauzas: I may have missed some discussion in Dublin, but what's the different between a NUMA_CORE and a VCPU and why keep track of both of them? | |
| 15:46:14 | bauzas | cdent: good question | |
| 15:46:21 | bauzas | cdent: see my alternatives section | |
| 15:47:05 | bauzas | if we shard VCPUs across NUMA nodes, then an instance spreading its cores amongst multilple NUMA nodes could have problems | |
| 15:47:09 | dansmith | mriedem: I forget, in my revert patch should I explain the problems I think there are or just do a straight revert and then discuss on a re-propose | |
| 15:47:10 | dansmith | ? | |
| 15:47:17 | cdent | bauzas: yeah, saw that, but that doesn't really answer the question. If you are counting NUMA_CORES (as a separate thing) why would you record VCPU at all? | |
| 15:47:26 | bauzas | dansmith: fast-revert and discuss later IMO | |
| 15:47:50 | dansmith | bauzas: okay that's what I was thinking too, but it's been a while | |
| 15:47:55 | bauzas | cdent: because operators that wouldn't care about NUMA things wouldn't then need to use a NUMA specific RC | |
| 15:48:55 | bauzas | dansmith: stephenfin: so I need a bit of context in order to chime in | |
| 15:49:07 | cdent | I'll have to think on that some more. Having to track the same thing multiple times doesn't seem simple to me. It may, however, be that simple is not possible. | |
| 15:49:36 | bauzas | cdent: np, I'll be on PTO till thurs starting EOB | |
| 15:49:44 | dansmith | bauzas: after further examination, rotating one uuid out of a set is not a legit thing to do to a uuid and maintain uniqueness, so I think it's flawed just on that basis alone | |
| 15:49:47 | bauzas | cdent: so take your time | |
| 15:49:52 | cdent | roger | |
| 15:50:27 | bauzas | cdent: that spec is maybe a strawman | |
| 15:50:33 | dansmith | i.e. if your key space is 0-4, and you have keys 1, 2, 3, you can't just rotate 2, or you will end up with a conflict | |
| 15:50:34 | bauzas | cdent: so I'm open to alternatives | |
| 15:50:38 | dansmith | you have to rotate them all equally | |
| 15:50:41 | cdent | cool | |
| 15:50:43 | stephenfin | dansmith: So my understanding of this was that we're creating something akin to a fake InstanceMapping so we could keep track of where we were in the process | |
| 15:50:45 | bauzas | dansmith: right, I don't like touching things | |
| 15:50:58 | dansmith | stephenfin: that's the marker functionality, yes | |
| 15:51:09 | bauzas | but that's not the only marker we had, right? | |
| 15:51:25 | bauzas | like, I provided a marker IIRC for the RequestSpec migration thing | |
| 15:51:47 | bauzas | now the code is gone, but lemme see how I did that and if that can help | |
| 15:51:53 | stephenfin | dansmith: Right, so if were re-using the UUID of an existing instance we were going to bump into the UNIQUE constraint, as noted on line 1127 there | |
| 15:51:56 | dansmith | bauzas: right, and also, this is storing an _actual_ uuid in this field | |
| 15:52:04 | dansmith | bauzas: so if the warning is complaining about it, then it's wrong | |
| 15:52:18 | stephenfin | and to work around that, alaski put in a patch that munged the instance UUID to avoid said constraint | |
| 15:52:31 | stephenfin | dansmith: UUIDs need to have hyphens in them, no? | |
| 15:52:45 | bauzas | aaaaah I see | |
| 15:53:00 | stephenfin | https://en.wikipedia.org/wiki/Universally_unique_identifier#Format | |
| 15:53:01 | bauzas | stephenfin: the RFC doesn't really mention that IIRC | |
| 15:53:14 | bauzas | it's just a python uuid thing IIRC | |
| 15:53:17 | dansmith | stephenfin: ah, I see what's going on | |
| 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? | |