Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-11-05
14:45:14 tssurya efried: well no all we do it increase the config's value to a very high number, ever since we added the config in queens
14:45:26 tssurya so if you allow setting the config to "0"
14:45:35 tssurya that will disable it its good for us
14:45:40 efried tssurya: The patch actually goes quite a bit further than that.
14:46:07 efried I believe with the refresh interval very high, you're still getting the inventory polls.
14:46:14 efried Now you won't even get those anymore.
14:46:41 openstackgerrit John Garbutt proposed openstack/nova-specs master: Add Unified Limits Spec https://review.openstack.org/602201
14:46:45 efried ...I think.
14:47:32 tssurya efried: oh just read the commit (message)
14:47:55 tssurya yea we don't currently have any of those yet downstream (meaning our code is the same as upstream at the tracker level)
14:48:15 tssurya but it would be cool to optimize the calls to placement to the maximum
14:49:01 sean-k-mooney efried: by the way if you have time can you review https://review.openstack.org/#/c/610034/
14:49:07 tssurya because in rocky you have more stuff than in queens right ? the way the updates are done with pulling the prodiver tree info and everything (meaing the new update_to_placement function)?
14:49:35 efried tssurya: What I think we'll be really looking for in order to have enough confidence to merge this is for deployments such as yours and mnaser's to put this in place and verify, not so much that the number of placement calls drops off a cliff - that should be a given - but that we don't wind up with the RT getting out of sync with placement's view.
14:49:43 efried tssurya: I don't believe there's much more in rocky than queens, no.
14:50:11 tssurya we are also investigating on our end
14:50:28 tssurya I can see if we can test this patch in our deployment
14:50:37 tssurya and give feedback
14:51:11 efried One thing about mnaser's comments, which I will also mention in a response on the thread, is that this should not affect *allocations* at all. IIUC, it was allocations getting out of sync that mnaser observed. I will be interested to know if he or you have ever observed anything else (inventories, etc.) getting out of sync.
14:51:29 efried Especially you, since you've been running the long-poll in production for a while, right?
14:51:52 tssurya efried: well the long poll used to only switch off traits and aggregates
14:52:21 efried tssurya: It would also affect inventory updates.
14:52:26 efried oh
14:52:41 efried yeah, I see what you're saying now. I think you're right.
14:52:50 efried and we're not doing much with those in q/r.
14:53:00 tssurya yea I am pretty sure it was only the traits and aggregates syn that was off
14:53:09 tssurya the inventory sync was like normal for us
14:53:28 efried the poll you're stretching would also update inventories, but there was an additional, separate inventory update that was happening outside of that one, so yeah.
14:53:45 tssurya ah yea that makes sense
14:54:00 tssurya because we didn't have that much out of syn inventory issues
14:54:04 tssurya sync*
14:55:16 efried tssurya: The theory behind the patch is that you shouldn't have those issues anyway, even if we *never* poll.
14:55:20 efried but
14:55:30 efried I'm not sure I hit the code paths that do that separate inventory update
14:55:32 efried sad face
14:55:53 jaypipes three more unit test failures to address... closing in...
14:55:56 tssurya haha, I am trying to go through your commit now... but
14:56:08 tssurya so idea is a inventory refresh only if something changes right ?
14:57:08 efried tssurya: well, at least in this code path, yes. Basically, if you set the refresh interval to zero, the only time you would get a refresh is if e.g. the virt driver pushes a change via update_provider_tree.
14:57:31 efried jaypipes: IYO, should there be a bp and/or spec for this? And should it be multiple patches?
14:58:06 jaypipes efried: the cache change stuff?
14:58:09 efried y
14:58:44 jaypipes efried: I think it would be useful to have a bp for tracking purposes, sure. spec, not so much.
14:58:56 efried ight, we'll start there.
15:02:31 mnaser tssurya: I can give you a script I wrote to audit placement (I think it’s in a paste somewhere). Interesting to see if you see things get out of sync too
15:02:50 tssurya efried: hmm yea makes sense as for the syncing issues which I am sure we would hit considering our size, can't we slowly build a heal/sync tool like we have for allocations/aggregates that deployments can run when they want ?
15:03:12 openstackgerrit John Garbutt proposed openstack/nova master: WIP: Integrating with unified limits https://review.openstack.org/615180
15:03:20 tssurya like a placement sync tool for plausible things that would go out of sync
15:04:01 tssurya mnaser: ah thanks so far we haven't like disabled any major sync updates
15:04:29 tssurya but we surely do have out of sync issues
15:04:31 efried tssurya: I think healing allocations is a separate issue. If we can show that inventories/traits/aggregates don't get out of sync when we don't refresh them, that's goodness. And then separately, if we can write something to heal allocations - or better yet, to identify why they're getting out of sync in the first place and close that gap - also goodness.
15:04:41 mnaser tssurya: this is for allocations being out of sync tho, so it’d be interesting if this is something we have broken I guess
15:05:52 efried mnaser: Do you have any suspicions (or better) about where the allocation drift is happening? Like, is it on instances that fail a migration or similar? Resizes? Evacuations? Or (eek) just steady state?
15:06:12 openstackgerrit Merged openstack/nova master: Minimal construct plumbing for nova show when a cell is down https://review.openstack.org/591658
15:06:53 mnaser I think it might be around live migrations, I suspect that if live migrations fail on a machine that somehow already has issues talking to placement then it won’t be able to revert the allocation or whatever
15:07:04 mnaser Do the compute nodes make all the claims during a resize or live migration?
15:07:25 mnaser I.e is it possible scheduler does something that compute cannot revert because of an intermittent issue
15:07:52 tssurya efried, mnaser: there is no way we don't have out of sync issues I am literally already working on a consistency tool but I can confirm more after looking further about the statistics regarding allocations/inventories - about how much those are out of sync
15:07:59 efried mnaser: certainly, although I thought we had (a bunch of really ugly) code to clean up that mess.
15:08:17 sean-k-mooney mnaser: i was under the impression the cpu ram and disk were still claimed in the schduler on migration
15:08:30 sean-k-mooney but pci device would be cliamied by the compute nodes
15:08:43 sean-k-mooney well that is a bad example
15:08:44 efried mnaser: I can't think of a way inventories would get out of sync unless a third party is mucking with them (e.g. CLI).
15:08:56 efried mnaser: But failed migrations of various types - absolutely.
15:09:05 efried ^^ for allocations
15:09:10 mnaser tssurya: let me share a tool that does exactly that!
15:09:25 mnaser Yeah I don’t think I ever had issue with inventories
15:09:41 tssurya mnaser: that would be great then I can get some statistics
15:09:58 efried mnaser: But as we were discussing above, even if you max out the refresh interval, we are still polling (and "healing") the inventories every periodic.
15:10:08 efried so if the inventories did drift, we would have fixed them.
15:10:16 tssurya efried: true
15:10:17 efried So there's really no way to know if drift issues exist there.
15:10:19 mnaser yeah but i dont think those would drift in our use cases
15:10:22 efried ...until I kill that.
15:10:23 mnaser even if they didnt sync
15:10:24 mnaser :P
15:10:57 efried I think I have a pretty major rework to do on this patch now that I'm thinking in terms of the inventory refreshes...
15:11:05 mriedem tssurya: https://bugs.launchpad.net/nova/+bug/1793569
15:11:06 openstack Launchpad bug 1793569 in OpenStack Compute (nova) "Add placement audit commands" [Wishlist,Confirmed]
15:11:31 tssurya mriedem: thanks
15:12:30 mnaser http://paste.openstack.org/show/734146/
15:12:35 mnaser easier to digest paste because of launchpad's wrapping
15:12:44 mnaser tssurya: ^
15:13:07 openstackgerrit John Garbutt proposed openstack/nova-specs master: Add Unified Limits Spec https://review.openstack.org/602201
15:18:21 tssurya mnaser: thanks :)
15:18:39 mriedem efried: fwiw, i would at least split out the change to disable the refresh interval (config value of 0)
15:18:47 mriedem since that's pretty straight forward i imagine
15:18:54 efried mriedem: ack, thx
15:50:48 openstackgerrit Maciej Jozefczyk proposed openstack/nova master: Force refresh instance info_cache during heal https://review.openstack.org/591607
15:50:48 openstackgerrit Maciej Jozefczyk proposed openstack/nova master: Add fill_virtual_interface_list online_data_migration script https://review.openstack.org/614167
15:55:39 openstackgerrit Maciej Jozefczyk proposed openstack/nova master: Add fill_virtual_interface_list online_data_migration script https://review.openstack.org/614167
16:11:26 openstackgerrit Eric Fried proposed openstack/nova master: DNM: Trust the report client cache more https://review.openstack.org/614886
16:33:03 efried stephenfin: Does :oslo.config:option: not work in renos yet?
16:33:55 stephenfin efried: https://github.com/openstack/nova/blob/master/doc/source/conf.py vs. https://github.com/openstack/nova/blob/master/releasenotes/source/conf.py
16:34:11 stephenfin efried: It's a different build. You'd need to configure it for reno
16:34:22 stephenfin But I strongly advise against doing so
16:34:31 efried stephenfin: ack. I think I don't care that much.
16:34:36 efried why would you advise against it?
16:34:49 stephenfin If we removed/renamed the option in the future, we'd break the build
16:35:05 efried as we should

Earlier   Later