Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-03
17:13:03 dansmith I'm not sure why it's confusing.. it makes sense to me
17:13:12 dansmith you did a delete of aggregate 2
17:13:18 dansmith can't remove aggregate 2 because it's not empty
17:13:45 dansmith oh, I see the confusing part "cannot remove host"
17:13:47 dansmith fair enough
17:15:45 mriedem http://logs.openstack.org/69/556669/11/check/nova-multiattach/b3195af/logs/screen-n-api.txt.gz#_Jun_22_14_30_55_677141
17:15:48 mriedem that's the failure
17:15:55 mriedem Unexpected exception in API method: ResourceProviderUpdateConflict: A conflict was encountered attempting to update resource provider b9342d94-3665-4da1-90c8-e48ab77e181e (generation 6): {"errors": [{"status": 409, "request_id": "req-fe8eea9d-ec9d-4128-9117-73a68e462394", "detail": "There was a conflict when trying to complete your request.\n\n Update conflict: Another thread concurrently updated the data. Please retry your u
17:15:55 mriedem e ", "title": "Conflict"}]}
17:16:20 efried just so
17:16:21 mriedem who wants to open the bug
17:16:28 efried Well, it's not a bug yet
17:16:31 dansmith so no retry around te mirror?
17:16:34 mriedem nope
17:16:43 efried it doesn't show up until we start gen checking aggs.
17:16:56 mriedem i read that as 'chicken eggs'
17:17:01 mriedem mmm
17:17:08 efried A rat snake has been eating our eggs in the coop.
17:17:21 efried I mean, the race is still there, we're just blowing away whatever the first one did with the second.
17:17:42 efried I hear you're supposed to put a light bulb in the nest box. They eat the bulb and... ouch.
17:17:44 mriedem add_host_to_aggregate does handle several other exceptions and log a warning and return
17:18:52 efried mm, we could add UpdateConflict in there, but that would just mask the problem (I had that objection to that try/except when this code went in iirc).
17:19:08 mriedem hmm, wtf
17:19:15 mriedem we don't pass a generatoin when updating the aggregates
17:19:22 efried mriedem: That patch does.
17:19:36 mriedem ah ok
17:19:52 efried Swhat I was saying earlier. This isn't causing failures except in that patch. (The ironic thing is different.)
17:20:11 efried (though related, and probably fixable by the same retry loop)
17:21:52 dansmith I'm not sure why being obsessive about the generation in reportclient would cause the api side to fail,
17:22:04 dansmith unless it's just that it generates a little more load and opportunity to collide with the api mirror
17:25:04 efried dansmith: Every time _update runs (so periodic and from instance_claim) it tries to sync placement. Part of that is hitting the method to push aggregates for the compute host, which is the same method the host agg sync runs. If _update thinks it needs to push *any* change associated with the compute node RP, and the host agg sync happens between when we retrieve the data and when we push it, we'll 409.
17:25:44 efried ...once we're checking gens on aggs, which is what this patch introduces.
17:25:54 dansmith okay so just a lot of bumping the agg generation yeah?
17:26:45 efried dansmith: update_from_provider_tree is smart enough to invalidate the cache entry for the provider that 409s, so the next time around it'll re-GET and have the new generation. But up to this point, we're not doing that retry until the next periodic hits.
17:27:10 efried So from the pov of periodic, we're fine, it heals itself. But if the 409 happens in the instance_claim path, it can cause a resched/fail.
17:27:27 dansmith okay but the failure that breaks a test is the agg mirroring in the api
17:27:35 dansmith which is presumably because it doesn't retry on 409
17:27:39 dansmith what I'm wondering is,
17:27:40 efried right
17:27:53 dansmith why is the compute causing any update churn on the agg generation
17:27:57 dansmith ?
17:28:15 efried Don't we bump the RP gen whenever we allocate?
17:28:29 efried The agg doesn't have a generation. The RP does.
17:29:19 dansmith okay, so the 409 that the api is getting is that the RP it's trying to add to the agg has changed generations (and it doesn't retry)?
17:29:57 efried right
17:30:07 dansmith so what extra churn on the RP is the patch causing?
17:30:28 efried It's not causing extra churn. Before this patch, we weren't checking generations on agg updates - we were just blasting them in.
17:30:40 efried So we never got 409 - just whoever went second won.
17:30:50 dansmith right, but that's my point,
17:30:52 efried which is a bug
17:30:56 efried but it would be hidden
17:31:02 dansmith we're doing the same number of RP updates as before this patch?
17:31:03 efried checking gens exposes the bug.
17:31:05 efried yes
17:31:22 dansmith ...so what did it change that causes it to race and conflict with the api mirroring?
17:31:42 efried The change that caused the race was the introduction of host agg sync.
17:31:42 dansmith or are we potentially doing one more update if we got a 409 on the compute node and do a retry?
17:31:51 efried But the race stayed hidden because we weren't checking rp gen on agg updates.
17:31:55 efried Until this patch.
17:32:08 efried Nothing does a retry (yet)
17:32:12 dansmith no, that doesn't make sense
17:32:15 dansmith you checking aggs on the compute node doesn't make something else get a 409
17:32:19 efried I'm suggesting that as the fix.
17:32:36 efried Oh, the generation checking is in the method that's common to both the compute side and the host agg sync.
17:32:52 efried sorry that wasn't clear
17:33:08 dansmith oh set_aggregates_for_provider()
17:33:09 dansmith ?
17:33:37 efried https://review.openstack.org/#/c/556669/11/nova/scheduler/client/report.py@1250 yeah
17:33:41 dansmith api is using that for the mirroring, which never would fail before because it didn't check, but now is failing because it does, in the patch
17:33:57 dansmith okay, I got it.. I thought you were saying lots of things other than this
17:34:14 efried :)
17:34:23 efried You were too late. I already killed your hit man.
17:34:36 dansmith I requested NFO
17:34:42 dansmith so that must have been jaypipes' hit man
17:34:42 efried while typing with the other hand
17:35:46 efried dansmith: So do you agree adding a retry on both sides of this thing would be a viable solution?
17:36:14 dansmith well, I agree that (a) mirroring should be checking generation and (b) that it should retry if generation conflicts, so .. yes?
17:36:25 efried dig
17:38:28 efried I'll retract what I said earlier about there not being a bug, mriedem. There is, we were just hiding it before. I'll open it and *try* to explain it. Then fix it via ^
17:38:40 dansmith agree
17:39:21 efried Does the Big Lock in the rt mutex the _update periodic with the instance_claim path?
17:39:39 efried I guess it would have to.
17:41:04 dansmith that's the point of it I think
17:41:55 efried confirmed.
18:11:50 openstackgerrit Merged openstack/nova master: Merge server create schema for multiple create extension https://review.openstack.org/578338
18:11:57 openstackgerrit Merged openstack/nova master: Merge server create schema for scheduler hint extension https://review.openstack.org/578343
18:42:49 karimull Eric: I have one issue with a functional test case in nova on shared RP, can you help me understand where the issue is? here is the link
18:43:07 karimull Eric: http://paste.openstack.org/show/i9a6MRtJ1IUxF9xlI8to/
18:45:49 openstackgerrit Matt Riedemann proposed openstack/nova master: Add action initiator attribute to the instance payload https://review.openstack.org/536243
18:52:11 karimull efried: oops! pasted with wrong handle
18:52:31 efried karimull: looking...
18:52:48 karimull efried:thank you..
18:53:05 efried karimull: What patch are you working on this test for?
18:57:01 karimull efried:https://review.openstack.org/#/c/569498/9
18:57:56 efried karimull: Is the rebuild succeeding
18:57:57 efried ?
18:58:21 efried karimull: This is trying to rebuild to the same host?
18:59:27 openstackgerrit Merged openstack/nova master: conf: Correct documentation for '[pci] passthrough_whitelist' https://review.openstack.org/552874
18:59:33 openstackgerrit Merged openstack/nova master: [doc] enhance admin/configuration/api.rst https://review.openstack.org/566008
18:59:40 openstackgerrit Merged openstack/nova master: Remove mox in sec group test and functional tests https://review.openstack.org/576751

Earlier   Later