Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-21
14:56:25 mriedem i'm cool with get_by_disabled, but don't default the 'disabled' param?
14:56:32 mriedem so caller has to know what they are asking for
14:56:41 dansmith yep
14:57:08 mriedem ok wfm
14:57:12 tssurya wiat, so get_by_disabled() will give enabled by default right ?
14:57:16 mriedem no
14:57:18 mriedem no default
14:57:21 mriedem no kwarg
14:57:22 stephenfin Is there another stable core that could take a look at this, please? https://review.openstack.org/#/c/550079/
14:57:29 tssurya ah so the user has to pass a value
14:57:34 mriedem stephenfin: i can
14:57:37 mriedem tssurya: yeah
14:57:41 tssurya it becomes mandatory, got it
14:57:48 tssurya works for me as well
14:57:55 tssurya thanks
14:59:18 stephenfin mriedem: Thank you
15:00:08 mriedem stephenfin: do you want to propose a release for os-vif on master?
15:00:14 mriedem if this is high severity
15:00:33 openstackgerrit Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729
15:00:34 openstackgerrit Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730
15:00:34 openstackgerrit Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990
15:00:35 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
15:00:35 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
15:04:39 mriedem stephenfin: what is the minimum version that --may-exist exists in ovs-vsctl?
15:10:12 mriedem looks like forever ago https://github.com/openvswitch/ovs/commit/bb1c67c813c9bd80c2bd9acf2bf5158b48841c61
15:12:13 cfriesen bauzas: you wanted to set up a meeting? I'm in CST timezone...it's 9:12.
15:12:39 bauzas cfriesen: just discussing about NUMA topology
15:12:53 bauzas cfriesen: but jaypipes asked for tomorrow morning EST
15:13:16 cfriesen should be doable
15:13:27 cfriesen jaypipes: my team is in Ottawa, I'm in Saskatchewan
15:16:09 mriedem sean-k-mooney: are you ok with this on stable/queens? https://review.openstack.org/#/c/554917/
15:16:41 gibi Kevin_Zheng: sorry I haven't had time yet to think about your request_id functional test problem but I did not forget it
15:18:01 gibi mriedem: fyi here is a followup bug for the notifications-calling-keystone problem https://bugs.launchpad.net/nova/+bug/1757407
15:18:01 openstack Launchpad bug 1757407 in OpenStack Compute (nova) "Notification sending sometimes hits the keystone API to get glance endpoints" [Undecided,New]
15:18:35 gibi mriedem: there is a case where we hit keystone even if only versioned notifications are configured to be emitted
15:18:48 Kevin_Zheng gibi: np I was also busy these days so I didn’t dig deeper, I will try to find out what’s going on tomorrow:)
15:19:19 stephenfin mriedem: Yup, it's there since forever. There was an issue with it but that was resolved in...OVS 2.5, iirc
15:19:40 stephenfin and it wasn't a significant issue. Could only be reproduced under very specific circumstances
15:21:18 mriedem gibi: yeah so we could optimize to not even do that lookup if only using versioned notifications,
15:21:39 mriedem also, efried said the glance endpoint / service catalog information should be cached in ksa, so we shouldn't be hitting the keystone API every time, only the first time,
15:21:59 stephenfin mriedem: Also, I can propose a fix, yup
15:22:08 mriedem but it's curious that we could create a server (which would fetch the image on the compute) and then the periodic (without a token) would have problems stopping it
15:22:31 mriedem unless you did something like had (1) cached images on the compute or (2) restarted nova-compute in between to invalidate the ksa cache
15:22:45 openstackgerrit Merged openstack/os-vif stable/queens: ovs: do not delete port if already exists https://review.openstack.org/550079
15:22:50 mriedem unless the cache has a timer on it? or is somehow otherwise request-specific
15:23:12 gibi mriedem: I can try to create a functional test for this
15:23:25 sahid jaypipes, mriedem, anychance to have you ack this ? https://review.openstack.org/#/c/485522/
15:23:41 mriedem sahid: i'll look at it again today
15:24:04 mriedem s/today/now
15:24:30 sahid cool thanks
15:24:53 efried mriedem, gibi: The caching would be specific to the context. So you would be hitting the endpoint (to do version discovery) once per unique context (as opposed to just the first time overall).
15:25:09 efried ...I think.
15:25:19 mriedem ah ok
15:25:22 mriedem well that makes sense then
15:25:33 gibi efried, mriedem: a periodic task runs with the same context every time, i guess
15:26:03 mriedem gibi: nope
15:26:26 mriedem gibi: periodic tasks actually run with the last context stored in the local thread,
15:26:41 mriedem something like that, but that's why we see request IDs for user contexts get mixed up with periodic tasks in the logs
15:26:44 mriedem and it's totally confusing
15:26:45 gibi mriedem: I thought about this https://review.openstack.org/#/c/524306
15:27:00 mriedem yes we were talking about that the otherday
15:27:41 gibi mriedem: so that patch will make sure that the periodic task runs with the same context every time
15:28:10 mriedem for the lifetime of that service process yes i think so
15:28:16 gibi mriedem: which will limit the performance impact of the notifications calling keystone
15:28:30 mriedem well, the notifications can't call keystone anyway
15:28:31 gibi mriedem: due to the cache in ksa
15:28:32 mriedem they don't have a token
15:29:01 gibi mriedem: OK, I'm confused. :)
15:29:12 mriedem we realized that "ctxt = context.get_admin_context()" has this overwrite=False flag: https://github.com/openstack/nova/blob/master/nova/context.py#L290
15:29:20 mriedem which is used in the parent class in oslo.context
15:29:43 mriedem https://github.com/openstack/oslo.context/blob/master/oslo_context/context.py#L225
15:31:18 gibi mriedem: so if the first call to keystone from the periodic task fails then there is nothing to cache so the next call will also go to keystone and fail again
15:31:23 mriedem from what i can tell, that's why periodic tasks re-use the request id from the local thread store
15:31:32 mriedem gibi: i think so
15:31:57 dansmith mriedem: stvnoyes1: this patch is changing code you wrote for the new attachment workflow, but makes it much simpler.. what am I missing? https://review.openstack.org/#/c/551302
15:32:04 gibi mriedem: then definitely need remove the keystone call from the notification codepath
15:32:40 gibi mriedem: what do you think can we do someting with the legacy notifications top of what you already did by catching the exception?
15:32:57 gibi mriedem: to avoid the keystone call
15:33:29 mriedem dansmith: i haven't dug into that one in detail yet
15:33:47 mriedem because it's going to require loading a bunch of context into my head, including stuff like mixed version compute issues
15:34:15 dansmith mriedem: I don't think it does actually,
15:34:19 gibi mriedem: maybe not even trying to generate the glance url (and call keystone) if the glance/api_server config is not set
15:34:20 dansmith mriedem: it doesn't change what we do over the wire
15:34:21 mriedem gibi: can't really do much there for the legacy ones
15:35:01 mriedem i'm mid-review in sahid's spec review so will have to look in a bit
15:35:09 dansmith ack, thanks
15:35:13 stvnoyes1 dansmith, thanks, I'll take a look at it. I'll have to remember what I did and why.
15:36:58 mriedem everything stvnoyes1 added would have just been conditional logic on the flow for the new attachment ID stuff
15:37:26 mriedem which was fairly mechanical, i.e. 'we used to call initialize_connection to get a new connection_info, now we call attachment_update'
15:37:59 dansmith mriedem: yeah, but the intersection is that this seems to preclude adding old_attachment_id to the migratedata stuff for the v3 attach,
15:38:15 dansmith but also fixes the same issue that old_attachment_id fixes for v3, but for v2
15:38:35 dansmith I mean, that's the assertion
15:39:25 dansmith mdbooth: the bug is basically just the commit message on the patch.. can you add some more detail (logs, traces, etc) to help make it more convincingly problematic?
15:39:52 dansmith mdbooth: because since the old side of all this code has been around for a while, it's legit to question that it's been broken this long, even if it's just for one driver
15:46:42 openstackgerrit Merged openstack/nova master: ironic: stop lying to the RT when ironic is down https://review.openstack.org/545479
15:48:13 openstackgerrit Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729
15:48:14 openstackgerrit Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730
15:48:15 openstackgerrit Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990
15:48:16 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
15:48:16 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282

Earlier   Later