| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-12 | |||
| 12:01:29 | stephenfin | I guess something could have changed in the time between the two tests | |
| 12:01:30 | sean-k-mooney | the filter does not operate on requests | |
| 12:01:34 | sean-k-mooney | they operate on hosts | |
| 12:01:50 | stephenfin | yeah, correct | |
| 12:02:16 | stephenfin | so if our order is preserved, we'll have hit that host already due to the allocation request for PCPUs | |
| 12:02:41 | stephenfin | if it failed once, it will fail if we hit it a second time due to the allocation request with VCPUs | |
| 12:02:43 | stephenfin | right? | |
| 12:03:00 | alex_xu | no, the scheduler filtered the hosts by allocation-candidtes first, then iterate the filtered hosts | |
| 12:03:03 | efried | "something could have changed" disregard this, already a window between GET /a_c and PUT /allocs | |
| 12:04:03 | stephenfin | efried: I mean we take the allocation request with PCPUs and try passing that through the NUMATopologyFilter or something and it fails | |
| 12:04:05 | sean-k-mooney | i though we got all the host form all allcoation candiate and looped over them once | |
| 12:04:14 | stephenfin | we then try a load of other hosts, which all fail | |
| 12:04:15 | sean-k-mooney | applying all fiters | |
| 12:04:28 | sean-k-mooney | then passed that to the weighers | |
| 12:04:34 | stephenfin | then we come back to that same original host only using the allocation candidate with VCPUs | |
| 12:04:42 | sean-k-mooney | and finally select a host then find the corresponing allcoaiton candiate | |
| 12:04:43 | alex_xu | stephenfin: maybe we ignore the allocation_request if the rp_uuid already in alloc_reqs_by_rp_uuid https://review.opendev.org/#/c/681719/1/nova/scheduler/manager.py@212 | |
| 12:05:06 | stephenfin | only in the time inbetween, something has changed on the host so the NUMATopologyFilter or whatever passes the request | |
| 12:05:24 | stephenfin | it's going to be such a tiny window but I wonder if it's worth worrying about? | |
| 12:05:47 | sean-k-mooney | stephenfin: if something changes the claim wil fial in placmeent | |
| 12:05:48 | alex_xu | ah, no, I'm wrong, we can't ignore directly, since we have same rp for child rp | |
| 12:05:53 | sean-k-mooney | then we will move on to the next host | |
| 12:06:09 | efried | I wouldn't have thought it was any different than what we're doing now for multiple candidates on one host. | |
| 12:06:27 | stephenfin | no, it's not really actually | |
| 12:06:38 | efried | sean-k-mooney: I think he's talking about if the second one spuriously succeeds | |
| 12:07:01 | efried | but yeah, I don't think this is worth worrying about. | |
| 12:07:05 | sean-k-mooney | my point is i dont think the numa toplogy filter will ever run twice for the same host | |
| 12:07:11 | sean-k-mooney | it will only run once | |
| 12:07:18 | efried | and i think that ^ is what we want | |
| 12:07:19 | alex_xu | ^ agree | |
| 12:07:32 | alex_xu | agree to sean-k-mooney... :) | |
| 12:07:33 | sean-k-mooney | because filter today have no knowadge of allcoation candates | |
| 12:08:14 | efried | because if there are available PCPUs for a host, we want to check that candidate only. But if there aren't any, there will only be (at most) VCPU candidates in the list. | |
| 12:08:47 | sean-k-mooney | if there are free pcpus and vcpus we will pass the host and have two candiates | |
| 12:09:00 | sean-k-mooney | and hopefully rely on orderr to pic the PCPU one | |
| 12:09:14 | efried | Yes exactly. Whole point of this is to make sure the first thing we look at is *either* a PCPU candidate *or* there are no PCPU candidates. | |
| 12:09:29 | sean-k-mooney | ya | |
| 12:09:49 | sean-k-mooney | stephenfin: rather then guessing | |
| 12:09:52 | sean-k-mooney | can you add a test | |
| 12:10:05 | stephenfin | sure. What should the test do? | |
| 12:10:06 | sean-k-mooney | create a host that has inventories fo both | |
| 12:10:15 | sean-k-mooney | and assrt the pcpu inventory was calimed | |
| 12:10:20 | sean-k-mooney | as a functional test | |
| 12:10:36 | stephenfin | I think I have that already | |
| 12:10:40 | sean-k-mooney | if it is we are all good | |
| 12:10:58 | efried | If ordering was random, that would be nondeterministic | |
| 12:11:07 | stephenfin | sean-k-mooney: Yeah, look at https://review.opendev.org/#/c/671801/46/nova/tests/functional/libvirt/test_numa_servers.py@364 | |
| 12:11:10 | sean-k-mooney | well we coudl do it in a loop | |
| 12:11:10 | alex_xu | If a host have both vcpu and pcpu, then we will get two allocation_request in alloc_reqs_by_rp_uuid https://review.opendev.org/#/c/681719/1/nova/scheduler/manager.py@212 | |
| 12:11:15 | sean-k-mooney | bug fair point | |
| 12:11:24 | stephenfin | It's a resize test but it does exactly this | |
| 12:11:25 | alex_xu | then we only claim the PCPU one | |
| 12:11:28 | stephenfin | Lemme run that in a loop | |
| 12:11:29 | efried | alex_xu: exactly | |
| 12:12:14 | alex_xu | so there will be check to found out there two allocation req, one for vcpu and another for pcpu, then only claim for pcpu | |
| 12:14:14 | sean-k-mooney | ok i think ye have that in hand to ill go back to the vpmem reviews | |
| 12:17:34 | openstackgerrit | Ivaylo Mitev proposed openstack/nova master: VMware: Update flavor-related metadata on resize https://review.opendev.org/681004 | |
| 12:18:08 | stephenfin | efried: What was the decision on '.update()' vs. the for-loop again? | |
| 12:21:18 | sean-k-mooney | stephenfin: update is fine | |
| 12:21:36 | sean-k-mooney | the sumaris will be the same | |
| 12:22:15 | sean-k-mooney | efried: the vpmem xml generation patch looks sane to me | |
| 12:22:29 | sean-k-mooney | im starting form the bottom of the seriese now and working my way up | |
| 12:25:00 | openstackgerrit | Merged openstack/nova master: Fix race in _test_live_migration_force_complete https://review.opendev.org/681540 | |
| 12:25:01 | sean-k-mooney | donnyd: thanks for the head up on the mirror issue | |
| 12:26:24 | openstackgerrit | Ivaylo Mitev proposed openstack/nova master: VMware: Update flavor-related metadata on resize https://review.opendev.org/681004 | |
| 12:28:03 | donnyd | sean-k-mooney: I think we have it narrowed down. should be back up in the next couple hours | |
| 12:28:30 | sean-k-mooney | it looks like its the same issue i was having from your conversation on infra | |
| 12:28:41 | sean-k-mooney | you are hitting MTU issue becasue of the tunnel | |
| 12:28:49 | sean-k-mooney | well similar | |
| 12:29:00 | sean-k-mooney | i needed to clamp it on HEs end | |
| 12:29:14 | sean-k-mooney | in your case you need to clamp the mtu in the neutron subnet | |
| 12:29:39 | sean-k-mooney | but same general symptoms of retransmits of large packets | |
| 12:30:24 | sean-k-mooney | by the way i also sit in the infra channel so you can always ping me there if there are issues | |
| 12:30:39 | sean-k-mooney | if you want to notify nova in general then here is fine too | |
| 12:37:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Skip querying resource request in revert_resize if no qos port https://review.opendev.org/681513 | |
| 12:38:12 | gibi | bauzas, mriedem: I've finished respinning the bandwidth series | |
| 12:41:45 | openstackgerrit | Thomas Bechtold proposed openstack/nova master: Add nova-status to man-pages list https://review.opendev.org/681733 | |
| 13:05:59 | bauzas | gibi: ack, will look | |
| 13:06:52 | openstackgerrit | Thomas Bechtold proposed openstack/nova master: Add nova-status to man-pages list https://review.opendev.org/681733 | |
| 13:10:35 | sean-k-mooney | luyao: efried stephenfin https://review.opendev.org/#/c/678448/21/nova/objects/base.py i think that is a proablem that should be fixed. it could be a follow up but all_things_equal is not a valid comparison implematnion of == | |
| 13:13:07 | efried | sean-k-mooney: say wha? | |
| 13:13:34 | sean-k-mooney | its not symetic so a == b will not always be the same as b == a | |
| 13:13:47 | sean-k-mooney | and it does not check type | |
| 13:14:17 | sean-k-mooney | so two ovo with the same filed will compare equal if they are complete unrelated types | |
| 13:14:18 | efried | sean-k-mooney: This code is copied in from numa.py | |
| 13:14:38 | efried | a is not None because this is invoked with `self` | |
| 13:14:55 | efried | and I don't care if they're different types. | |
| 13:15:05 | mriedem | lyarwood: have you seen this? https://bugs.launchpad.net/nova/+bug/1843643 | |
| 13:15:06 | openstack | Launchpad bug 1843643 in OpenStack Compute (nova) "VM on encrypted boot volume fails to start after compute host reboot" [Undecided,New] | |
| 13:16:14 | sean-k-mooney | efried: well it a free function so you can only rely on it being invoked with self when it is used to implement _equal_ | |
| 13:16:32 | sean-k-mooney | * __eq__ | |
| 13:16:41 | efried | right, but it's not called "two_objects_identical" | |
| 13:16:46 | efried | it's called "all_things_equal" | |
| 13:17:06 | sean-k-mooney | yes and equal is a stonger guarentee then equvalent | |
| 13:17:19 | sean-k-mooney | anyway | |
| 13:17:23 | lyarwood | mriedem: no if it's new | |
| 13:17:25 | sean-k-mooney | the way its used it will work | |
| 13:18:03 | efried | sean-k-mooney: the only reason this code is added to base is so it can be removed from numa and reused | |
| 13:18:07 | efried | the code isn't changed. | |
| 13:18:19 | sean-k-mooney | so ill change to a +1 i guess but i still think that is an incorerct implemation of __eq__ | |