<div dir="ltr">Comments inline.<div><br></div><div>Salvatore<br><div class="gmail_extra"><br><div class="gmail_quote">On 23 March 2015 at 15:41, John Belamaric <span dir="ltr"><<a href="mailto:jbelamaric@infoblox.com" target="_blank">jbelamaric@infoblox.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">



<div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif"><span class="">
<div><br>
</div>
<div><br>
</div>
<span>
<div>
<div>On 3/20/15, 8:31 PM, "Salvatore Orlando" <<a href="mailto:sorlando@nicira.com" target="_blank">sorlando@nicira.com</a>> wrote:</div>
</div>
<div><br>
</div>
<blockquote style="BORDER-LEFT:#b5c4df 5 solid;PADDING:0 0 0 5;MARGIN:0 0 0 5">
<div>
<div>
<div dir="ltr">
<div><br>
</div>
<div>As the IPAM driver is called in <font color="#010181"><span style="font-size:14px">NeutronDbPluginV2, this call happens while another transaction is typically in progress. Initiating a separate session within the IPAM driver causes the outer transaction
 to fail.</span></font><br>
</div>
<div><font color="#010181"><span style="font-size:14px">I do not think there is a lot we can do about this at the moment, unless we agree on a more pervasive refactoring:</span></font></div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</blockquote>
</span>
</span><div>
<div><br>
</div>
<div>This is essentially what is described in the "Refactoring" section of the spec [1] (see the third paragraph in that section specifically). The original expectation (as you say) was that we would be able to achieve this by only changing the DB plugin, but
 this looks to not be feasible at this point given the way sessions are handled in the subclasses.</div>
</div><span class="">
<div><br>
</div>
<span>
<blockquote style="BORDER-LEFT:#b5c4df 5 solid;PADDING:0 0 0 5;MARGIN:0 0 0 5">
<div>
<div>
<div dir="ltr">
<div><br>
</div>
<div>Otherwise, we'd just made the IPAM driver session aware. This implies changes to the Pool and Subnet interface to accept an optional session parameter.</div>
<div>The above works and is probably quicker from an implementation perspective. However, doing so somehow represents a failure of the pluggable IPAM effort as total separation between IPAM and API operation processing was one of our goals.
</div>
</div>
</div>
</div>
</blockquote>
</span>
<div><br>
</div>
</span><div>
<div>Actually, I don't see this as a big deal or a failure. In fact, it may be quite common and useful for a given driver to store some state in its own tables (like the reference driver is doing). The primary goal is to enable alternate IPAM implementations,
 whether external or completely local. That goal is still achieved even with this change. So, I don't see a big problem with the IPAM driver being session-aware.</div></div></div></blockquote><div><br></div><div>Whether is a big deal or a failure it depends on one's expectation. If the expectation is to enable external IPAM backends, then I agree that this is not a big deal.</div><div>However, my expectation (and I hope I'm not the only one), was to disentangle IPAM logic from the API request processing code. In this way 3rd party backend transactions would have been executed independently of the database operation for processing the API request. Also, this would have enabled us to integrate tools like taskflow (I think Pavel looked into that). And most importantly avoid long database transactions which include remote calls as well.</div><div><br></div><div>However, this is not achievable - and mostly for an oversight on our side. So we should welcome passing down the context to the driver, with the goal of removing it in the next release. I think it will be fair to assume the interface "experimental" for this release cycle - and "stable" for the next one.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif"><div>
</div><span class="">
<div><br>
</div>
<span>
<blockquote style="BORDER-LEFT:#b5c4df 5 solid;PADDING:0 0 0 5;MARGIN:0 0 0 5">
<div>
<div>
<div dir="ltr">
<div>Also, for drivers with a remote backend, remote calls will be made within a DB transaction, which is another thing we wanted to avoid.</div>
</div>
</div>
</div>
</blockquote>
</span>
<div><br>
</div>
</span><div>This is more of a concern, due to the issue with the mysql connector. I guess I'd like to understand better what that issue is and how we may resolve it, since it is a source of pain not just for IPAM.</div></div></blockquote><div><br></div><div>The issue is not really with mysql connector but more in its interaction with eventlet. This is something that will eventually (hopefully soon) be sorted within oslo_db. However, generally speaking it is better to avoid long-standing database transactions.</div><div>However, we've provided enough reasons for which we'd need, at least for this release, push down the database session to the IPAM driver. So it's now up to reviewers to decide whether this is acceptable or not.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif"><span class="">
<div><br>
</div>
<span>
<blockquote style="BORDER-LEFT:#b5c4df 5 solid;PADDING:0 0 0 5;MARGIN:0 0 0 5">
<div>
<div>
<div dir="ltr">
<div><br>
</div>
<div>And finally, there is the third option. I know IPAM contributors do not even want to hear it... but the third option is to enjoy 6 more months to come up with a better implementation which does not add any technical debt. In Kilo from the IPAM side we're
 already introducing subnet pools, so it won't be a total failure!</div>
</div>
</div>
</div>
</blockquote>
</span>
<div><br>
</div>
</span><div>I think we can still handle the primary use case without adding technical debt. We had hoped to *re-pay* more technical debt with the refactor than we are able to achieve, but I don't see any additional debt by making the driver session-aware.</div></div></blockquote><div><br></div><div>The technical debt aspect of adding a session to the driver is simply due to the fact that we'll need to have a follow-up action to remove it.</div><div>Such follow up action will imply some more refactoring in the db base class, possibly removing some of the code Pavel is introducing and moving it into an appropriate IPAM integration module. This will be the additional technical debt.</div><div>On the other hand, if the developers working on IPAM agree that it is ok to keep passing down the session to the IPAM driver as a permanent solution, then we have less technical debt to deal with (and this would be the one we anticipated because for Kilo Neutron should be able to run with and without the IPAM driver)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif">
<div><br>
</div>
<div>John</div>
<div><br>
</div>
<div>[1] <a href="http://specs.openstack.org/openstack/neutron-specs/specs/kilo/neutron-ipam.html" target="_blank">http://specs.openstack.org/openstack/neutron-specs/specs/kilo/neutron-ipam.html</a></div>
<span>
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div dir="ltr">
<div class="gmail_extra"><br>
</div>
</div>
</div>
</div>
</div>
</div>
</span>
</div>

<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></blockquote></div><br></div></div></div>