Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-06
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 :)
16:45:03 efried jaypipes I think I mainly undid carnage
16:45:27 mdbooth jaypipes: That is, all your retries will fail.
16:45:31 jaypipes mdbooth: no.
16:45:52 jaypipes mdbooth: the retry decorator is designed to work with the enginefacade's transaction context...
16:46:08 jaypipes zzzeek: you around? need your expert opinion on something.
16:46:15 mdbooth jaypipes: Ah... lemme check that. I may have missed that.
16:46:48 efried jaypipes Argh, found a pep8 error in your REST API change.
16:46:59 efried jaypipes in_tree pushed a docstring line over 80c :(
16:47:02 jaypipes efried: that I did or that you did? :)
16:47:07 zzzeek jaypipes: gotta leave in about 3 minutes but whats up
16:47:08 jaypipes efried: ah
16:47:18 jaypipes zzzeek: sec, grabbing link
16:47:19 efried jaypipes You did. But let me fix it, since I've got the stack right here.
16:47:40 mriedem cdent: done https://review.openstack.org/#/c/521639/ thanks
16:47:55 jaypipes zzzeek: line 123, my comment here: https://review.openstack.org/#/c/242603/25/nova/objects/block_device.py
16:48:07 cdent mriedem: cool, yeah on the IRON_NFV cleanup, I didn’t want to disrupt the entire functional test
16:48:10 jaypipes zzzeek: AFAIK, there's no need to do that
16:48:30 openstackgerrit Eric Fried proposed openstack/nova master: placement: allow filter providers in tree https://review.openstack.org/377215
16:48:31 openstackgerrit Eric Fried proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807
16:48:32 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663
16:48:32 openstackgerrit Eric Fried proposed openstack/nova master: Scheduler set_inventory_for_provider does nested https://review.openstack.org/520643
16:48:32 jaypipes zzzeek: i.e. no need for the txtct.using(context) thing on line 125
16:48:32 openstackgerrit Eric Fried proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693
16:48:33 openstackgerrit Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
16:48:33 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097
16:48:34 openstackgerrit Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
16:48:34 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
16:48:35 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
16:48:40 efried jaypipes Fixed. Very bottom of the stack, of course.
16:48:47 jaypipes efried: ack
16:48:49 jaypipes ty
16:48:59 mdbooth jaypipes: I'm looking at the code for wrap_db_retry, and I don't see anything which would create a new transaction.
16:49:25 mdbooth We normally use this decorator on a function which also has a decorator to create a transaction context, though.
16:49:38 jaypipes mdbooth: the whole *point* of the retry decorator is to catch the ROLLBACK and restart a transaction :)

Earlier   Later