[ovs-dev] [PATCH v2] ovn: Avoid nb_cfg update notification flooding

Ben Pfaff blp at ovn.org
Thu Apr 12 17:19:55 UTC 2018


On Wed, Apr 11, 2018 at 11:29:28PM -0700, Han Zhou wrote:
> On Tue, Apr 10, 2018 at 6:21 PM, Han Zhou <zhouhan at gmail.com> wrote:
> >
> >
> >
> > On Tue, Apr 10, 2018 at 5:04 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > > On Fri, Apr 06, 2018 at 02:40:21PM -0700, Han Zhou wrote:
> > > > On Fri, Apr 6, 2018 at 1:54 PM, Ben Pfaff <blp at ovn.org> wrote:
> > > > >
> > > > > Thanks for working on making OVN faster and scale better.
> > > > >
> > > > > I see what you mean about how nb_cfg can be a scale problem.
> Really,
> > > > > each hypervisor only cares about its own row, but each of them is
> being
> > > > > notified about the current value in every row.  Ideally, we want
> the HVs
> > > > > to be able to say to ovsdb-server, "Give me all the data in the
> Chassis
> > > > > table, except don't bother with nb_cfg in any rows except my own
> row."
> > > > >
> > > > > Actually, there's a way for ovn-controller to do that: the OVSDB
> > > > > protocol definition of monitor_cond allows the client to specify
> > > > > multiple sets of columns to monitor in a table, and each set of
> columns
> > > > > has an independently attached condition.  This can be used to
> specify
> > > > > that most of the columns are monitored unconditionally but nb_cfg is
> > > > > monitored only for a particular row.
> > > > >
> > > > > But there are still problems:
> > > > >
> > > > > 1. The implementation in ovsdb-server looks broken.  I don't think
> it
> > > > >    implements the spec.  It certainly isn't tested--none of the
> existing
> > > > >    clients ever passes more than a single set of columns.  I think
> that
> > > > >    it will work if all the sets of columns use the same condition,
> but
> > > > >    that doesn't help with this case.  It will need to be fixed.
> > > > >
> > > > > 2. The client implementation in ovsdb-idl doesn't anticipate
> difference
> > > > >    conditions for different columns.  It will need to be enhanced to
> > > > >    support this use.
> > > > >
> > > > > In the short term this is going to be more work than just creating
> a new
> > > > > table.  In the long term, though, it will allow us to be more
> flexible
> > > > > and more agile, since improvements similar to the one in this patch
> will
> > > > > require only client changes, not database schema changes.
> > > > >
> > > > > Comments?
> > > >
> > > > Thanks for the valuable information! I didn't even know that ovsdb is
> > > > supposed to support multiple set columns with different monitor
> conditions.
> > > > It surely would be the right way to go if it is supported. However,
> as you
> > > > mentioned the mechanism doesn't work yet and I am not sure how much
> work is
> > > > needed to make it work, it seems reasonable to me to "workaround" it
> at
> > > > least for short term until that is ready in the future. Then we could
> > > > simply resurrect the old column in the old table with the new monitor
> > > > conditions. What do you think?
> > >
> > > This kind of workaround would essentially persist forever.  We'd never
> > > be able to get rid of it--or, more to the point, we would never be able
> > > to justify the work.  We'd be stuck with something ugly.
> > >
> > > So: if you think the workaround is valuable, then one way to justify it
> > > would be to reformulate it in a way that isn't so ugly.  As currently
> > > written, when a person looks at the table structure, (s)he notices a
> > > table that has a single column and is named for that single column.
> > > That sticks out, clearly, as some kind of compromise, and the
> > > documentation even calls it out as an optimization.  But there might be
> > > a logical, conceptual reason why it makes sense to have a separate table
> > > for some kinds of chassis-related data.  If there is, then that would
> > > suggest a name for the table, and for the class of data that should
> > > should go in there, and it would make it possible to have something that
> > > is both faster and a reasonable design.
> > >
> > > I'm not saying that there is necessarily a logical, conceptual reason
> > > that one can come up here, but if there is then it would certainly make
> > > it much easier to support the patch.
> >
> > Hi Ben, I'm not sure if I got your point completely. A logical reason for
> this separate table might be for holding chassis private data, the data
> that other chassis is not interested in. Of course the reason for holding
> such private data in a separate table is still this huge performance
> difference as it is shown in the test result. So I think the table can be
> named as Chassis_Private_Data, and I can document in ovn-sb.xml more about
> the performance considerations, and the future improvement about the
> monitor_cond. Would this address your point or did I totally missed your
> point?
> >
> Hi Ben, could you confirm so that I can work on v3?
> 
> > Thanks,
> > Han

That sounds like a good direction.


More information about the dev mailing list