Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-06
16:08:10 mriedem i'm not even sure why we need this
16:08:14 dansmith what if we do it only if save() succeeded
16:08:15 mriedem given https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L597
16:08:50 dansmith mriedem: I think it's because that might've happened in a separate api db
16:08:57 mriedem we could alternatively check if we've set the cell mapping on the instance mapping, meaning we've picked a host, https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L509 but that's racy
16:09:19 mriedem because then conductor and cells/messaging are racing to delete the build request
16:09:30 mriedem but cells/messaging will only do it on the next update at top
16:09:40 mriedem which presumably would come after conductor has set the mapping and already deleted the build request
16:09:57 dansmith if they're sharing an api db, but not the cell db,
16:10:13 mriedem hmm, so the thing conductor deletes isn't necessarily the same thing that cells/messaging deletes?
16:10:24 dansmith then an update will delete the BR in the shared api db, which will then make conductor delete from the cell db, which will sync back up as a delete to the top level db
16:10:44 dansmith but if they're separate api dbs,
16:10:45 dansmith then you have to sync the BR delete up or you shadow the actual instance
16:10:52 dansmith since we were in the cell db when we did the BR delete
16:11:27 dansmith mriedem: you wanna borrow my rusty spoon when I'm done with it?
16:11:35 mriedem for your eyeballs?
16:11:37 dansmith yes
16:11:43 mriedem sure
16:11:46 mriedem send'er over
16:11:48 dansmith heh
16:11:55 mriedem pre-gooped please
16:14:10 dansmith mriedem: anyway, I want to fix this, but I can't really justify spending time on it ahead of other stuff on my plate
16:14:37 dansmith the patch is there for people to apply if they need it for that scenario, and maybe I can circle back after FF or something
16:17:05 belmoreira dansmith: +1, for me this patch makes sense in older versions (newton, ocata). Not sure how useful it will be in Pike, Queens.
16:17:10 belmoreira dansmith mriedem thanks. I will keep you posted
16:19:02 mriedem dansmith: edleafe: so after careful contemplation in the last 5 minutes,
16:19:08 mriedem i think we can unpin the Selection object patch
16:19:21 dansmith okay
16:19:31 mriedem my concerns in the top patch in the series about filtering out the hosts that have already been tried can be done in other ways using the filter_properties retry list of hosts
16:19:39 mriedem i.e. knowing what's already been claims
16:19:41 mriedem *claimed
16:20:22 bauzas I need to look at those Selection changes me too
16:20:55 dansmith gdi gerrit
16:21:38 mriedem jaypipes: https://review.openstack.org/#/c/495854/ if you will +W
16:23:01 mriedem or i can just re-approve, it was a rebase plus some minor changes
16:23:24 mriedem i'll just re-approve :)
16:25:33 gibi mriedem: I proposed a fix for bug 1735407 but I feel that it only solves part of the race.
16:25:34 openstack bug 1735407 in OpenStack Compute (nova) "[Nova] Evacuation doesn't respect anti-affinity rules" [Medium,In progress] https://launchpad.net/bugs/1735407 - Assigned to Balazs Gibizer (balazs-gibizer)
16:25:37 mriedem efried: per your earlier question about empty vs 404 ,this was the spec discussion i was thinking of https://review.openstack.org/#/c/497713/9/specs/queens/approved/add-trait-support-in-allocation-candidates.rst@78
16:25:37 cdent efried: jaypipes, mriedem and edleafe are all correct
16:25:41 gibi mriedem: I would appreciate your view about the possible solution I drafted in the bug report (see my last 3 comments there)
16:26:03 efried cdent mriedem ack
16:26:13 cdent the only time a 404 should happen on a collection resource is if the URL doesn’t exist (in which case it’s not a collection resource)
16:27:30 mriedem gibi: in general i think reducing the window is a positive step forward, despite other known limitations as pointed out
16:27:35 mdbooth jaypipes: Pretty sure that anywhere that uses retry without creating a transaction context would be a bug, no?
16:28:06 mriedem gibi: #3 in comment 11 would be a further improvement in case we still have issues
16:29:12 gibi mriedem: I agree. We have to see if the current fix solves the problem in the environment it was found earlier
16:29:53 gibi mriedem: if yes then we are OK, if no then I can try to implement option #3
16:30:10 sean-k-mooney2 mriedem: dansmith when ye have a second could ye take alook at this trival update to the .gitreview file for stable/pike in os-vif https://review.openstack.org/#/c/488670/
16:31:18 dansmith snagged
16:31:20 mriedem gibi: there is an issue in your patch
16:31:30 mriedem gibi: the compute can't up-call to get the request spec
16:31:35 mriedem b/c the reqspec is in the api db
16:31:40 mriedem and the compute should be isolated fromthat
16:32:02 gibi mriedem: that is bad
16:32:12 mriedem :)
16:32:25 mriedem it just means that you would have to plumb the request_spec through to the compute rebuild_instance method
16:32:28 mriedem using rpc api parms
16:32:44 gibi mriedem: would that mean an rpc version bump as well?
16:32:50 mriedem yes
16:32:56 mriedem granted, _validate_instance_group_policy is already doing an up-call to the api db
16:32:59 mriedem by getting the server groups
16:33:09 mriedem and that's why we have CONF.workarounds.disable_group_policy_check_upcall
16:34:40 gibi would it make sense doing the request spec upcall in the _validate_instance_group_policy ?
16:34:50 gibi in _do_validation
16:34:54 mriedem no,
16:35:00 mriedem we need fewer up-calls, not more
16:35:21 gibi true
16:35:22 mriedem https://docs.openstack.org/nova/latest/user/cellsv2-layout.html#caveats-of-a-multi-cell-deployment
16:35:28 mriedem ^ that list needs to shrink
16:36:20 gibi this would be inside the 'The late anti-affinity check' item in that list, but I agree to look at the other option instead
16:36:45 gibi so, if the rpc version is bumped, can I still backport the fix to stable branches?
16:36:54 mriedem no
16:37:57 mriedem gibi: given the super latent nature of this bug,
16:38:04 gibi it seems this bug exists from at least Mitaka
16:38:16 mriedem i don't think we need to add more up-calls within the validate method just to backport
16:38:44 mriedem the bug has existed since evacuate i assume
16:38:49 gibi could be
16:39:01 gibi so we say we only fix it in master and not on any stable
16:39:23 mriedem i think so
16:39:52 gibi OK, I will do the rpc change
16:40:06 openstackgerrit Mike Perez proposed openstack/nova master: Replace support matrix ext with common library https://review.openstack.org/481304
16:40:10 gibi mriedem: thanks for feedback on the fix
16:40:15 mriedem np
16:43:31 jaypipes mdbooth: the transaction context is automatically managed by the engine facade. I'm saying there's no need to do the secondary independent transaction context thing
16:43:34 openstackgerrit Eric Fried proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807
16:43:34 openstackgerrit Eric Fried proposed openstack/nova master: placement: allow filter providers in tree https://review.openstack.org/377215
16:43:35 openstackgerrit Eric Fried proposed openstack/nova master: Scheduler set_inventory_for_provider does nested https://review.openstack.org/520643
16:43:35 openstackgerrit Eric Fried proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693
16:43:36 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097
16:43:36 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663
16:43:37 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
16:43:37 openstackgerrit Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
16:43:38 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
16:43:38 openstackgerrit Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
16:44:06 efried jaypipes cdent edleafe ^ The bottom three are just rebases onto latest master. The rest that aren't WIP ought to be clear and ready for reviews.
16:44:38 jaypipes efried: cool. hopefully you didn't leave the same carnage as I did trying to rebase those...
16:44:40 cdent efried: roger. just header into that stack now
16:44:48 mdbooth jaypipes: But it's not managed by the retry, right? So if you retry without an enginefacade call at the same scope, you'll still have an aborted transaction.
16:44:52 efried jaypipes I am also hopeful :)

Earlier   Later