Re: [openstack-dev] [Neutron] Update on DB IPAM driver
I must have archived this on accident. Sorry to not respond earlier. Comments inline... On Feb 12, 2015 6:40 AM, Salvatore Orlando sorla...@nicira.com wrote: I have updated the patch; albeit not complete yet it's kind of closer to be an allocator decent enough to replace the built-in logic. I hope to look at it soon. Thanks for your work here. I will be unable to attend today's L3/IPAM meeting due to a conflict, so here are some highlights from me on which your feedback is more than welcome: We missed you. - I agree with Carl that the IPAM driver should not have explicit code paths for autoaddress subnets, such as DHCPv6 stateless ones. In that case, the consumer of the driver will generate the address and then to the IPAM driver that would just be allocation of a specific address. However, I have the impression the driver still needs to be aware of whether the subnet has an automatic address mode or not - since in this case 'any' address allocation won't be possible. There already comments about this in the review [1] Good point. - We had a discussion last week on whether the IPAM driver and neutron should 'share' database tables. I went back and forth a lot of time, but now to me it seems the best thing to do is to have the IPAM driver maintain an 'ip_requests' tables, where it stores allocation info. This table partially duplicates data in IPAllocation, but on the plus side it makes the IPAM driver self sufficient. The next step would be to decide whether we want to go a step further and also assume the driver should not access at all Neutron's DB, but I would defer that discussion to the next iteration (for both the driver and the IPAM interface) I like this. It is how I was leaning too. - I promised a non blocking algorithm for IP allocation. The one I was developing was based on specifying the primary key on the ip_requests table in a way that it would prevent two concurrent requests from getting the same address, and would just retry getting an address until the primary key constraint was satisfied. However, recent information emerged on MySQL galera's (*) data set [2] certification clarified that this kind of algorithm would still result in a deadlock error from failed data set certification. It is worth noting that in this case a solution based on traditional compare-and-swap is not possible because concurrent requests would be inserting data at the same time. I am now working on an alternative solution, and I would like to first implement a PoC for it (so that I can prove it works). Hmm. Didn't see that coming. - The db base refactoring being performed by Pavel is under way [3]. It is worth noting that this is a non-negligible change to some of Neutron's basic and more critical workflows. We should expect pushback from the community regarding the introduction of this change in the 3rd milestone. At this stage I would suggest either: A) consider a strategy for running pluggable IPAM as optional Looking at Pavel's code, it appears it is optional in its current state. It seems to leave all of the existing code in place. The if/then/else statements make me want to pull my hair out but if it is just a temporary strategy to mitigate risk them I guess I could tolerate it until Liberty. B) consider delaying to Liberty. (and that's where I get virtually jeered and pelted with rotten tomatoes) I think we still have time maybe with risk mitigation from A. I'm not ready to give up yet! Carl __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
On Feb 13, 2015 5:54 AM, Salvatore Orlando sorla...@nicira.com wrote: - Considering an alternative to availability ranges. Pre-generation of IP entries is unpractical (think IPv6), so that's not an option. Unfortunately, I have not yet explored in detail this route. The availability range stuff is hard. I was talking about it with Ryan Tidwell last week. I actually wonder if it would be much worse to just load all of the allocations and compute availability on demand. It seems drastic and probably isn't better but that is how bad storing and maintaining availability is in the db. I suspect it will be more difficult for subnet allocation. Would you consider an alternative that pregenerated up to N entries and kept a flag on allocations stating whether they are in use or can be recycled? Carl __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
On Thu, Feb 12, 2015 at 6:36 AM, Salvatore Orlando sorla...@nicira.com wrote: - I agree with Carl that the IPAM driver should not have explicit code paths for autoaddress subnets, such as DHCPv6 stateless ones. In that case, the consumer of the driver will generate the address and then to the IPAM driver that would just be allocation of a specific address. However, I have the impression the driver still needs to be aware of whether the subnet has an automatic address mode or not - since in this case 'any' address allocation won't be possible. There already comments about this in the review [1] I had another thought on this. You say that any address allocation won't be possible. I wonder if this is really true. The EUI-64 space is only one part in 64k of total /64 space. IPAM could allocate from the rest. There could be use cases for mixed modes in the future. I think best practice for any IPAM should be to steer clear of allocating address which could be EUI-64 addresses. Even if it ignores it completely, what are the chances that IPAM will allocate from that sub-space *and* allocate an address that would conflict? I'd say pretty low. If it did happened, IPAM would reject the address computed and the port create would fail I think. You'd try again and it would likely succeed. I may be over-simplifying. Carl __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
On 02/12/2015 02:36 PM, Salvatore Orlando wrote: - I promised a non blocking algorithm for IP allocation. The one I was developing was based on specifying the primary key on the ip_requests table in a way that it would prevent two concurrent requests from getting the same address, and would just retry getting an address until the primary key constraint was satisfied. However, recent information emerged on MySQL galera's (*) data set [2] certification clarified that this kind of algorithm would still result in a deadlock error from failed data set certification. It is worth noting that in this case a solution based on traditional compare-and-swap is not possible because concurrent requests would be inserting data at the same time. I am now working on an alternative solution, and I would like to first implement a PoC for it (so that I can prove it works). Would something like the following work: insert the data in the DB, if any error is got open a new transaction and try again ? enikanorov proposed a retry mechanism here [1] . Can't wait for your POC! I had been playing a while in the past to try to remove the locking from the IP allocation, it's hard! cheers, Rossella [1] https://review.openstack.org/#/c/149261/ __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
On 13 February 2015 at 12:40, Rossella Sblendido rsblend...@suse.com wrote: On 02/12/2015 02:36 PM, Salvatore Orlando wrote: - I promised a non blocking algorithm for IP allocation. The one I was developing was based on specifying the primary key on the ip_requests table in a way that it would prevent two concurrent requests from getting the same address, and would just retry getting an address until the primary key constraint was satisfied. However, recent information emerged on MySQL galera's (*) data set [2] certification clarified that this kind of algorithm would still result in a deadlock error from failed data set certification. It is worth noting that in this case a solution based on traditional compare-and-swap is not possible because concurrent requests would be inserting data at the same time. I am now working on an alternative solution, and I would like to first implement a PoC for it (so that I can prove it works). Would something like the following work: insert the data in the DB, if any error is got open a new transaction and try again ? enikanorov proposed a retry mechanism here [1] . Can't wait for your POC! I had been playing a while in the past to try to remove the locking from the IP allocation, it's hard! Retry is always an option, however the mechanism with the separate driver would be a bit different, since we need to identify conflicts before storing the IP allocation entry. As a matter of fact, we're indeed splitting the IPAM transaction from the transaction which creates or updates the port. The patch you linked is a mechanism for retrying a database transaction which can be adopted in any case, and is perhaps worth adopting also in the IPAM case - if nothing else to avoid code duplication. However, what I am aiming for is a lock-free and wait-free algorithm that does not make assumptions on the isolation level. If that would not be practical, there are several alternatives to be considered: - Leveraging unique constraint violations, and then trying different IPs until the constraint is not satisfied. This is apparently easy, but availability ranges can be quite tricky when you remove locking. - Trying whether ti would be possible do so some sort of compare and swap on IP availability ranges themselves. I think you were already developing something like that in [1] - Considering an alternative to availability ranges. Pre-generation of IP entries is unpractical (think IPv6), so that's not an option. Unfortunately, I have not yet explored in detail this route. Salvatore [1] https://review.openstack.org/#/c/100963/32/neutron/db/db_base_plugin_v2.py cheers, Rossella [1] https://review.openstack.org/#/c/149261/ __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
On 12 February 2015 at 19:57, John Belamaric jbelama...@infoblox.com wrote: From: Salvatore Orlando sorla...@nicira.com Reply-To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org Date: Thursday, February 12, 2015 at 8:36 AM To: OpenStack Development Mailing List openstack-dev@lists.openstack.org Subject: [openstack-dev] [Neutron] Update on DB IPAM driver Hi, I have updated the patch; albeit not complete yet it's kind of closer to be an allocator decent enough to replace the built-in logic. I will be unable to attend today's L3/IPAM meeting due to a conflict, so here are some highlights from me on which your feedback is more than welcome: - I agree with Carl that the IPAM driver should not have explicit code paths for autoaddress subnets, such as DHCPv6 stateless ones. In that case, the consumer of the driver will generate the address and then to the IPAM driver that would just be allocation of a specific address. However, I have the impression the driver still needs to be aware of whether the subnet has an automatic address mode or not - since in this case 'any' address allocation won't be possible. There already comments about this in the review [1] I think the auto-generated case should be a base class as you described in [1], but each subclass would implement the specific auto-generation. See the discussion at line 468 in [2] and see what you think. Of course for addresses that come from RA there would be no IPAM. I think this makes sense. [1] https://review.openstack.org/#/c/150485/ [2] https://review.openstack.org/#/c/153236/2/neutron/db/db_base_plugin_v2.py,unified - We had a discussion last week on whether the IPAM driver and neutron should 'share' database tables. I went back and forth a lot of time, but now to me it seems the best thing to do is to have the IPAM driver maintain an 'ip_requests' tables, where it stores allocation info. This table partially duplicates data in IPAllocation, but on the plus side it makes the IPAM driver self sufficient. The next step would be to decide whether we want to go a step further and also assume the driver should not access at all Neutron's DB, but I would defer that discussion to the next iteration (for both the driver and the IPAM interface) - I promised a non blocking algorithm for IP allocation. The one I was developing was based on specifying the primary key on the ip_requests table in a way that it would prevent two concurrent requests from getting the same address, and would just retry getting an address until the primary key constraint was satisfied. However, recent information emerged on MySQL galera's (*) data set [2] certification clarified that this kind of algorithm would still result in a deadlock error from failed data set certification. It is worth noting that in this case a solution based on traditional compare-and-swap is not possible because concurrent requests would be inserting data at the same time. I am now working on an alternative solution, and I would like to first implement a PoC for it (so that I can prove it works). - The db base refactoring being performed by Pavel is under way [3]. It is worth noting that this is a non-negligible change to some of Neutron's basic and more critical workflows. We should expect pushback from the community regarding the introduction of this change in the 3rd milestone. At this stage I would suggest either: A) consider a strategy for running pluggable IPAM as optional B) consider delaying to Liberty. (and that's where I get virtually jeered and pelted with rotten tomatoes) I wish I had some old tomatoes! Seriously, I think A is a reasonable approach. To make this really explicit we may want to basically replace the DB plugin class with a shim that delegates to either the current implementation or the new implementation, depending on the flag. The fact that the current implementation is pretty much a bunch of private methods in the db base plugin class executed within a transaction for creating a port makes the situation a wee bit more complex. I'm not sure we can replace the db plugin class with a shim so easily, because we need to consider the impact on plugins which inherit from this base class. For instance some plugins override methods from the base class, and this would be a problem. For those plugins we must ensure old-style IPAM is performed. A transitory solution might be to have, for the relevant methods 2 versions - one would be the current one, and the other one would be the one leveraging pluggable IPAM. During plugin initialisation, the plugin itself will decide whether use or not the latter methods. This might be tuneable with a configuration parameter too. The downside of this approach is that it will not allow us to remove old baked in IPAM code, and will have an impact on code maintainability which ultimately will result in accumulating even
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
Put it in this way, it also makes sense. But I think I need to see it translated in code to figure it out properly. Anyway, this is something which pertains the base classes rather than the reference driver. I think from the perspective of the reference driver we should just raise if a AnyAddressRequest is sent for a subnet where addresses are supposed to be autogenerated, because the ipam driver won't generate the address. Makes sense. Hmm. How dynamic is Python? I know in Ruby I could do something like this at class load time: config.use_ipam ? DbBasePluginV2 = IpamDbBasePluginV2 : DbBasePluginV2 = LegacyDbBasePluginV2 and all the subclasses would work fine as before... Technically yes. From a practical perspective however if the subclass is assuming that create_port works in the old way, and then instead is working in the ipam way, it might be mayhem! Yes, certainly. But it provides a transition period for plugins to migrate to support the new IPAM model. __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
From: Salvatore Orlando sorla...@nicira.commailto:sorla...@nicira.com Reply-To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org Date: Friday, February 13, 2015 at 8:26 AM To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org Subject: Re: [openstack-dev] [Neutron] Update on DB IPAM driver ... I think the auto-generated case should be a base class as you described in [1], but each subclass would implement the specific auto-generation. See the discussion at line 468 in [2] and see what you think. Of course for addresses that come from RA there would be no IPAM. I think this makes sense. Thinking a little more on this, in the case of magic address prefixes, we probably should have the factory method generate the right request class. That way, the logic for those magic prefixes is all in one place. You could still specify the class in the request but the magic prefixes would take priority. [1] https://review.openstack.org/#/c/150485/ [2] https://review.openstack.org/#/c/153236/2/neutron/db/db_base_plugin_v2.py,unified - The db base refactoring being performed by Pavel is under way [3]. It is worth noting that this is a non-negligible change to some of Neutron's basic and more critical workflows. We should expect pushback from the community regarding the introduction of this change in the 3rd milestone. At this stage I would suggest either: A) consider a strategy for running pluggable IPAM as optional B) consider delaying to Liberty. (and that's where I get virtually jeered and pelted with rotten tomatoes) I wish I had some old tomatoes! Seriously, I think A is a reasonable approach. To make this really explicit we may want to basically replace the DB plugin class with a shim that delegates to either the current implementation or the new implementation, depending on the flag. The fact that the current implementation is pretty much a bunch of private methods in the db base plugin class executed within a transaction for creating a port makes the situation a wee bit more complex. I'm not sure we can replace the db plugin class with a shim so easily, because we need to consider the impact on plugins which inherit from this base class. For instance some plugins override methods from the base class, and this would be a problem. For those plugins we must ensure old-style IPAM is performed. A transitory solution might be to have, for the relevant methods 2 versions - one would be the current one, and the other one would be the one leveraging pluggable IPAM. During plugin initialisation, the plugin itself will decide whether use or not the latter methods. This might be tuneable with a configuration parameter too. The downside of this approach is that it will not allow us to remove old baked in IPAM code, and will have an impact on code maintainability which ultimately will result in accumulating even more technical debt. However, I might be missing some better alternative, so if you have any proposal just let me know. Hmm. How dynamic is Python? I know in Ruby I could do something like this at class load time: config.use_ipam ? DbBasePluginV2 = IpamDbBasePluginV2 : DbBasePluginV2 = LegacyDbBasePluginV2 and all the subclasses would work fine as before... __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Neutron] Update on DB IPAM driver
From: Salvatore Orlando sorla...@nicira.commailto:sorla...@nicira.com Reply-To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org Date: Thursday, February 12, 2015 at 8:36 AM To: OpenStack Development Mailing List openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org Subject: [openstack-dev] [Neutron] Update on DB IPAM driver Hi, I have updated the patch; albeit not complete yet it's kind of closer to be an allocator decent enough to replace the built-in logic. I will be unable to attend today's L3/IPAM meeting due to a conflict, so here are some highlights from me on which your feedback is more than welcome: - I agree with Carl that the IPAM driver should not have explicit code paths for autoaddress subnets, such as DHCPv6 stateless ones. In that case, the consumer of the driver will generate the address and then to the IPAM driver that would just be allocation of a specific address. However, I have the impression the driver still needs to be aware of whether the subnet has an automatic address mode or not - since in this case 'any' address allocation won't be possible. There already comments about this in the review [1] I think the auto-generated case should be a base class as you described in [1], but each subclass would implement the specific auto-generation. See the discussion at line 468 in [2] and see what you think. Of course for addresses that come from RA there would be no IPAM. [1] https://review.openstack.org/#/c/150485/ [2] https://review.openstack.org/#/c/153236/2/neutron/db/db_base_plugin_v2.py,unified - We had a discussion last week on whether the IPAM driver and neutron should 'share' database tables. I went back and forth a lot of time, but now to me it seems the best thing to do is to have the IPAM driver maintain an 'ip_requests' tables, where it stores allocation info. This table partially duplicates data in IPAllocation, but on the plus side it makes the IPAM driver self sufficient. The next step would be to decide whether we want to go a step further and also assume the driver should not access at all Neutron's DB, but I would defer that discussion to the next iteration (for both the driver and the IPAM interface) - I promised a non blocking algorithm for IP allocation. The one I was developing was based on specifying the primary key on the ip_requests table in a way that it would prevent two concurrent requests from getting the same address, and would just retry getting an address until the primary key constraint was satisfied. However, recent information emerged on MySQL galera's (*) data set [2] certification clarified that this kind of algorithm would still result in a deadlock error from failed data set certification. It is worth noting that in this case a solution based on traditional compare-and-swap is not possible because concurrent requests would be inserting data at the same time. I am now working on an alternative solution, and I would like to first implement a PoC for it (so that I can prove it works). - The db base refactoring being performed by Pavel is under way [3]. It is worth noting that this is a non-negligible change to some of Neutron's basic and more critical workflows. We should expect pushback from the community regarding the introduction of this change in the 3rd milestone. At this stage I would suggest either: A) consider a strategy for running pluggable IPAM as optional B) consider delaying to Liberty. (and that's where I get virtually jeered and pelted with rotten tomatoes) I wish I had some old tomatoes! Seriously, I think A is a reasonable approach. To make this really explicit we may want to basically replace the DB plugin class with a shim that delegates to either the current implementation or the new implementation, depending on the flag. Thanks for reading this post, Salvatore [1] https://review.openstack.org/#/c/150485/ [2] http://lists.openstack.org/pipermail/openstack-dev/2015-February/056007.html [3] https://review.openstack.org/#/c/153236/ __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
