| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-06 | |||
| 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: allow filter providers in tree https://review.openstack.org/377215 | |
| 16:43:34 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807 | |
| 16:43:35 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693 | |
| 16:43:35 | openstackgerrit | Eric Fried proposed openstack/nova master: Scheduler set_inventory_for_provider does nested https://review.openstack.org/520643 | |
| 16:43:36 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663 | |
| 16:43:36 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097 | |
| 16:43:37 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 16:43:37 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 16:43:38 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 16:43:38 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 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: placement: update client to set parent provider https://review.openstack.org/385693 | |
| 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: Scheduler set_inventory_for_provider does nested https://review.openstack.org/520643 | |
| 16:48:32 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663 | |
| 16:48:33 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097 | |
| 16:48:33 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 16:48:34 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 16:48:34 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 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 | |
| 16:53:58 | mdbooth | def my_foo(): | |
| 16:54:05 | zzzeek | mdbooth jaypipes haven't looked at this stuff in awhile, the context manager looks unfamiliar to me, would want to see other places the rerty decorator is used and see if a. they also have a nested trans or b. they dont, and then how do these work? | |
| 16:54:38 | gibi | efried: done | |
| 16:54:43 | efried | gibi Thanks! | |
| 16:54:57 | mdbooth | jaypipes: Let me think again if there's a good enough reason not to use the decorator there. | |
| 16:55:13 | dansmith | sales meeting in 5 | |
| 16:55:36 | jaypipes | mdbooth: I'm not saying don't use the decorator. I'm saying no need to use the inner tctxt.using() thing. | |
| 16:56:21 | mdbooth | jaypipes: So I'm also entirely sure we must have an enginefacade transaction scope there | |
| 16:56:34 | mdbooth | Which we normally do at function scope using a decorator | |
| 16:56:47 | mdbooth | I'm actually being super-defensive there, I think | |