| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-09 | |||
| 14:28:17 | kashyap | lyarwood: Will update the change with the three ways, and pros/cons | |
| 14:28:21 | mriedem | elod: ok thanks | |
| 14:30:45 | lyarwood | kashyap: ah I wasn't aware that you could rebase disks between backing files, that would be super useful here. | |
| 14:30:51 | lyarwood | kashyap: thanks! | |
| 14:31:37 | kashyap | lyarwood: Yes, indeed. By default 'qemu-img rebase' does a "real rebase" option. And 'qemu-img rebase -U' -- "unsafe" -- will only update the backing file | |
| 14:31:57 | kashyap | The man page explains it. /me recalls documenting that part of 'qemu-img' many moons ago :-) | |
| 14:32:14 | kashyap | (Read the Safe mode vs. Unsafe mode discussion) | |
| 14:36:11 | aarents | mdbooth: kashyap Hi, basicly by using convert, I reused stuff done when we extract a snapshot to glance (we need to flaten before upload) | |
| 14:42:43 | kashyap | aarents: Hi, there are a couple of trade-offs here, based on the method we're using | |
| 14:43:38 | kashyap | 'convert' has the advantage of also handling sparsification (as guessed earlier); but has the disadvantage of being relatively slow _and_ requires double the space | |
| 14:44:52 | kashyap | aarents: Slow because, 'convert' copies both base and overlay into a new image; and thus double the space. (Instead of copying only into base or the overlay, in case of 'commit' or 'rebase') | |
| 14:45:23 | kashyap | aarents: Writing a further comment in the change, once we have the options laid out, then we can make a decision | |
| 15:00:21 | aarents | kashyap: ok great | |
| 15:18:17 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Follow up to I5b9d41ef34385689d8da9b3962a1eac759eddf6a https://review.opendev.org/698028 | |
| 15:18:36 | kashyap | mdbooth: aarents: lyarwood: Added comparison notes of the three possible ways we can take: https://review.opendev.org/#/c/696084/ | |
| 15:20:54 | efried | slaweq: catching up... | |
| 15:23:18 | efried | slaweq: Any time I see spurious grenade fails in the last couple months, I attribute it to oversubscribed CI nodes, per mriedem's "State of the Gate" thread, started here http://lists.openstack.org/pipermail/openstack-discuss/2019-October/thread.html#10484 and continued here http://lists.openstack.org/pipermail/openstack-discuss/2019-November/thread.html#10502 | |
| 15:24:48 | efried | slaweq: and per the first note in that thread, yes, the bug you identified (bug 1844929) is the one we're "tracking" the issue with. | |
| 15:24:48 | openstack | bug 1844929 in OpenStack Compute (nova) "grenade jobs failing due to "Timed out waiting for response from cell" in scheduler" [High,Confirmed] https://launchpad.net/bugs/1844929 | |
| 15:26:10 | efried | ...and the right solution is to get the CI providers to tweak their environments accordingly. We would rather have lower job throughput and lower failure rates. | |
| 15:26:18 | efried | But so far there has been no reaction from them. | |
| 15:26:45 | mriedem | note that for that particular bug the vast majority of fails are on ovh nodes | |
| 15:27:33 | mriedem | i don't know why it's mostly on grenade jobs | |
| 15:27:58 | mriedem | must have something to do with restarting mysql a few times, but idk | |
| 15:29:00 | mriedem | note that it also started with train, i don't know why though | |
| 15:29:12 | mriedem | tl;dr i don't really know much of anything | |
| 15:33:26 | sean-k-mooney | do we also restart rabbitmq during the grenade upgrade. i assume so but i have not checked | |
| 15:33:58 | sean-k-mooney | it is porably restarted at least once by devstack on the second stacking | |
| 15:35:11 | sean-k-mooney | i noticed that the subnode hand messaging timeouts in both the n-cpu and q-agt services | |
| 15:35:57 | sean-k-mooney | so it looked like we lost messages or something | |
| 15:45:14 | slaweq | mriedem: efried thx for confirmation that this is the bug which we hit most of the times now | |
| 15:45:18 | efried | dansmith: it would seem that the 422 event code is masked when there's only one event (as is the case with cyborg) | |
| 15:45:25 | slaweq | sean-k-mooney: and thx for info about rabbitmq too | |
| 15:45:36 | efried | slaweq: It would help to get some more voices complaining at the node providers. | |
| 15:45:54 | dansmith | efried: really? I didn't see that condition | |
| 15:45:58 | efried | dansmith: when all events are dropped, the handler turns the whole thing into a 404 | |
| 15:46:06 | efried | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_external_events.py#L146 | |
| 15:46:41 | efried | which is arguably a bug | |
| 15:46:44 | dansmith | efried: ah, yeah, that code isn't really right.. "no instances found for any event" isn't true, just no hosts.. | |
| 15:46:46 | dansmith | yeah | |
| 15:47:05 | efried | to "fix" the bug would be a microversion? | |
| 15:47:07 | dansmith | unless mriedem feels that's a microversion problem, I can change that | |
| 15:47:31 | dansmith | I dunno, it's correcting the error code, which I thought was allowable in some situations | |
| 15:47:51 | efried | what would you correct it to? 207? | |
| 15:48:26 | efried | Meaning there's actually no way the API ever returns an error. | |
| 15:48:28 | dansmith | that's what we return per-event in that case right? | |
| 15:48:32 | dansmith | no, there is | |
| 15:48:42 | dansmith | if the instance itself is not found | |
| 15:48:50 | efried | instances plural, right? | |
| 15:49:21 | dansmith | yeah | |
| 15:49:22 | efried | so if some of the instances are not found, that's a 207 with some 404s | |
| 15:49:32 | efried | but if none of the instances are found, that's an overall 404? | |
| 15:50:28 | efried | or, | |
| 15:50:28 | efried | If all events tank, 400 with the granular payload | |
| 15:50:28 | efried | If some events succeed, 207 with the granular payload | |
| 15:50:51 | efried | or, | |
| 15:50:51 | efried | If all events tank for the same reason, make that the overall status code | |
| 15:51:10 | efried | otherwise use 400 or 207 | |
| 15:51:33 | dansmith | it really should be that if everything was a 404, then you get overall 404. otherwise it's 207 with granular statuses right? | |
| 15:51:58 | dansmith | or maybe if everything is 200, you return 200 as well, I dunno | |
| 15:52:34 | efried | I'm not offended by the idea that you always return 207 | |
| 15:52:56 | efried | the definition of 207 allows for it to be full failure situations | |
| 15:53:02 | efried | "The response MAY be used in success, partial success and also in failure situations." | |
| 15:54:25 | dansmith | yeah, I think the only reason not to do that is just that things like response.raise_for_status() isn't usable for a dumb cleint | |
| 15:54:26 | efried | If clients are properly coded, that should be an acceptable behavior. But I suspect existing clients are written to how the code is currently mad. | |
| 15:54:56 | efried | well, a client coded to pass blindly on 207 is buggy, period. | |
| 15:55:19 | efried | Sundar: o/ We're just discussing the bug you identified in the server_external_events algo, and how to fix it. | |
| 15:55:21 | dansmith | well, sure, but lots of code does "if s<300: continue" | |
| 15:55:36 | dansmith | efried: anyway, I'm really fine with 207 across the board if others are, | |
| 15:55:42 | dansmith | I'm just arguing what I think some people would argue | |
| 15:55:45 | efried | yes, I know, I'm saying code that does that with a response from this particular API is wrong, before or after the fix. | |
| 15:55:55 | dansmith | very few other things use this interface and they're really all openstack projects | |
| 15:56:01 | dansmith | sure | |
| 15:56:18 | efried | so, 200 if all are green, 207 if any/all fail. I'm happy with that. | |
| 15:56:44 | efried | which basically just means removing https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_external_events.py#L149-L151 | |
| 15:57:24 | efried | dansmith: I can work that up. Unless you're already doing it. | |
| 15:57:49 | dansmith | efried: I'm not, but I'd also like to hear mriedem say he's okay with it | |
| 15:58:07 | efried | okay. I'll put it up so we can talk about it around the patch. | |
| 15:58:24 | dansmith | sure | |
| 16:04:11 | mriedem | doesn't sound crazy to me, and it's an admin-only api by default | |
| 16:04:22 | efried | cool, forthcoming... | |
| 16:04:35 | mriedem | gmann and alex_xu can pounce on you | |
| 16:04:50 | mriedem | you'll want a bug either way i think | |
| 16:05:18 | efried | Sundar: care to open that, since you uncovered this? | |
| 16:05:37 | mriedem | https://docs.openstack.org/nova/latest/contributor/microversions.html#when-do-i-need-a-new-microversion will be noted by someone (surely not me just now) | |
| 16:06:39 | mriedem | i think it probably falls into this category a bit "Fixing a bug so that a 400+ code is returned rather than a 500 or 503 does not require a microversion change. It’s assumed that clients are not expected to handle a 500 or 503 response and therefore should not need to opt-in to microversion changes that fixes a 500 or 503 response from happening." | |
| 16:06:58 | mriedem | 207 is returned rather than 404 - and this is likely the only api we have that returns 207 | |
| 16:07:30 | mriedem | this probably hasn't come up before either since we don't have events coming in on instances that don't have a host (i don't think anyway - maybe shelve offload?) | |
| 16:09:42 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Nix os-server-external-events 404 condition https://review.opendev.org/698037 | |
| 16:09:51 | dansmith | well, in those cases, they're likely firing and ignoring the status or getting a different error code than they're really expecting, | |
| 16:09:54 | efried | Ima let zuul tell me which tests to fix --^ | |
| 16:09:56 | dansmith | so it doesn't seem likely to affect anyone | |
| 16:10:51 | sean-k-mooney | if you did a neutorn port update on a shelve offload intance it would trigger a network changed event so ya that might be one case | |
| 16:10:58 | openstackgerrit | Stephen Finucane proposed openstack/os-traits master: Add COMPUTE_SAME_HOST_COLD_MIGRATE trait https://review.opendev.org/666604 | |
| 16:11:03 | Sundar | efried: I will get back in ~30min - 1 hour, since I have some personal matters to attend now. Sorry. | |
| 16:11:22 | mriedem | yeah 404 isn't really appropriate - that's if we didn't find any instances, but clearly we can but they don't have hosts | |
| 16:12:10 | efried | mriedem: as noted above, we could arguably keep 404 for "none of the instances were found" | |
| 16:12:18 | mriedem | yeah that's fine | |
| 16:12:19 | efried | but even that's kind of weird. | |
| 16:12:42 | efried | would we then do an overall 422 if none of the instances were mapped to hosts? | |