Ben Pfaff <b...@ovn.org> wrote on 04/22/2016 04:04:11 PM: > From: Ben Pfaff <b...@ovn.org> > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: ovs dev <dev@openvswitch.org> > Date: 04/22/2016 04:04 PM > Subject: Re: [ovs-dev] [RFC] Idea for fixing "raceful" E2E ovn tests > > On Fri, Apr 22, 2016 at 03:56:52PM -0500, Ryan Moats wrote: > > Ben Pfaff <b...@ovn.org> wrote on 04/22/2016 03:06:16 PM: > > > > > From: Ben Pfaff <b...@ovn.org> > > > To: Ryan Moats/Omaha/IBM@IBMUS > > > Cc: ovs dev <dev@openvswitch.org> > > > Date: 04/22/2016 03:06 PM > > > Subject: Re: [ovs-dev] [RFC] Idea for fixing "raceful" E2E ovn tests > > > > > > On Thu, Apr 14, 2016 at 08:37:27AM -0500, Ryan Moats wrote: > > > > I've pretty much become fed up with the "raceful" nature of the E2E > > > > ovn test cases and so I've set my self the goal of fixing them. > > > > > > > > After some thought last night, I *think* I might have found a way > > > > to do it. Now, since I'm not 100% that my idea is the cleanest way > > > > to fix things, I thought I'd throw the idea out on the table first > > > > and get some feedback before I go off and write code. > > > > > > > > I'm thinking of the following changes: > > > > > > > > in ovn/controller/ofctrl.c, change ofctrl_put to return a boolean > > > > (true if a flow is queue, false otherwise). > > > > > > > > in ovn/controller/ovn-controller.c: > > > > 1. add a command line argument (--unit-test). > > > > 2. if ofctrl_put return true and the above command line argument > > > > is specified, dump a line to the log file saying that ovn-controller > > > > hasn't made any OF changes in the last loop > > > > > > > > in the ovn E2E tests cases: > > > > 1. start ovn-controller with the --unit-test argument > > > > 2. instead of sleep 1 for waiting for ovn-northd/ovn-controller to > > > > quiesce, look at the tail of the ovn-controller logs for two > > > > consecutive loops where ovn-controller hasn't made any OF changes > > > > in the last loop > > > > > > I've had a different idea for how to do this for a while now. Let me > > > see... > > > > > > You might not be familiar with how ovs-vsctl waits until the changes > > > that it makes to the Open vSwitch configuration take effect before > > > exiting. It uses a sequence number protocol in the Open_vSwitch > > > database table. This table (which has exactly one row) has two integer > > > columns, named cur_cfg and next_cfg. Initially cur_cfg and next_cfg are > > > both zero. When ovs-vsctl modifies the configuration, it also > > > atomically increments next_cfg. Before ovs-vswitchd reconfigures > > > itself, it reads next_cfg, then when it finishes it sets cur_cfg to the > > > next_cfg value that it read. ovs-vsctl waits until cur_cfg is greater > > > than or equal to the next_cfg that it set before it exits. > > > > > > This has a number of nice properties. It has low overhead in time and > > > space, it works efficiently with any number of writers, and later on it > > > is easy to observe what happened and when by looking at the database > > > log. > > > > > > My thought was to extend this concept to the OVN distributed system. > > > For example: > > > > > > * Add a next_cfg value somewhere in the OVN_Northbound > > > database. When ovn-nbctl or something else updates the > > > database and wants to allow for finding out where the updates > > > are propagated, it increments next_cfg. > > > > > > * Add a similar next_cfg somewhere in OVN_Southbound, and make > > > ovn-northd propagate the value from northbound to southbound. > > > > > > * Add a cur_cfg column to the Chassis table in OVN_Southbound. > > > When an ovn-controller brings its chassis up to date with a > > > given configuration, it stores the next_cfg value that it just > > > got up-to-date with into its own cur_cfg. > > > > > > * In theory ovn-northd could propagate the current minimum value > > > of cur_cfg back to the northbound database, if that's useful. > > > In a real system with constant failures though it's hard for > > > me to guess whether it's realistic. > > > > > > This could even be used such that ovn-nbctl waits for the whole > > > distributed system to catch up before it exits. > > > > > > What are your thoughts? > > > > This is definitely an interesting idea, but I'm not convinced of the last > > two items. I agree that while having ovn-northd propagate the current > > minimum value of cur_cfg up is good in theory, the idea of blocking > > ovn-nbctl from exiting until everything catches up worries me. For > > example, > > what happens if the SB Chassis table has an orphaned entry due to error > > conditions? Does that lead to the provisioning plane shutting down because > > ovn-nbctl never exits? > > > > I think I'd rather see ovn-nbctl exit immediately upon making changes and > > we provide an additional commands to ovn-nbctl that either returns a status > > code based on whether the system is up to date or not and/or the values of > > the next_cfg and cur_cfg columns (I assume that ovn-sbctl will show the > > next_cfg and cur_cfg for chassis if that table is shown). That way we can > > continue making progress in the case of corner failure cases and have a > > way to identify which row(s) in the chassis table form the source of a > > problem. > > > > Thus (to close the loop), the test case would go ahead and make the needed > > changes with ovn-nbctl and then have a gate where it waits until ovn-nbctl > > reports that the system is up to date, at which point the test packets can > > be sent. > > The default for ovs-vsctl is to wait, and one can override it with > --no-wait. It would be fine with me for the default for ovn-nbctl to be > not to wait and to provide a --wait option if one does want to wait. I > agree that a large, real system is going to have failures that mean that > it may be impractical to wait by default, but I also think that the > ability to wait is going to be valuable for unit tests especially.
Sure, the idea of having a --wait flag for unit tests make sense to me, but for the real world, I still want a way to look at the nb DB's next_cfg and cur_cfg values as part of a hypothetical health monitoring system. Before I forget (again), I'm still willing to take a swag at coding this all up once I'm back from Austin. Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev