<p dir="ltr">>Given that it shares the session, it wouldn't have to do anything. But, again, it wouldn't behave like an external driver.</p>
<p dir="ltr">Why not? The only additional thing an external driver would be doing at this step is calling an external system. Any local accounting in the DB that driver would do would automatically be rolled back just like the in-tree system. </p>
<p dir="ltr">Keep in mind that anything else can fail later in the transaction outside of IPAM (e.g. ML2 driver precommit validation) and none of this IPAM rollback logic will be called. Maybe the right answer is to get rid of the IPAM rollback logic completely because it gives the wrong impression that it is going to be called on all failures to commit when it's really only called in failures inside of IPAM's module. Essentially every instance of _safe_rollback in [1] is in an exception handler that isn't triggered if there are exceptions anywhere in the core plugin after the initial base DB calls.</p>
<p dir="ltr">1. <a href="https://github.com/openstack/neutron/blob/master/neutron/db/ipam_pluggable_backend.py">https://github.com/openstack/neutron/blob/master/neutron/db/ipam_pluggable_backend.py</a></p>
<div class="gmail_extra"><br><div class="gmail_quote">On Aug 1, 2016 18:11, "Carl Baldwin" <<a href="mailto:carl@ecbaldwin.net">carl@ecbaldwin.net</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 1, 2016 at 2:29 PM, Kevin Benton <span dir="ltr"><<a href="mailto:kevin@benton.pub" target="_blank">kevin@benton.pub</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span>><span style="font-size:12.8px">We still want the exception to rollback the entire API operation and stopping it with a nested operation I think would mess that up.</span><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">Well I think you would want to start a nested transaction, capture the duplicate, call the ipam delete methods, then throw a retryrequest. The exception will still trigger a rollback of the entire operation.</span></div></div></blockquote><div><br></div><div>This is kind of what I was headed when I decided to solicit some feedback. It is a possibility should still be considered.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span><div><span style="font-size:12.8px">></span><span style="font-size:12.8px">Second, I've been throwing around the idea of not sharing the session with the IPAM driver.</span><br></div><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">If the IPAM driver does not have access to the session, it can't see any of the uncommitted data. Would that be a problem? In particular, doesn't the IPAM driver's DB table have foreign key constraints with the data waiting to be committed in the other session? I'm hesitant to take this approach because it means other (if the in-tree doesn't already) IPAM drivers cannot have any relational integrity with the objects in question.</span></div></div></blockquote><div><br></div><div>The in-tree driver doesn't have any FK constraints back to the neutron db schema for IPAM [1]. I don't think that would make sense since it is supposed to work like an external driver.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>A related question is, why does the in-tree IPAM driver have to do anything at all on a rollback? It currently does share a session which is automatically going to rollback all of it's DB operations for it. If it's because the driver cannot distinguish a delete call from a rollback and a normal delete, I suggest we change the delete call to pass a flag indicating that it's for a rollback. That would allow any DB-based drivers to just do nothing at this step.<br></div></div></blockquote><div><br></div><div><div>Given that it shares the session, it wouldn't have to do anything. But, again, it wouldn't behave like an external driver. I'd like to not have special drivers that behave differently than drivers that are really external; we end up finding things that the in-tree driver does in our testing that doesn't work right for other drivers.</div><div><br></div><div>Drivers might need to access uncommitted data from the neutron DB. I think even external drivers do this. However, there is a hard line between the Neutron tables (even IPAM related ones) and the pluggable IPAM driver database schema. I should have been a little more explicit that I wasn't suggesting that we hide the Neutron session from the driver. What I meant to suggest is that we use a different session for the part of the database schema that belongs solely to the driver. All of the changes would be inside the driver implementation and the interface to the driver wouldn't change at all.</div><div><br></div><div>Carl</div><div><br></div><div>[1] <a href="https://github.com/openstack/neutron/blob/2b1c143ca9/neutron/ipam/drivers/neutrondb_ipam/db_models.py" target="_blank">https://github.com/openstack/neutron/blob/2b1c143ca9/neutron/ipam/drivers/neutrondb_ipam/db_models.py</a></div></div></div></div></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div></div>