Re: [ovs-dev] incremental updates (was: Re: [PATCH 2/2] ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.)
Ben Pfaff wrote on 08/19/2016 02:36:42 PM: > From: Ben Pfaff > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: ovs dev , Numan Siddique > Date: 08/19/2016 02:37 PM > Subject: Re: incremental updates (was: Re: [ovs-dev] [PATCH 2/2] > ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.) > > On Fri, Aug 19, 2016 at 10:24:21AM -0500, Ryan Moats wrote: > > Ben Pfaff wrote on 08/19/2016 10:11:59 AM: > > > > > From: Ben Pfaff > > > To: Ryan Moats/Omaha/IBM@IBMUS > > > Cc: Numan Siddique , ovs dev > > > Date: 08/19/2016 10:12 AM > > > Subject: incremental updates (was: Re: [ovs-dev] [PATCH 2/2] ovn/ > > > TODO: Add items proposed for 2.7 in OVN IRC meeting.) > > > > > > On Fri, Aug 19, 2016 at 08:34:45AM -0500, Ryan Moats wrote: > > > > We've been running incremental processing in ovn-controller here for > > quite > > > > a while (even before it merged officially) and given our experience, > > > > I've been doing some hard thinking about it. > > > > > > > > The original goal of the patch set was to allow ovn-controller to use > > > > incremental updates as much as possible. However, the range of > > possible > > > > changes in inputs resulted in ovn-controller having to continue to > > > > maintain the ability to run a full update in those cases where > > previously > > > > calculated rules are no longer correct. > > > > > > > > What we've found is that most configuration events from our CMS > > (Neutron) > > > > end up triggering a full ovn-controller update, and so the end result > > of > > > > the incremental processing code is not that ovn-controller is doing > > > > incremental updates most of the time, but rather that ovn-controller > > > > doesn't recalculate changes in-between modifications. > > > > > > > > While we still feel that the above is a win, I'm coming to the > > conclusion > > > > that the current code base has added unnecessary complexity to achieve > > > > this. Based on this, I'm thinking of the following approach: > > > > > > > > 1) going back to doing full processing every cycle, while still > > > > keeping the persistence of items where we can, because I feel that > > > > persistence has allowed us to handle cases where we need to skip a poll > > > > cycle that we didn't have before and that has improved things. > > > > > > What I'm hearing here is that we'll do full processing if any processing > > > at all is necessary. Seems fine to me. > > > > > > > 2) introducing a new command flag to allow those that don't want to > > > > run in what I'm now calling quiet mode to continue to do full > > processing > > > > every cycle. (In retrospect, I should have proposed this up front for > > i-p > > > > to allow for better isolation of that code, but as they say, hindsight > > > > is always 20/20). > > > > > > Hmm. I would hope that "Full processing if any processing" is fairly > > > easy to get right. However, I'm OK with having a flag as a fail-safe. > > > In time, I would hope that we could delete the flag. > > > > > > > 3) For quiet mode, check the integration bridge and Ben's sequence > > > > number information in the SB database to determine if anything has > > > > changed since the last cycle. If something has changed, run the full > > > > processing code. If not, quiesce for a poll cycle. > > > > > > I don't think that the sequence numbers are a good way to determine > > > whether something has changed. Without --wait, for example, ovn-nbctl > > > doesn't change the sequence number. > > > > Yes on #1 - that's the intent. > > > > Apologies for not being more clear that in re my thinking for suggesting > > the command line flag. I agree that we'd deprecate it long term if we > > added it, but (and now I move to #3) > > > > I know that sequence numbers are only there today if asked for, and I was > > thinking of arguing that they should always be there and that ovn-nbctl > > should *check* them only if --wait is set. However, there might be > > other ways to figure out if the SB DBs have changed (I'll admit, I've not > > thought of any yet - anybody have any ideas?) > > The IDL has its own sequence number for that, see ovsdb_idl_get_seqno(). > Yep, that helped turn the trick - ran into a few other nits, so I broke the patches in two: http://patchwork.ozlabs.org/patch/661159/ http://patchwork.ozlabs.org/patch/661161/ The unit test cases pass for me, but it would be good if folks take a look at both as extra eyes are always good... ___ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] incremental updates (was: Re: [PATCH 2/2] ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.)
On Fri, Aug 19, 2016 at 10:24:21AM -0500, Ryan Moats wrote: > Ben Pfaff wrote on 08/19/2016 10:11:59 AM: > > > From: Ben Pfaff > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: Numan Siddique , ovs dev > > Date: 08/19/2016 10:12 AM > > Subject: incremental updates (was: Re: [ovs-dev] [PATCH 2/2] ovn/ > > TODO: Add items proposed for 2.7 in OVN IRC meeting.) > > > > On Fri, Aug 19, 2016 at 08:34:45AM -0500, Ryan Moats wrote: > > > We've been running incremental processing in ovn-controller here for > quite > > > a while (even before it merged officially) and given our experience, > > > I've been doing some hard thinking about it. > > > > > > The original goal of the patch set was to allow ovn-controller to use > > > incremental updates as much as possible. However, the range of > possible > > > changes in inputs resulted in ovn-controller having to continue to > > > maintain the ability to run a full update in those cases where > previously > > > calculated rules are no longer correct. > > > > > > What we've found is that most configuration events from our CMS > (Neutron) > > > end up triggering a full ovn-controller update, and so the end result > of > > > the incremental processing code is not that ovn-controller is doing > > > incremental updates most of the time, but rather that ovn-controller > > > doesn't recalculate changes in-between modifications. > > > > > > While we still feel that the above is a win, I'm coming to the > conclusion > > > that the current code base has added unnecessary complexity to achieve > > > this. Based on this, I'm thinking of the following approach: > > > > > > 1) going back to doing full processing every cycle, while still > > > keeping the persistence of items where we can, because I feel that > > > persistence has allowed us to handle cases where we need to skip a poll > > > cycle that we didn't have before and that has improved things. > > > > What I'm hearing here is that we'll do full processing if any processing > > at all is necessary. Seems fine to me. > > > > > 2) introducing a new command flag to allow those that don't want to > > > run in what I'm now calling quiet mode to continue to do full > processing > > > every cycle. (In retrospect, I should have proposed this up front for > i-p > > > to allow for better isolation of that code, but as they say, hindsight > > > is always 20/20). > > > > Hmm. I would hope that "Full processing if any processing" is fairly > > easy to get right. However, I'm OK with having a flag as a fail-safe. > > In time, I would hope that we could delete the flag. > > > > > 3) For quiet mode, check the integration bridge and Ben's sequence > > > number information in the SB database to determine if anything has > > > changed since the last cycle. If something has changed, run the full > > > processing code. If not, quiesce for a poll cycle. > > > > I don't think that the sequence numbers are a good way to determine > > whether something has changed. Without --wait, for example, ovn-nbctl > > doesn't change the sequence number. > > Yes on #1 - that's the intent. > > Apologies for not being more clear that in re my thinking for suggesting > the command line flag. I agree that we'd deprecate it long term if we > added it, but (and now I move to #3) > > I know that sequence numbers are only there today if asked for, and I was > thinking of arguing that they should always be there and that ovn-nbctl > should *check* them only if --wait is set. However, there might be > other ways to figure out if the SB DBs have changed (I'll admit, I've not > thought of any yet - anybody have any ideas?) The IDL has its own sequence number for that, see ovsdb_idl_get_seqno(). ___ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] incremental updates (was: Re: [PATCH 2/2] ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.)
Ben Pfaff wrote on 08/19/2016 10:11:59 AM: > From: Ben Pfaff > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: Numan Siddique , ovs dev > Date: 08/19/2016 10:12 AM > Subject: incremental updates (was: Re: [ovs-dev] [PATCH 2/2] ovn/ > TODO: Add items proposed for 2.7 in OVN IRC meeting.) > > On Fri, Aug 19, 2016 at 08:34:45AM -0500, Ryan Moats wrote: > > We've been running incremental processing in ovn-controller here for quite > > a while (even before it merged officially) and given our experience, > > I've been doing some hard thinking about it. > > > > The original goal of the patch set was to allow ovn-controller to use > > incremental updates as much as possible. However, the range of possible > > changes in inputs resulted in ovn-controller having to continue to > > maintain the ability to run a full update in those cases where previously > > calculated rules are no longer correct. > > > > What we've found is that most configuration events from our CMS (Neutron) > > end up triggering a full ovn-controller update, and so the end result of > > the incremental processing code is not that ovn-controller is doing > > incremental updates most of the time, but rather that ovn-controller > > doesn't recalculate changes in-between modifications. > > > > While we still feel that the above is a win, I'm coming to the conclusion > > that the current code base has added unnecessary complexity to achieve > > this. Based on this, I'm thinking of the following approach: > > > > 1) going back to doing full processing every cycle, while still > > keeping the persistence of items where we can, because I feel that > > persistence has allowed us to handle cases where we need to skip a poll > > cycle that we didn't have before and that has improved things. > > What I'm hearing here is that we'll do full processing if any processing > at all is necessary. Seems fine to me. > > > 2) introducing a new command flag to allow those that don't want to > > run in what I'm now calling quiet mode to continue to do full processing > > every cycle. (In retrospect, I should have proposed this up front for i-p > > to allow for better isolation of that code, but as they say, hindsight > > is always 20/20). > > Hmm. I would hope that "Full processing if any processing" is fairly > easy to get right. However, I'm OK with having a flag as a fail-safe. > In time, I would hope that we could delete the flag. > > > 3) For quiet mode, check the integration bridge and Ben's sequence > > number information in the SB database to determine if anything has > > changed since the last cycle. If something has changed, run the full > > processing code. If not, quiesce for a poll cycle. > > I don't think that the sequence numbers are a good way to determine > whether something has changed. Without --wait, for example, ovn-nbctl > doesn't change the sequence number. Yes on #1 - that's the intent. Apologies for not being more clear that in re my thinking for suggesting the command line flag. I agree that we'd deprecate it long term if we added it, but (and now I move to #3) I know that sequence numbers are only there today if asked for, and I was thinking of arguing that they should always be there and that ovn-nbctl should *check* them only if --wait is set. However, there might be other ways to figure out if the SB DBs have changed (I'll admit, I've not thought of any yet - anybody have any ideas?) ___ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
