<div dir="ltr">I think removing all occurrences of create_port inside of another transaction is something we should be doing for a couple of reasons.<div><br></div><div>First, it's a recipe for the cherished "lock wait timeout" deadlocks because create_port makes yielding calls. These are awful to troubleshoot and are pretty annoying for users (request takes ~60 seconds and then blows up). <div><br></div><div>Second, create_port in ML2 expects the transaction to be committed to the DB by the time it's done with pre-commit phase, which we break by opening a parent transaction before calling it so the failure handling semantics may be messed up.<div><br></div><div><br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 13, 2015 at 9:48 AM, Carl Baldwin <span dir="ltr"><<a href="mailto:carl@ecbaldwin.net" target="_blank">carl@ecbaldwin.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Have we found the last of them?  I wonder.  I suppose any higher level<br>
service like a router that needs to create ports under the hood (under<br>
the API) will have this problem.  The DVR fip namespace creation comes<br>
to mind.  It will create a port to use as the external gateway port<br>
for that namespace.  This could spring up in the context of another<br>
create_port, I think (VM gets new port bound to a compute host where a<br>
fip namespace needs to spring in to existence).<br>
<span class="HOEnZb"><font color="#888888"><br>
Carl<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Mon, Apr 13, 2015 at 10:24 AM, John Belamaric<br>
<<a href="mailto:jbelamaric@infoblox.com">jbelamaric@infoblox.com</a>> wrote:<br>
> Thanks Pavel. I see an additional case in L3_NAT_dbonly_mixin, where it<br>
> starts the transaction in create_router, then eventually gets to<br>
> create_port:<br>
><br>
> create_router (starts tx)<br>
>   ->self._update_router_gw_info<br>
>   ->_create_gw_port<br>
>   ->_create_router_gw_port<br>
>   ->create_port(plugin)<br>
><br>
> So that also would need to be unwound.<br>
><br>
> On 4/13/15, 10:44 AM, "Pavel Bondar" <<a href="mailto:pbondar@infoblox.com">pbondar@infoblox.com</a>> wrote:<br>
><br>
>>Hi,<br>
>><br>
>>I made some investigation on the topic[1] and see several issues on this<br>
>>way.<br>
>><br>
>>1. Plugin's create_port() is wrapped up in top level transaction for<br>
>>create floating ip case[2], so it becomes more complicated to do IPAM<br>
>>calls outside main db transaction.<br>
>><br>
>>- for create floating ip case transaction is initialized on<br>
>>create_floatingip level:<br>
>>create_floatingip(l3_db)->create_port(plugin)->create_port(db_base)<br>
>>So IPAM call should be added into create_floatingip to be outside db<br>
>>transaction<br>
>><br>
>>- for usual port create transaction is initialized on plugin's<br>
>>create_port level, and John's change[1] cover this case:<br>
>>create_port(plugin)->create_port(db_base)<br>
>><br>
>>Create floating ip work-flow involves calling plugin's create_port,<br>
>>so IPAM code inside of it should be executed only when it is not wrapped<br>
>>into top level transaction.<br>
>><br>
>>2. It is opened question about error handling.<br>
>>Should we use taskflow to manage IPAM calls to external systems?<br>
>>Or simple exception based model is enough to handle rollback actions on<br>
>>third party systems in case of failing main db transaction.<br>
>><br>
>>[1] <a href="https://review.openstack.org/#/c/172443/" target="_blank">https://review.openstack.org/#/c/172443/</a><br>
>>[2] neutron/db/l3_db.py: line 905<br>
>><br>
>>Thanks,<br>
>>Pavel<br>
>><br>
>>On 10.04.2015 21:04, <a href="mailto:openstack-dev-request@lists.openstack.org">openstack-dev-request@lists.openstack.org</a> wrote:<br>
>>> L3 Team,<br>
>>><br>
>>> I have put up a WIP [1] that provides an approach that shows the ML2<br>
>>>create_port method refactored to use the IPAM driver prior to initiating<br>
>>>the database transaction. Details are in the commit message - this is<br>
>>>really just intended to provide a strawman for discussion of the<br>
>>>options. The actual refactor here is only about 40 lines of code.<br>
>>><br>
>>> [1] <a href="https://review.openstack.org/#/c/172443/" target="_blank">https://review.openstack.org/#/c/172443/</a><br>
>>><br>
>>><br>
>>> Thanks,<br>
>>> John<br>
>><br>
>><br>
>>__________________________________________________________________________<br>
>>OpenStack Development Mailing List (not for usage questions)<br>
>>Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>><a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
><br>
> __________________________________________________________________________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div>Kevin Benton</div></div>
</div>