| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-06 | |||
| 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 :) | |
| 16:49:53 | mdbooth | I don't think so, no. | |
| 16:49:58 | mdbooth | I think it just retries. | |
| 16:50:09 | mdbooth | And the way we always use it is that we also create a transaction. | |
| 16:50:22 | zzzeek | jaypipes: i belive your comment is accurate but the method itself has to be called before there's some larger transaction going on , unless the retry decorators know to propagate all the way up to the top one | |
| 16:50:59 | efried | bauzas gibi Would you please re-+A https://review.openstack.org/377215 (rebase & trivial pep8 change) | |
| 16:51:01 | mdbooth | There's no nested context stuff going on with the retry decorator | |
| 16:51:17 | bauzas | efried: did | |
| 16:51:31 | efried | bauzas Merci | |
| 16:51:43 | mdbooth | jaypipes: I'm looking at the code: it just runs it again | |
| 16:52:27 | dansmith | the transaction is in the decorator, so if the retry is above that, it'll do another transaction | |
| 16:52:36 | mdbooth | dansmith: Right. | |
| 16:52:53 | mdbooth | They're 2 separate things, but we normally use them together. | |
| 16:52:55 | zzzeek | jaypipes mdbooth i dont see the point of the retry decorator if you have a context manager inside to handle the thing anyway | |
| 16:53:21 | mdbooth | zzzeek: Well the context manager always has to be inside the retry decorator | |
| 16:53:24 | jaypipes | zzzeek: me neither, which is why I wrote that comment. | |
| 16:53:44 | mdbooth | However, we normally put it inside the retry decorator using another decorator | |
| 16:53:47 | mdbooth | So: | |
| 16:53:51 | mdbooth | @retry | |
| 16:53:55 | mdbooth | @transaction | |