| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-06 | |||
| 13:41:45 | mriedem | dtantsur: is this the job we care about? ironic-grenade-dsvm-multinode-multitenant | |
| 13:41:55 | dtantsur | lemme double-check | |
| 13:42:24 | mriedem | that's the only multinode grenade job i see in zuul on that change | |
| 13:42:28 | dtantsur | mriedem: oh, it seems to be a normal (non-grenade) multinode job: ironic-tempest-ipa-wholedisk-direct-tinyipa-multinode | |
| 13:43:01 | dtantsur | although I did see something similar on one or two grenade runs, but they don't seem consistently broken | |
| 13:43:01 | mriedem | ack | |
| 13:43:13 | mriedem | maybe randomly picking the correct node | |
| 13:43:18 | dtantsur | yep | |
| 13:43:24 | dansmith | mriedem: mdbooth decided to talk about some details about that patch downstream, so let me fill you in | |
| 13:44:04 | dansmith | mriedem: there are (suddenly?) two customer cases he thinks are caused by this issue, yet we apparently have no confirmation from either see a problem solved by applying this | |
| 13:44:20 | dansmith | mriedem: one clearly isn't fixed by it, and we don't have logs to show why | |
| 13:45:02 | alex_xu | stephenfin: thanks, actually I already go through that patch, I didn't found problem, I'm just waiting for huaqiang to do some manual test, then I will go through all the left patches and +2 | |
| 13:45:03 | dansmith | mriedem: so while the rationale for making the change seems fairly legit to me, I don't know if (a) that one place was never wrapped for some reason or (b) whether changing that is going to cause other thread interaction or timing-related problems | |
| 13:46:44 | dansmith | mriedem: so I'm not sure what to say... we could just merge it and watch carefully and hope that any negative impact is not in two years when people actually roll to it, as it seems likely to be just an oversight | |
| 13:46:59 | yonglihe | mriedem: sorry this is a barrier, i did not bring back my laptop today, and stephenfin, thanks. | |
| 13:47:01 | dansmith | but it definitely makes me nervous not having any "yes this fixes my problem" confirmation for code that has been around for ten years | |
| 13:47:17 | mriedem | dansmith: an oversight that we don't know if it has caused problems until now, though that code hasn't changed in a long time | |
| 13:47:26 | stephenfin | alex_xu: Huzzah. Good to hear (y) | |
| 13:47:42 | mriedem | dansmith: any chance of some weird eventlet patches in these customer environments? | |
| 13:47:51 | mriedem | so it would be rhosp specific? | |
| 13:48:04 | mriedem | or python-libvirt patches, something external to nova? | |
| 13:48:15 | dansmith | mriedem: I doubt it, but if they did and it was related, then they'd have been fixed by this (if that's what you mean) | |
| 13:48:21 | mriedem | also, if we land this on master, will backports be proposed upstream? | |
| 13:48:31 | dansmith | mriedem: they're probably on newer *libvirt*, but I dunno how often python-libvirt changes | |
| 13:49:01 | mriedem | i think you have to rebuild python-libvirt from whatever version of libvirt binary you're using | |
| 13:49:05 | mriedem | otherwise things get weird | |
| 13:49:32 | dansmith | mriedem: I dunno, ask mdbooth. I would expect he would want to | |
| 13:49:49 | mriedem | i remember we had issues in the past with running libvirt x in the gate jobs but python-libvirt was built against y and reporting y to nova, so we'd try to use features that weren't actually there | |
| 13:49:50 | dansmith | mriedem: right but that's just because the bindings are auto-generated I think | |
| 13:50:10 | mriedem | ok, so i'm less comfortable with backporting a thing that we're not sure fixes the issue | |
| 13:50:25 | mriedem | it's one thing to land on master and let it simmer until people get to it, | |
| 13:50:32 | mriedem | than to backport it and force it on people with otherwise stable environments | |
| 13:50:40 | dansmith | although we are right before FF, so not as much runway | |
| 13:51:01 | mriedem | rh probably just cares that it's merged on master and can backport internally is less of an issue yeah? | |
| 13:51:19 | mriedem | merge conflict risk on stable branches and such i know | |
| 13:51:24 | dansmith | maybe we could get efried to comment on the risk, and maybe cdent on the actual eventlet bits? | |
| 13:51:32 | dansmith | I'm not sure what context cdent has on eventlet, but it seems like sometimes he has a lot | |
| 13:51:44 | cdent | hello what? | |
| 13:51:49 | mriedem | efried isn't a stable branch maintainer though, or you just mean risk for FF? | |
| 13:51:49 | cdent | which thing? | |
| 13:52:01 | dansmith | mriedem: I mean for merging to master right now or in general yeah | |
| 13:52:10 | mriedem | cdent: https://review.opendev.org/#/c/677736/ | |
| 13:52:20 | mriedem | cdent: tl;dr old ass code, | |
| 13:52:40 | mriedem | suddenly 2 rhosp customers are seeing weird issues which mdbooth thinks are related to libvirt + python + eventlet, | |
| 13:52:41 | dansmith | (I wish I was working on things that *have* to merge by FF right now) | |
| 13:52:52 | mriedem | but there isn't a clear indication that patch fixes those problems | |
| 13:52:54 | openstackgerrit | Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616 | |
| 13:53:07 | aspiers | stephenfin: Your doc feedback should be addressed now ^^^ | |
| 13:53:16 | dansmith | mriedem: I see no indication that it fixes those problems.. I think the only indication is that it "seems wrong" and "these tests pass" | |
| 13:53:36 | mriedem | sure, so ruling it out i guess | |
| 13:53:53 | mriedem | i also need to investigate these fixes that are blocking the ironic gate atm | |
| 13:54:01 | mriedem | since i'm guessing ironic would like to merge code before FF | |
| 13:54:15 | aspiers | stephenfin: https://review.opendev.org/#/c/666616/47..48/nova/conf/libvirt.py | |
| 13:54:18 | donnyd | sean-k-mooney: Is the NUMA job still throwing errors? | |
| 13:54:34 | aspiers | stephenfin: BTW I've checked that the rst all renders OK | |
| 13:54:54 | sean-k-mooney | donnyd: i have not re run it | |
| 13:55:08 | sean-k-mooney | with the queue thats in the gate i was going to wiat until the weekend | |
| 13:55:23 | sean-k-mooney | donnyd: i can kick it off now if you like and see | |
| 13:55:30 | donnyd | sure | |
| 13:58:07 | sean-k-mooney | donnyd: there is a build stitting in the experimental pipeline now | |
| 13:58:16 | sean-k-mooney | donnyd: ill let you know how it goes | |
| 13:58:20 | donnyd | kk | |
| 13:58:29 | donnyd | where can i check its status? | |
| 13:58:39 | sean-k-mooney | http://zuul.openstack.org/status | |
| 13:58:47 | sean-k-mooney | filter by 679656 | |
| 13:59:10 | sean-k-mooney | currently all jobs are queued | |
| 13:59:47 | openstackgerrit | Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158 | |
| 14:00:15 | luyao | dansmith, efried: I updated the patch for 'resource' object, many thanks for your detailed comments. It helps me a lot. https://review.opendev.org/#/c/678448/17 | |
| 14:00:22 | cdent | mriedem, dansmith : I'd be inclined to think that the non-proxy original code was wrong and the fix is good and we've tended to get lucky. However, without any clear proof that presenting problem(s) is fixed by the change...hard to say. I would think merge to master and watch wouldn't be a bad thing. | |
| 14:00:29 | sean-k-mooney | i proably should have kicked off one of the later patces to run less tests | |
| 14:00:42 | sean-k-mooney | but i did want that to report back in anycase | |
| 14:00:51 | aspiers | mriedem: thanks for feedback; addressed - https://review.opendev.org/#/c/680158/4..5/nova/compute/api.py | |
| 14:01:29 | openstackgerrit | Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616 | |
| 14:01:56 | alex_xu | anyone can tell me...can we boot instance without allocation | |
| 14:02:22 | sean-k-mooney | am we should be able to start them. but create new instances no | |
| 14:02:50 | sean-k-mooney | e.g. you deleted the allocation in placmenet for an instance nova should still be able to start/stop the vm | |
| 14:03:26 | dansmith | cdent: specifically one customer clearly *still* has the problem after applying this patch | |
| 14:03:39 | cdent | ah | |
| 14:03:40 | cdent | well | |
| 14:03:52 | dansmith | cdent: but I agree that it *seems* like the change we'd want to make, I'd just like confirmation that it does something positive you know :) | |
| 14:04:02 | alex_xu | sean-k-mooney: probably we can before https://github.com/openstack/nova/blob/master/nova/tests/functional/test_nova_manage.py#L405 | |
| 14:04:03 | cdent | is there concrete evidence it is stuck in eventlet, or just that it is stuck? | |
| 14:05:01 | dansmith | cdent: there's some guru meditation that makes it look like it, but we don't get to control when that happens and thus it's pretty likely to always be blocking on some C call on a busy system I think | |
| 14:05:02 | artom | mriedem, FWIW, I've done an inital read-through of all your comments, and they all make sense, so I should have answers/fixes later today | |
| 14:06:16 | alex_xu | sean-k-mooney: that functional test ensure we can boot instance without allocation, but the goal is testing the heal_allocations. | |
| 14:06:40 | cdent | dansmith: terrific! Does it happen often? Therapuetic process kills to the rescue? | |
| 14:07:23 | dansmith | yeah, just nuke the compute service I think | |
| 14:07:24 | dansmith | and sometimes they can't reproduce it, like when we got on a call to watch :) | |
| 14:07:46 | dansmith | what's strange to me is that this code has been in place for ages and suddenly there are two customers hitting it simultaneously, but not fixed by this patch | |
| 14:07:53 | dansmith | just makes me highly skeptical :) | |
| 14:08:41 | cdent | yeah. eventlet, libc, libvirt changes all possible contributors | |
| 14:10:38 | melwitt | does anyone know anything about the nova.task_log table? we have customers whose nova.task_log table grows indefinitely and I found there's no methods for ever deleting task_log entries. does anyone know what people usually do about this? manually truncate the task_log table? what are these records for? is arbitrary truncation safe? | |
| 14:10:46 | mriedem | dtantsur: dustinc: efried: what do you think about these comments? https://review.opendev.org/#/c/680684/2 | |
| 14:10:54 | mriedem | the fix is fine, but wondering about the 'this shouldn't happen' case | |
| 14:13:31 | dtantsur | will check in a few minutes | |
| 14:13:42 | dtantsur | yeah, I was also unsure whether to log or to raise | |
| 14:15:05 | mriedem | alex_xu: with the caching scheduler yes it was possible to create an instance with no allocations in placement, | |
| 14:15:14 | mriedem | because the caching scheduler didn't use placement or GET /allocatoin_candidates | |
| 14:15:17 | mriedem | the caching scheduler was removed in stein, | |
| 14:15:24 | mriedem | but you can still load up out of tree scheduler drivers that may not use placement | |