Comments inline. Salvatore
On 23 March 2015 at 15:41, John Belamaric <[email protected]> wrote: > > > On 3/20/15, 8:31 PM, "Salvatore Orlando" <[email protected]> wrote: > > > As the IPAM driver is called in 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. > I do not think there is a lot we can do about this at the moment, unless > we agree on a more pervasive refactoring: > > > > 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. > > > 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. > 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. > > > 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. > 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. 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. 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. > > Also, for drivers with a remote backend, remote calls will be made > within a DB transaction, which is another thing we wanted to avoid. > > > 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. > 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. 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. > > 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! > > > 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. > 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. 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. 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) > > John > > [1] > http://specs.openstack.org/openstack/neutron-specs/specs/kilo/neutron-ipam.html > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
