| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-27 | |||
| 17:36:24 | dansmith | sean-k-mooney: isn't the whole point of this work to integrate the move claims with this stuff so that it works? | |
| 17:36:42 | dansmith | artom: yes | |
| 17:36:50 | sean-k-mooney | we thought that was too much work to justify doing that for sriov | |
| 17:37:09 | sean-k-mooney | also if i recall jay was not a fan of that approch | |
| 17:37:27 | dansmith | so the plan was just to re-use most of the same code paths but special-case out the live migration? | |
| 17:37:30 | sean-k-mooney | which is why we built on the multiple port binding feature and do not use port bindings | |
| 17:38:00 | artom | dansmith, because for the subset of pci_requests that's supported for live migration, claiming resources on the destination is already being handled in https://review.opendev.org/#/c/634606/58/nova/compute/manager.py@6465 | |
| 17:38:03 | sean-k-mooney | no sriov migration jsut does not use move claims because live migration dose not use them | |
| 17:38:38 | artom | sean-k-mooney, so you left me to do the gruntwork l) | |
| 17:38:40 | artom | ;) | |
| 17:39:02 | sean-k-mooney | artom: honestly i was hoping you would not use move claims either but i kind of gave up on that | |
| 17:39:04 | artom | dansmith, and that code predates using claims for live migration at all | |
| 17:39:38 | dansmith | I honestly don't understand this | |
| 17:39:48 | artom | dansmith, can you expand on that "this" is :) | |
| 17:39:53 | dansmith | you want to use move claims for numa but not pci and have a separate code path just for pci live migration? | |
| 17:40:03 | artom | dansmith, PCI predates NUMA | |
| 17:40:08 | dansmith | and? | |
| 17:40:39 | artom | And they decided not use to claims, presumably because the PCI tracker makes it easier to claim resources without going through actual MoveClaim objects | |
| 17:40:47 | sean-k-mooney | and both spec propsoed different approche to the same thing because different peopel reveies and had differen preferences | |
| 17:40:58 | dansmith | you seem to be saying "I can make the mess bigger because it's already a mess" and "even though we're rounding out live support for a thing that already works cold, I'm going to do it a different way because there are already lots of fragments" | |
| 17:41:06 | dansmith | neither of those really satisfy me | |
| 17:41:19 | artom | For NUMA LM, MoveClaim was a handy way to get both claiming of resources and the new instance NUMA topology in a "single operation" | |
| 17:41:47 | sean-k-mooney | move claims are not stictly requried for numa migration either | |
| 17:42:03 | sean-k-mooney | but it would require a lttile more work | |
| 17:42:19 | sean-k-mooney | artom was trying to reuse the existing cold migraiton code to reduce the code change | |
| 17:42:19 | artom | No, but they do make things less racy and simplify getting the new instance numa topology | |
| 17:42:36 | sean-k-mooney | artom: i dont think they do | |
| 17:42:48 | sean-k-mooney | we both claim in the same place. more or less | |
| 17:44:18 | artom | sean-k-mooney, yeah, maybe there was a way to do it less racy-ly even without claims | |
| 17:44:39 | dansmith | tbh, I don't really care whether this uses claims or not, | |
| 17:44:42 | dansmith | but if you're going to, | |
| 17:44:57 | dansmith | I think that just "if live migration: do different thing" in a bunch of random places is not moving us forward | |
| 17:45:16 | dansmith | especially when it comes to things that may silently break data | |
| 17:45:28 | mriedem | "if pci: something something dragons that no one but sean understands" | |
| 17:45:29 | artom | dansmith, so that would mean folding SRIOV live migration into the claims | |
| 17:45:31 | mriedem | ^ since juno | |
| 17:45:37 | dansmith | artom: btw, you've done all this local testing of this.. have you included sriov migration and not seen it clobber pci_requests? | |
| 17:45:44 | sean-k-mooney | we currently do the claim here for sriov https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6428-L6467 | |
| 17:45:57 | artom | dansmith, SRIOV with this is completely untested :( | |
| 17:46:05 | sean-k-mooney | right after we call check_can_live_migrate_source in check_can_live_migrate_destination | |
| 17:46:17 | dansmith | artom: so it's very likely that you're blowing those away with this yeah? | |
| 17:46:22 | artom | Although sean-k-mooney's saying apparently all recent-ish NICs can do SRIOV, so maybe I *do* have the hardware? | |
| 17:46:52 | sean-k-mooney | artom: i have hardware and i set up port forwading so you can ssh in. | |
| 17:47:03 | sean-k-mooney | but im going to check both again after dinner | |
| 17:49:20 | sean-k-mooney | im going to grab dinner but ill kick of a devstack run before i go and setup the test enviromint | |
| 17:49:57 | artom | dansmith, seems likely, yeah | |
| 17:51:11 | sean-k-mooney | we dont use the pci request spec object from the instance sriov migration by the way | |
| 17:51:27 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6428-L8993 | |
| 17:51:46 | dansmith | sean-k-mooney: because why? we store details in neutron about it? | |
| 17:52:13 | artom | Hrmm | |
| 17:52:15 | artom | So actually | |
| 17:52:16 | sean-k-mooney | the only info we need to pass back as the pci address of the new device | |
| 17:52:18 | artom | AFAICT | |
| 17:52:31 | artom | The PCI stuff in the claim doesn't actually *claim* any resources | |
| 17:52:34 | sean-k-mooney | so we store that in the vif port profile which is wher ewe pass it to neutron | |
| 17:52:36 | artom | Just tests that it's supported | |
| 17:53:00 | dansmith | sean-k-mooney: okay, but nova's own structure would still have them in the pci_requests right? | |
| 17:53:07 | dansmith | so what about like rebuild or something? | |
| 17:53:07 | sean-k-mooney | yes | |
| 17:53:35 | artom | Ah, no, it does the _decrease_pool_count() thing, which "claims" the resource | |
| 17:54:20 | sean-k-mooney | how does it interact with this? | |
| 17:54:33 | sean-k-mooney | the claims have 3 states | |
| 17:54:41 | sean-k-mooney | free,cliamed and allocated | |
| 17:54:50 | sean-k-mooney | something like that | |
| 17:55:07 | sean-k-mooney | one of them meens its reserved for an instace the the otehr is its in use | |
| 17:55:10 | dansmith | I'm saying I would expect we use pci_requests if we're doing a rebuild on the instance | |
| 17:55:23 | dansmith | or other operations | |
| 17:55:43 | dansmith | point is just that corrupting our own accounting because sriov is tracked in neutron is not okay I don't think | |
| 17:55:53 | sean-k-mooney | so we are claiming the ot reserve them and later we update the instace with them weh we move it to the allocated state i think. its beed a whild and i dont really rememerb the details | |
| 17:56:08 | sean-k-mooney | this is the import cahgne https://review.opendev.org/#/c/620115/35/nova/compute/manager.py | |
| 17:56:12 | mriedem | we do'nt claim for rebuild | |
| 17:56:16 | mriedem | it's a noop claim | |
| 17:56:28 | dansmith | mriedem: sure, not related to claim | |
| 17:56:59 | dansmith | cross-cell-migrate looks at pci_requests | |
| 17:57:50 | dansmith | looks like regular live migration also looks at them to determine if they're all neutron-related | |
| 17:58:07 | dansmith | just trying to confirm that blowing them away is not something we can just ignore :) | |
| 18:01:02 | efried | dansmith: I'm looking at consecutive_build_service_disable_threshold and not seeing how it could be working | |
| 18:01:24 | dansmith | efried: we removed the compute side of that, if that's what you're looking at | |
| 18:01:40 | efried | "the compute side" | |
| 18:01:53 | efried | meaning... the part that makes it behave in any way other than a bool? | |
| 18:01:54 | dansmith | the part where the compute node self-disables | |
| 18:02:21 | dansmith | there should be renos about this | |
| 18:02:33 | efried | ack | |
| 18:02:46 | dansmith | change-consecutive-boot-failure-counter-to-weigher-428de7da0ed2033a.yaml | |
| 18:03:25 | efried | dansmith: okay | |
| 18:03:26 | efried | so | |
| 18:04:00 | efried | this is now up to deployers installing their own weigher? | |
| 18:04:12 | dansmith | eh? | |
| 18:04:19 | dansmith | did you read the reno? | |
| 18:04:56 | dansmith | efried: https://review.opendev.org/#/c/572195/ | |
| 18:05:01 | efried | ack | |
| 18:05:18 | dansmith | added a weigher, changed the meaning of the threshold for compatibility reasons | |
| 18:05:25 | mriedem | https://docs.openstack.org/nova/latest/user/filter-scheduler.html#weights | |
| 18:05:28 | mriedem | BuildFailureWeigher | |
| 18:07:24 | dansmith | efried: you hip to that jive? | |
| 18:07:58 | efried | Still trying to grok the implications. | |
| 18:08:31 | efried | So in a biggish cloud, if I have a bunch of failures in a row on a particular host, even if I don't hit the default multiplier, it's still going to make scheduling to that node way less likely | |
| 18:08:36 | dansmith | mriedem: beat you to the youtube ref: https://www.youtube.com/watch?v=sgW3RxKdN0Q | |
| 18:09:04 | dansmith | efried: hit the multiplier meaning "change" it? | |
| 18:09:27 | efried | sorry, no, I just mean... | |