Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-14
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?
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?

Earlier   Later