| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-29 | |||
| 19:57:17 | sean-k-mooney | ya | |
| 19:57:25 | melwitt | I thought you were suggesting increasing the idle timeout as a way of allowing threads > 1 to work | |
| 19:57:27 | melwitt | ok | |
| 19:57:35 | aspiers | efried: I finally found MAX_INT in placement.db.constants, but that's not available from the unit testing testenvs | |
| 19:57:55 | efried | aspiers: Swear there was somewhere else we were using that. | |
| 19:57:56 | aspiers | using nova.db.constants for now | |
| 19:58:01 | aspiers | but that's not really correct | |
| 19:58:06 | aspiers | even if they happen to be the same value | |
| 19:58:41 | aspiers | efried: it's available to functional testenvs, but I need it in one unit test also | |
| 19:58:51 | melwitt | mriedem: I can take a stab at the stein reno and then have sean-k-mooney correct me because sean knows more than I do about it at this point | |
| 19:58:52 | efried | aspiers: You need it in your code too | |
| 19:59:10 | sean-k-mooney | aspiers: you could just use 2**64 or 2**32 | |
| 19:59:27 | aspiers | efried: well sure, but presumably it's safe to use placement from nova code? | |
| 19:59:34 | efried | not at all | |
| 19:59:41 | aspiers | huh | |
| 19:59:58 | sean-k-mooney | aspiers: we cant import random parts of placement in nova | |
| 20:00:04 | aspiers | OK | |
| 20:00:14 | efried | Right. Placement, from nova's perspective, is supposed to be purely a REST interface. Absolutely nothing else. | |
| 20:00:25 | efried | I think we're importing a fixture to make some testing easier | |
| 20:00:36 | efried | but prod code should be purely REST touchpoints. | |
| 20:00:38 | aspiers | Seems totally reasonable to me if it exported constants at least, if nothing else | |
| 20:00:48 | efried | yeah, I could get behind that in theory. | |
| 20:00:56 | efried | There has been talk of making a placement | |
| 20:01:00 | efried | placement-lib project | |
| 20:01:01 | sean-k-mooney | why not just creat the constant in nova | |
| 20:01:12 | melwitt | sean-k-mooney: aside, I wonder did we ever get this sorted downstream in the bz's now that we know we should _not_ disable heartbeats | |
| 20:01:18 | aspiers | sean-k-mooney: like I said, I'm just using nova.db.constants.MAX_INT | |
| 20:01:21 | sean-k-mooney | if its a constant its not going to change often one would hope | |
| 20:01:23 | efried | which would contain things like os-traits and os-resource-classes as well as constants and test fixtures. | |
| 20:01:45 | artom | Wouldn't test fixtures go into oslo_test? | |
| 20:01:57 | efried | not placement test fixtures | |
| 20:01:58 | sean-k-mooney | melwitt: i think martin was going to try testing it with heatbeats disabel to see what happens | |
| 20:02:09 | aspiers | nova.db.constants.MAX_INT isn't technically correct, but it's so damn close that I think it's good enough | |
| 20:02:28 | efried | aspiers: Yeah, was gonna say, as long as you use something you're sure is <= what placement can handle, "arbitrarily large" could be pretty much anything. | |
| 20:02:34 | artom | 31337 :D | |
| 20:02:36 | efried | You could hard code it to 2000 and that would probably be fine. | |
| 20:03:09 | aspiers | efried: actually answering that might breach an NDA ... | |
| 20:03:18 | melwitt | sean-k-mooney: hm... well, it will work on the surface but the issues dansmith pointed out were that rabbit will leave stale connections lying around in that case, so overall it's not a good idea to disable. hopefully martin will test that out as part of it | |
| 20:03:59 | sean-k-mooney | oh i did not know about the stale connection issue? | |
| 20:04:28 | aspiers | artom: probably safer than 1337 ;) | |
| 20:04:35 | sean-k-mooney | melwitt: we we ar confident we should not disable heatbeats then we can save martin time and do? | |
| 20:04:58 | aspiers | artom: which was one of the codes in one of the Denver escape rooms XD | |
| 20:05:07 | artom | Noice. | |
| 20:05:13 | sean-k-mooney | nothing? supress log? make ideal time 5 minuets for mod_wsgi? | |
| 20:06:36 | sean-k-mooney | i think the current situation is more or less lets figure out how to fix it upstream and then backport that fix upstream and downstream | |
| 20:07:36 | melwitt | sean-k-mooney: yes, based on the fact that rabbit will not clean up stale connections in that case, I don't think we should disable it. admittedly I don't understand a ton about this, but the docs recommend against disabling unless tcp keepalives are used in their place https://www.rabbitmq.com/heartbeats.html#disabling | |
| 20:08:03 | sean-k-mooney | we do use tcp keepalives | |
| 20:08:17 | sean-k-mooney | and we set it to like 1 second intervals | |
| 20:08:58 | sean-k-mooney | triplos defalts are pretty agressive in trying to keep the connection alive | |
| 20:09:22 | melwitt | the only possible thing we could do with the logging, if oslo.messaging ppl are agreeable, would be to downgrade the "connection reset" message to WARNING and then set our log level to ERROR to hide it | |
| 20:09:47 | sean-k-mooney | melwitt: i was looking it python log filters a bit too | |
| 20:09:57 | melwitt | and then obviously we have to have threads=1 | |
| 20:10:18 | sean-k-mooney | i think we could intall a logfilter that would match on the oslo meesge and drop them too if oslo do not want to change the level | |
| 20:10:38 | mriedem | melwitt: ok, i'm torn between waffling over a reno on the issue and figuring out just the right words vs releasing what we have (which is quite a bit unreleased in stable/stein) and just letting the reno happen later if at all | |
| 20:10:40 | melwitt | oh, I see | |
| 20:10:47 | mriedem | b/c i'm not an authority to review that reno | |
| 20:11:23 | melwitt | yeah, that's fair | |
| 20:11:49 | melwitt | we don't need to do a release to get the reno out there right? it will just get published to the docs automatically | |
| 20:11:51 | sean-k-mooney | mriedem: i dont think the reno is critcal jsut nice to have and sice we are still debating what the best solution is im not sure we should hold up the release for it | |
| 20:15:48 | melwitt | sean-k-mooney: ok, well, I guess according to that doc it's maybe probably ok to disable heartbeats if we have tcp keepalives in place https://www.rabbitmq.com/heartbeats.html#tcp-keepalives I think either way we'd want thorough testing of it before we go ahead and put the change out there | |
| 20:19:26 | sean-k-mooney | melwitt: this is what triplo configures https://github.com/openstack/tripleo-heat-templates/blob/d90434187315e96d3ebc184866dcd91954c0471d/deployment/kernel/kernel-baremetal-puppet.yaml#L117-L122 | |
| 20:21:48 | sean-k-mooney | it looks like that is even more agressive then rabbitmqs suggestions https://www.rabbitmq.com/networking.html#tcp-keepalives | |
| 20:22:30 | melwitt | mriedem: ack | |
| 20:22:49 | melwitt | oops | |
| 20:22:51 | melwitt | sean-k-mooney: ack | |
| 20:22:55 | sean-k-mooney | :) | |
| 20:25:39 | melwitt | mriedem: I'm fine either way releasing the reno now vs "soon" since AFAIK it doesn't have to be connected with the release, reno gets published independently. I was mostly saying I think we should publish a known issue vs not | |
| 20:26:11 | mriedem | i removed the -WIP so the release gods can do as they please | |
| 20:26:25 | melwitt | k | |
| 20:52:10 | sean-k-mooney | fun fact. resize between flavor with different size hugepages is broken | |
| 20:53:01 | sean-k-mooney | mainly becasue the nova.virt.hardware modeuld does not know what a rebuild or resize is and we end up using the page size form the embed instance numa toplogy instead of the new flavor/image | |
| 21:14:41 | melwitt | sean-k-mooney: reading through the full comments on https://bugs.launchpad.net/nova/+bug/1829062 it doesn't look like it's the same issue as the pause/resume rabbit reconnecting thing we were talking about | |
| 21:14:43 | openstack | Launchpad bug 1829062 in StarlingX "nova placement api non-responsive due to eventlet error" [Critical,In progress] - Assigned to Gerry Kopec (gerry-kopec) | |
| 21:15:30 | mriedem | melwitt: replies to your replies in https://review.opendev.org/#/c/638073/31 but +W - some of the stuff in there could go into your FUP | |
| 21:15:31 | melwitt | I would expect their wsgi config is default threads=1 | |
| 21:16:39 | melwitt | mriedem: yaaaaaay thanks. will add to the FollowUP | |
| 21:16:53 | sean-k-mooney | melwitt: the fact that its complaining about resuming on a different thread implies they have treads!=1 | |
| 21:17:55 | sean-k-mooney | but part of the annoyance it that placmeent has never used eventlet and we only started monkeypatching it in stien after extracting placment | |
| 21:18:06 | melwitt | sean-k-mooney: o. yeah | |
| 21:18:30 | sean-k-mooney | so people running runnign placment under the non extract version are running a config that we really have not tested well at all | |
| 21:18:41 | sean-k-mooney | since the gate uses extracted placement | |
| 21:19:04 | melwitt | right | |
| 21:19:25 | melwitt | ok, I'll throw another comment onto that bug to clarify the wsgi config thing | |
| 21:19:47 | melwitt | I'm trying to write the reno also | |
| 21:19:52 | sean-k-mooney | placement is written assuming eventlet is not used so there might be dragons there wen run under eventlet in general | |
| 21:20:03 | sean-k-mooney | ack | |
| 21:20:22 | melwitt | this whole issue makes me :*****( | |
| 21:21:02 | melwitt | have two paragraphs so far | |
| 21:21:09 | sean-k-mooney | there are too many ways to fix it all of which suck for different reasons | |
| 21:25:19 | mriedem | tonyb: this is all yours https://review.opendev.org/#/c/655692/ | |
| 21:26:06 | tonyb[m] | mriedem: Okay I | |
| 21:26:14 | tonyb[m] | 'll look after the school run | |
| 21:27:03 | aspiers | efried: you still around? | |
| 21:27:10 | efried | y | |
| 21:27:22 | aspiers | turns out that placement's inventory policy hardcodes a minimum of 1, not 0, for resource class inventory :-/ | |
| 21:27:35 | aspiers | for MEM_ENCRYPTION_CONTEXT, this is clearly a problem | |
| 21:28:14 | efried | why? | |
| 21:28:23 | aspiers | there are even explicit tests for this https://opendev.org/openstack/placement/src/branch/master/placement/tests/functional/gabbits/allocations.yaml#L107 | |
| 21:28:37 | aspiers | because hosts which can't do SEV need to be inventoried as 0 units | |
| 21:29:02 | efried | the test you pointed to is for an *allocation* requesting zero units of a thing. | |