| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-27 | |||
| 15:29:23 | cdent | cfriesen: is missing events of that nature a thing happens a) often, b) sometimes, c) rarely? | |
| 15:29:23 | melwitt | cdent: looking back at the code to refresh my memory ... I think without the periodic clear, the only thing that removes instances from tracked_instances is the vm_state DELETED or SHELVED_OFFLOADED and if for some reason (errors) an instance doesn't get its state saved that way, it would never be removed | |
| 15:32:29 | cfriesen | cdent: I would expect rarely | |
| 15:33:54 | cfriesen | melwitt: abort_instance_claim() also removes it, by calling _update_usage_from_instance(is_removed=True) | |
| 15:34:10 | melwitt | yeah, sorry. I forgot to mention that too | |
| 15:34:12 | cfriesen | melwitt: cdent: but I think melwitt is correct that there would be cases we'd need to add explicit removal | |
| 15:34:57 | cdent | at least over the long run, making things more explicit would probably be better | |
| 15:35:29 | cfriesen | cdent: agreed, and it would make the resource tracking react faster to freeing up of resources. | |
| 15:40:54 | johnthetubaguy | cdent: at somepoint doesn't nova-conductor create the instance resource allocations, and retries if the generation id of the resource provider doesn't match, (as generation id increments on a related allocation, or something like that)? So the resource tracker is more just checking for rogue instances with missing allocations? | |
| 15:41:34 | cfriesen | so hardware.numa_usage_from_instances() expects a list, but we only ever call it with a single instance. Do we plan on ever using it with a list or should we maybe clean it up to expect a single instance? | |
| 15:42:29 | cdent | johnthetubaguy: when you say "allocations" do you mean allocations in the placement api, or something more general? because if the former, the conductor doesn't talk to the placement api at all. | |
| 15:43:18 | johnthetubaguy | cdent: I mean placement API, and when I say conductor, that happens in nova-scheduler today | |
| 15:45:06 | cdent | johnthetubaguy: as far as I know, right now, the only thing that writes allocations is the resource tracker | |
| 15:45:19 | cdent | and the only thing that writes inventories is the resource tracker | |
| 15:45:28 | cdent | when nova-scheduler talks to placement API it is read only | |
| 15:47:42 | johnthetubaguy | cdent: I am thinking about the (not too distant) future, as the sooner we add the allocations, the sooner other queries "notice" them | |
| 15:48:10 | johnthetubaguy | but thats probably just a distraction right now | |
| 15:48:12 | cdent | oh you mean "at some point won't it be the cast that...". | |
| 15:48:33 | cdent | Yes, at some point the scheduler will request and claim resources (thus make allocations) in one go | |
| 15:48:51 | cdent | probably with a POST to /allocations of something resembling a request spec | |
| 15:49:09 | cdent | instead of the curerrent GET /resoruce_provdiers (which then passing into existing filter scheduler) | |
| 15:49:30 | cdent | when that time comes, the resource tracker will change quite a bit, presumably | |
| 15:49:39 | cdent | but I suspect that that time is probably further off than we think | |
| 15:49:44 | cdent | s/think/hope/ | |
| 16:03:30 | johnthetubaguy | cdent: fair point, just wondering if we can make baby steps in that direction, while we still have the middle ground | |
| 16:06:11 | cdent | johnthetubaguy: I'm not super familiar with the existing claim handling, so I'm too clear on what the options are for steps | |
| 16:08:59 | openstackgerrit | Chris Dent proposed openstack/nova: Add more debug logging on RP inventory update failures https://review.openstack.org/414230 | |
| 16:09:00 | openstackgerrit | Chris Dent proposed openstack/nova: Add more debug logging on RP inventory delete failures https://review.openstack.org/426290 | |
| 16:16:50 | superdan | sdague: are you back today? | |
| 16:16:57 | sdague | superdan: yeh | |
| 16:17:35 | superdan | sdague: you wanna look at the mess I've made here? https://review.openstack.org/#/c/424730/ | |
| 16:17:53 | superdan | I think the wait for services is probably a good thing we want to avoid a race with the subnode checking back in, | |
| 16:18:03 | superdan | although I don't think it's related to the problem we were trying to solve | |
| 16:18:08 | superdan | and I figure that should be pulled out | |
| 16:18:12 | superdan | to a separate patch | |
| 16:18:30 | superdan | but, the configuring/starting of the placement stuff would be good to validate if you could | |
| 16:21:00 | sdague | yeh, it seems like that should be separate, and I do agree, based on how long all these phases take, I seriously doubt we'll have a practical issue there | |
| 16:21:05 | cdent | anyone: probably too late but: CORS for placement had a +2 on it a while back and there was some support for getting it in as part of ocata (probably not a huge deal if it doesn't happen): https://review.openstack.org/#/c/392891/ | |
| 16:21:40 | sdague | superdan: I saw this patch briefly the other day, how about we bite the bullet now and make a 55_placement top level construct and do the placement service enablement and start there | |
| 16:21:43 | superdan | sdague: it would specifically catch the situation where we introduce a thing that broke the subnode entirely and call it out, instead of just trying to decipher it from the failed tempest run, at least | |
| 16:22:18 | sdague | superdan: sure, but has that shown up yet? | |
| 16:22:47 | superdan | sdague: meaning have we ever broken the subnode? | |
| 16:23:40 | superdan | sdague: at one point the service version code had a bug where the subnode couldn't check back in because its service record had timed out and it was told it couldn't check back in and regress the minimum version, yeah | |
| 16:23:40 | sdague | superdan: in this flow right now | |
| 16:23:55 | superdan | sdague: and it took a while to realize that the subnode was running, but never being sent instances | |
| 16:24:14 | sdague | I get the idea that it can happen, but lets not mix up problems being solved here | |
| 16:24:18 | superdan | sdague: unrelated to this flow, although this flow can definitely race with the subnode checking back in after the upgrade, I just haven't seen it happen yet | |
| 16:24:41 | sdague | we can always put a check like this back in later if it's happening at any frequency, and even backport it | |
| 16:24:57 | superdan | cdent: mriedem: scheduling patch just passed multinode grenade | |
| 16:25:00 | superdan | johnthetubaguy: ^ | |
| 16:25:26 | cdent | amazeballs | |
| 16:25:35 | superdan | sdague: it just doesn't seem responsible to not check for and catch a thing we know can happen when it's trivial to do, but.. the fight is all gone from me this week | |
| 16:26:26 | sdague | superdan: I just want to fix one thing at a time here and get people unblocked. We can fix these other things later. | |
| 16:26:47 | superdan | sdague: I'm not saying that has to be in front of this necessarily | |
| 16:26:48 | mriedem | superdan: how is that possible | |
| 16:26:53 | superdan | mriedem: cosmic rays | |
| 16:27:07 | sdague | because it's not like you'll get the wrong test results if it happens, it's just that figuring out what went wrong takes a little longer | |
| 16:27:14 | superdan | mriedem: cdent johnthetubaguy I will get a bite to eat and then do the grenade refactor sdague is asking for | |
| 16:27:23 | superdan | there's also pep8 and unit fails in the nova side patch | |
| 16:27:46 | johnthetubaguy | superdan: cool, I can roll back the patch so the hacks are removed, and the tests are passing? | |
| 16:27:54 | mriedem | jroll: throwaway hack yes please | |
| 16:27:54 | superdan | sdague: it does mean we run another 50m of tests that we know will fail instead of just bailing early, but yeah | |
| 16:28:16 | superdan | johnthetubaguy: remove the logging and such, but keep the *correct* min ver check yeah :) | |
| 16:28:26 | johnthetubaguy | cdent: sorry, I meant to ask you, did you have any more thoughts on this one: https://review.openstack.org/#/c/404472 | |
| 16:28:36 | johnthetubaguy | superdan: thats my *aim* anyways | |
| 16:28:46 | sdague | superdan: sure, but that makes it an optimization we can fix later, even deep in the freeze, once we see how often it happens | |
| 16:28:49 | superdan | johnthetubaguy: I wish you luck sir | |
| 16:29:12 | cdent | superdan, johnthetubaguy is the issue with making sure the service cache gets updated after each compute node update suitably okay? are we assuming the scheduler will be restarted every time some uprades a compute node? | |
| 16:30:11 | cdent | johnthetubaguy: sorry, I started looking and got distracted by a merge conflict on some logging fixes and lost my place. Is there a summary of what we're trying to resolve? | |
| 16:30:17 | johnthetubaguy | cdent: I was thinking you needed to SIG_HUP for the RPC API change anyways, but it might be the first time | |
| 16:30:27 | johnthetubaguy | cdent: the -1 comment on the patch should do it | |
| 16:30:32 | superdan | cdent: that's the assumption that my grenade service check ensures is safe, and which sdague wants to wait on since we haven't seen it yet, even though it's 100% possible | |
| 16:30:41 | cdent | sig hup won't do it, as there's no reset handler? | |
| 16:30:50 | superdan | cdent: so I'll pull that out and do it separately and if we start to break the world after this merges, we'll have it ready | |
| 16:30:56 | johnthetubaguy | cdent: basically, seems like the instance allocations will fail for ironic nodes that have resource_classes defined | |
| 16:31:16 | superdan | cdent: correct, there's no reset handler, which I did ask for, but we can do after as well, and won't help the gate anyway | |
| 16:31:24 | johnthetubaguy | cdent: ah, I missed that | |
| 16:31:33 | johnthetubaguy | superdan: +1, but lets keep that separate | |
| 16:31:40 | cdent | superdan: k, I wasn't think in terms of the gate, but you know, like real people ;) | |
| 16:32:52 | superdan | johnthetubaguy: yeah, I was going to pull it out regardless, but I was going to keep it in the path of this set, but I won't now | |
| 16:33:00 | superdan | cdent: yes, which is why I asked for the reset thing initially | |
| 16:34:14 | cdent | johnthetubaguy: tacking back to that allocations thing now | |
| 16:36:42 | mriedem | is someone fixing up the nova side scheduler change? because if they are, can they address the exception wording comment in https://review.openstack.org/#/c/425806/ ? | |
| 16:37:47 | johnthetubaguy | cdent: ack | |
| 16:38:06 | mriedem | snikitin: are you saying the diagnostics patches were all approved yesterday before nit fixing? | |
| 16:38:10 | mriedem | or just had +2s on them? | |
| 16:38:44 | openstackgerrit | Jim Rollenhagen proposed openstack/nova: DNM: hack ironic with resource providers https://review.openstack.org/426296 | |
| 16:38:49 | jroll | mriedem: enjoy | |
| 16:41:07 | mriedem | jroll: cool hopefully i can load that sometime today | |
| 16:41:24 | jroll | heh | |
| 16:48:53 | sdague | cdent: there are many ways that things can go wrong. My biggest concern is that we don't have a failed software stack that provides a passing result. As long as we don't have that, our cost is debug time and time until test completion. | |
| 16:49:33 | sdague | because we have a specific test that attempts to schedule to all nodes, and a min node count, we have one test that points in the right direction already, so debug time isn't hugely negatively impacted | |
| 16:49:46 | sdague | and test run time until failure is basically +15m here | |
| 16:49:47 | cdent | sdague: I'm not sure which of several context to put that response in? | |
| 16:50:01 | figleaf | johnthetubaguy: can you summarize the sticking points in https://review.openstack.org/#/c/404472 for your -1? | |
| 16:50:29 | cdent | johnthetubaguy: are you concerned about the situation of old style ironic allocations being deleted but then nothing allocation the new style? I'm still confused (apparently like figleaf too) | |
| 16:50:44 | sdague | cdent: about why I said don't both with this - https://review.openstack.org/#/c/424730/12/projects/60_nova/resources.sh@136 at the moment | |
| 16:51:17 | cdent | sdague: I hadn't made any comments with respect to that? | |
| 16:51:37 | sdague | cdent: ok, well a bunch of the superdan cross comments were about that | |
| 16:51:45 | johnthetubaguy | cdent: figleaf: the bit I don't understand, is who make allocations for ironic instances, making use of the new resource class | |