[ovs-dev] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

[Ryan Moats]

"dev" <dev-bounces at openvswitch.org> wrote on 08/04/2016 10:34:08 AM:

> From: Ryan Moats/Omaha/IBM at IBMUS
> To: Ben Pfaff <blp at ovn.org>
> Cc: ovs-dev <dev at openvswitch.org>
> Date: 08/04/2016 10:34 AM
> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> we've see scaling the networking-ovn to NB DB connection
> Sent by: "dev" <dev-bounces at openvswitch.org>
>
> "dev" <dev-bounces at openvswitch.org> wrote on 08/03/2016 04:53:42 PM:
>
> > From: Ben Pfaff <blp at ovn.org>
> > To: Russell Bryant <russell at ovn.org>
> > Cc: ovs-dev <dev at openvswitch.org>
> > Date: 08/03/2016 04:54 PM
> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > we've see scaling the networking-ovn to NB DB connection
> > Sent by: "dev" <dev-bounces at openvswitch.org>
> >
> > On Wed, Aug 03, 2016 at 11:58:52AM -0400, Russell Bryant wrote:
> > > On Wed, Aug 3, 2016 at 11:39 AM, Kyle Mestery <mestery at mestery.com>
> wrote:
> > >
> > > > On Wed, Aug 3, 2016 at 10:30 AM, Ryan Moats <rmoats at us.ibm.com>
> wrote:
> > > > >
> > > > > Russell Bryant <russell at ovn.org> wrote on 08/03/2016 10:11:57 AM:
> > > > >
> > > > >> From: Russell Bryant <russell at ovn.org>
> > > > >> To: Ryan Moats/Omaha/IBM at IBMUS
> > > > >> Cc: Ben Pfaff <blp at ovn.org>, ovs-dev <dev at openvswitch.org>
> > > > >> Date: 08/03/2016 10:12 AM
> > > > >> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > > > >> we've see scaling the networking-ovn to NB DB connection
> > > > >>
> > > > >> On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats <rmoats at us.ibm.com>
> wrote:
> > > > >>
> > > > >>
> > > > >> Ben Pfaff <blp at ovn.org> wrote on 08/03/2016 12:27:48 AM:
> > > > >>
> > > > >> > From: Ben Pfaff <blp at ovn.org>
> > > > >> > To: Ryan Moats/Omaha/IBM at IBMUS
> > > > >> > Cc: ovs-dev <dev at openvswitch.org>
> > > > >> > Date: 08/03/2016 12:28 AM
> > > > >> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I -
things
> > > > >> > we've see scaling the networking-ovn to NB DB connection
> > > > >> >
> > > > >> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> > > > >> > > Ben Pfaff <blp at ovn.org> wrote on 08/02/2016 11:52:23 PM:
> > > > >> > >
> > > > >> > > > From: Ben Pfaff <blp at ovn.org>
> > > > >> > > > To: Ryan Moats/Omaha/IBM at IBMUS
> > > > >> > > > Cc: ovs-dev <dev at openvswitch.org>
> > > > >> > > > Date: 08/02/2016 11:52 PM
> > > > >> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I -
> things
> > > > >> > > > we've see scaling the networking-ovn to NB DB connection
> > > > >> > > >
> > > > >> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats
wrote:
> > > > >> > > > > "dev" <dev-bounces at openvswitch.org> wrote on 08/02/2016
> > > > 10:56:07
> > > > >> PM:
> > > > >> > > > > > Ben Pfaff <blp at ovn.org> wrote on 08/02/2016 10:14:46
PM:
> > > > >> > > > > > > Presumably this means that networking-ovn is calling
> > > > "verify"
> > > > >> on
> > > > >> > > the
> > > > >> > > > > > > column in question.  Probably, networking-ovn
> > should use the
> > > > >> > > partial
> > > > >> > > > > map
> > > > >> > > > > > > update functionality introduced in commit
> f199df26e8e28
> > > > >> "ovsdb-idl:
> > > > >> > > Add
> > > > >> > > > > > > partial map updates functionality."  I don't know
> whether
> > > > > it's
> > > > >> in
> > > > >> > > the
> > > > >> > > > > > > Python IDL yet.
> > > > >> > > > > >
> > > > >> > > > > > Indeed they are and thanks for the pointer to the
commit
> -
> > > > I'll
> > > > >> dig
> > > > >> > > > > > into it tomorrow and see if that code is reflected in
> the
> > > > > Python
> > > > >> > > > > > IDL via that or another commit.  If it is, great.  If
> not,
> > > > > there
> > > > >> > > > > > will likely also be a patch adding it so that we can
> move
> > > > > along.
> > > > >> > > > >
> > > > >> > > > > Hmm, maybe I'm misreading something, but I don't thing
> that's
> > > > > going
> > > > >> > > > > to work without some additional modifications - the
> partial map
> > > > >> commit
> > > > >> > > > > currently codes for columns that have a particular value
> type
> > > > >> defined
> > > > >> > > > > by the schema.  The problem we are seeing is with
theports
> and
> > > > >> acls
> > > > >> > > > > columns of the logical switch table, which are lists of
> strong
> > > > >> > > > > references.  Since they don't have a defined value, the
> > > > generated
> > > > >> IDL
> > > > >> > > > > code doesn't provide hooks for using partial map
> operations and
> > > > > we
> > > > >> > > default
> > > > >> > > > > back to update/verify with the given above results.
> > > > >> > > > >
> > > > >> > > > > Now, I think this an oversight, because I can argue that
> since
> > > > >> these
> > > > >> > > > > are strong references, I should be able to use partial
> maps to
> > > > >> update
> > > > >> > > > > them as keys with a null value.  Does this make sense or
> am I
> > > > >> breaking
> > > > >> > > > > something if I look at going this route?
> > > > >> > > >
> > > > >> > > > If they're implemented as partial map operations only,
then
> we
> > > > > should
> > > > >> > > > extend them to support partial set operations too--the
same
> > > > >> principles
> > > > >> > > > apply.
> > > > >> > >
> > > > >> > > I'm not sure I parsed this correctly, but I think we are
> saying the
> > > > >> same
> > > > >> > > thing: change the IDL for columns that contain sets of
strong
> > > > >> references
> > > > >> > > from using update/verify to using mutate for partial set
> operations
> > > > > (I
> > > > >> > > realized after hitting the send button that I should have
said
> > > > > partial
> > > > >> > > sets instead of partial maps...)
> > > > >> >
> > > > >> > Yes, I think we're saying the same thing.
> > > > >> >
> > > > >
> > > > >> Cool, I'll see how far I can get (not sure where my previous
> message
> > > > > saying
> > > > >> this went...)
> > > > >>
> > > > >> From the perspective of the branch point, we'd really like to
see
> the
> > > > >> following make the 2.6 release to allow for easier integration
> with
> > > > >> upstream
> > > > >> OpenStack testing:
> > > > >>
> > > > >> - partial sets for the C IDL
> > > > >> - partial maps for the Python IDL
> > > > >> - partial sets for the Python IDL
> > > > >>
> > > > >> In fact, I'd sorta like to see them all backported to 2.5, but I
> doubt
> > > > >> that's
> > > > >> possible.
> > > > >>
> > > > >> (...off to craft patch sets...)
> > > > >>
> > > > >> but based on:
> > > > >>
> > > > >> http://openvswitch.org/pipermail/dev/2016-July/076855.html
> > > > >>
> > > > >> it seems too late to try to get new things like this in for 2.6.
> > > > >
> > > > > I realize that I'm asking for an exception - at the time I wrote
> that
> > > > > we didn't realize how much of an issue using verify/update
> semantics was
> > > > > going to be when scaling (mea culpa).
> > > > >
> > > > > If the exception isn't granted, so be it - we'll carry it locally
> until
> > > > > the 2.6.90 branch opens...
> > > > >
> > > >
> > > > I'll jump in here and say that this issue has become a huge scaling
> > > > issue for us internally, and anyone else who wants to scale OVN.
I'd
> > > > also like to request a feature exception for this. I realize we
have
> a
> > > > similar issue to fight on the OpenStack side, but without these
> > > > changes, we'll be severely limiting the scalability of OVN. And
this
> > > > is even without OpenStack, as Ryan and I suspect this issue is the
> > > > cause of the "8k ports on 400 chassis is really slow as it grows"
> test
> > > > with ovn-scale-test.
> > >
> > >
> > > OK.  I suppose we can't really discuss an exception until the code is
> > > ready, anyway.
> >
> > Yes, let's see the code...
>
> Ask and ye shall receive.  The cover letter is at [1], the patch sets in
> patchworks at [2] and [3]...
>
> Ryan
>
> [1] http://openvswitch.org/pipermail/dev/2016-August/077238.html
> [2] http://patchwork.ozlabs.org/patch/655841/
> [3] http://patchwork.ozlabs.org/patch/655842/

The above patches have been superceded [1] and [2], but Kyle has added
comments about the performance gain we are seeing with the new approach
at [3]

Ryan

[1] http://patchwork.ozlabs.org/patch/656271/
[2] http://patchwork.ozlabs.org/patch/656272/
[3] http://openvswitch.org/pipermail/dev/2016-August/077308.html