| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-24 | |||
| 08:52:58 | alex_xu | gibi: thanks | |
| 08:59:45 | ameeda | please please please, can anyone approve my gerrit here ? https://review.openstack.org/#/c/526900/ | |
| 09:23:20 | mdbooth | stephenfin: You were holding off on a +2 for melwitt ? https://review.openstack.org/#/c/523958/18 | |
| 09:58:20 | bauzas | morning folks | |
| 09:58:28 | maciejjozefczyk | bauzas: hiho | |
| 09:58:29 | bauzas | my presence is a bit off, given I'm in a conference | |
| 09:58:47 | bauzas | just ping me directly if you need me | |
| 09:59:02 | maciejjozefczyk | bauzas: good to know cause I have something for you | |
| 10:08:18 | openstackgerrit | nalini proposed openstack/nova master: Modify show aggregate to display 'updated_at' value https://review.openstack.org/537334 | |
| 10:11:34 | Spazmotic | morning | |
| 10:23:31 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Avoid suspending guest with attached vGPUs https://review.openstack.org/535693 | |
| 10:53:06 | Spazmotic | Too full.. will die | |
| 10:53:11 | Spazmotic | or burp.. one of those things | |
| 11:21:40 | openstackgerrit | Alex Xu proposed openstack/nova master: placement: enable required traits from the flavor extra specs https://review.openstack.org/536085 | |
| 11:21:40 | openstackgerrit | Alex Xu proposed openstack/nova master: placement: using the dict format for the allocation in claim_resources https://review.openstack.org/536083 | |
| 11:22:21 | alex_xu | gibi: stephenfin, ^ sorry for I missed that two unittests, just fix them | |
| 11:22:33 | stephenfin | (y) | |
| 11:24:24 | gibi | alex_xu: no worries, I was pulled into something internally so I have to go back to your patches | |
| 11:25:02 | alex_xu | gibi: no problem, thanks for your time | |
| 11:25:56 | openstackgerrit | Alex Xu proposed openstack/nova master: Fix nits in support traits on allocation candidates API https://review.openstack.org/537351 | |
| 11:44:25 | rgerganov | cdent, the issue I had yesterday: https://review.openstack.org/#/c/533821/7/nova/scheduler/client/report.py | |
| 11:44:46 | rgerganov | cdent, you were right that the problem is in update_from_provider_tree :) | |
| 11:48:50 | cdent | rgerganov: ah, interesting, nice sleuthing. Was the trait failure because you were using a not-allowed trait? | |
| 11:49:02 | rgerganov | yes :) | |
| 11:49:23 | rgerganov | speaking of that, how do I add a new trait? | |
| 11:52:18 | cdent | rgerganov: are you wanting to establish a new official trait, or set a CUSTOM trait in the local deployment? | |
| 11:52:42 | cdent | if the latter: https://developer.openstack.org/api-ref/placement/#update-traits | |
| 11:52:58 | cdent | if the former: https://github.com/openstack/os-traits | |
| 11:53:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Don't filter out sibling sets with one core https://review.openstack.org/537361 | |
| 11:53:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Update tests to set 'NUMACell.siblings' https://review.openstack.org/537362 | |
| 11:53:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Always pass 'NUMACell.siblings' to _pack_instance_onto_cores' https://review.openstack.org/537364 | |
| 11:53:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure emulator threads are always calculated https://review.openstack.org/537363 | |
| 11:53:14 | stephenfin | sahid: Fancy taking a look at those ^ ? | |
| 11:53:16 | rgerganov | cdent, thanks, I believe it is the latter | |
| 12:57:21 | efried_back_wed | rgerganov Haven't caught up fully; what news? | |
| 12:58:43 | rgerganov | efried, hi, I found why the local tree was missing the nested RP, see my comment here: https://review.openstack.org/#/c/533821/7/nova/scheduler/client/report.py | |
| 12:59:32 | rgerganov | efried, my driver was trying to create an incorrect trait and update_from_provider_tree silently removed the nested RP | |
| 13:02:42 | efried | rgerganov Hum, yeah, that code will remove the entire sub-branch from the tree (but only remove its root from the association cache - yet another leak in that cache, though it's resolved a couple patches up the series). | |
| 13:03:45 | efried | rgerganov But that's *supposed* to be okay, because we should wind up restoring it next time we refresh_associations | |
| 13:04:47 | rgerganov | efried, this is not the case, we don't restore the nested RP and when we try to recreate it, we fail | |
| 13:05:34 | efried | rgerganov So the bug we need to track down is *why* we don't restore it in the cache. | |
| 13:05:48 | efried | rgerganov Oh, is all of this happening in a single invocation of update_from_provider_tree ? | |
| 13:06:00 | rgerganov | efried, yes | |
| 13:06:13 | efried | Aha | |
| 13:06:26 | rgerganov | efried, in one invocation we create the nested RP in placement and mess up the local tree | |
| 13:06:46 | rgerganov | and on the invocation we try to create it again and then fail | |
| 13:06:53 | rgerganov | and on the next invocation we try to create it again and then fail | |
| 13:07:06 | efried | Wellll | |
| 13:07:31 | efried | I would have expected us to refresh at some point before that second invocation | |
| 13:07:43 | rgerganov | that's why I was thinking that https://review.openstack.org/#/c/536902 is a good idea | |
| 13:08:09 | rgerganov | even if the root node exists, refresh the tree before calling the virt driver for changes | |
| 13:09:29 | efried | rgerganov As written, that change will *not* refresh the tree, though | |
| 13:10:17 | efried | If you want a simple try-out, change to force=True | |
| 13:11:10 | rgerganov | efried, this loop doesn't run at all: for u in self._provider_tree.get_provider_uuids(uuid): | |
| 13:11:25 | rgerganov | because there are no children in the local tree | |
| 13:11:39 | efried | rgerganov That call should at least return `uuid` itself. | |
| 13:11:48 | rgerganov | ah, correct | |
| 13:11:53 | rgerganov | my bad | |
| 13:12:00 | efried | And then the _refresh_associations call should re-grab all the tree-associated providers | |
| 13:12:23 | efried | But possibly only if the cache timeout has occurred. | |
| 13:12:32 | efried | Which may actually be the issue. | |
| 13:12:54 | efried | So we'll refresh the root there, but then on the recursive part of the call, we'll check the cache timeout map | |
| 13:13:21 | efried | ...which will still have stale entries because we only removed the nodes from the ProviderTree; we didn't remove the cache timeout entry for the descendants. | |
| 13:14:02 | efried | So force=True as mentioned above would work; but the more correct solution would be to remove all those other association refresh times when we invalidate that root from the cache. | |
| 13:14:06 | rgerganov | efried, do you want me to try it? | |
| 13:14:22 | efried | rgerganov If you've got a setup all ready to go, please do. It would take me some time to get to that point. | |
| 13:14:32 | efried | rgerganov Meanwhile, let me come up with the other solution real quick... | |
| 13:14:33 | rgerganov | ok | |
| 13:17:53 | rgerganov | efried, with force=True it refreshed the associations for the root provider but it didn't add the nested RP in the tree | |
| 13:20:06 | efried | rgerganov At a glance, I don't see (and I don't offhand remember) how tree-associated providers get refreshed. While I poke at that, would you please try this: https://review.openstack.org/#/c/533821/7/nova/scheduler/client/report.py@1381 | |
| 13:21:19 | rgerganov | efried, ok, trying | |
| 13:21:20 | efried | Okay, that's not gonna work either. | |
| 13:21:28 | efried | But try it anyway, for grins. | |
| 13:21:33 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Transform instance.resize_prep notification https://review.openstack.org/465081 | |
| 13:24:47 | efried | rgerganov https://review.openstack.org/#/c/526541/19/nova/scheduler/client/report.py@631 and @639 are the things that *should* be repopulating the _provider_tree from placement. | |
| 13:25:00 | rgerganov | efried, AttributeError: 'SchedulerReportClient' object has no attribute 'association_refresh_time' | |
| 13:25:17 | efried | rgerganov Oh, are you sitting on the very top of the series? | |
| 13:25:29 | efried | Or did I forget the underscore :) | |
| 13:27:33 | openstackgerrit | Gao Fei proposed openstack/nova master: Update VMWare vSphere link address https://review.openstack.org/535244 | |
| 13:27:58 | gibi | is it only me or gerrit is slow like hell today | |
| 13:27:58 | rgerganov | efried, my patches are based on top of "Move refresh time from report client to prov tree" | |
| 13:28:01 | gibi | ? | |
| 13:28:11 | rgerganov | gibi, yes, it's really slow | |
| 13:28:13 | efried | gibi Not just you. Slow as hell. Started grinding yesterday. | |
| 13:28:21 | efried | Maybe ask -infra to bounce it. | |
| 13:28:26 | gibi | thanks for the info | |
| 13:28:53 | efried | rgerganov Okay, that patch got rid of _association_refresh_time in the report client, and would have made my suggested fix moot anyway. | |
| 13:29:51 | efried | rgerganov So what I can't figure out is why, in _ensure_resource_provider, the _get_providers_in_tree => populate_from_iterable isn't restoring the descendants to the cache. | |
| 13:30:03 | efried | Are you pdb'ing? | |
| 13:30:07 | rgerganov | efried, yes | |
| 13:30:58 | efried | Can you break at https://review.openstack.org/#/c/536902/1/nova/scheduler/client/report.py@626 at make sure rps_to_refresh contains all the descendants? | |
| 13:31:24 | rgerganov | with or without my patch? | |
| 13:31:35 | efried | Shouldn't matter. | |
| 13:31:52 | rgerganov | ok, tracing | |
| 13:32:13 | efried | Because as it turns out, _refresh_associations isn't where we pick up tree-associated providers. It's in fact in this chunk of _ensure_resource_provider. | |
| 13:33:43 | rgerganov | efried, it does matter because without my patch we don't get there | |
| 13:34:03 | efried | Oh, because 'return uuid'... | |
| 13:34:07 | rgerganov | correct | |
| 13:34:24 | efried | ...but that .exists(uuid) should be False! | |
| 13:34:28 | efried | Because we removed that guy. | |