[ovs-dev] [PATCH 2/5] ovn-controller: Pass around pointers to individual tables.

[Han Zhou]

On Mon, Jun 11, 2018 at 2:01 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> On Mon, Jun 11, 2018 at 11:23:58AM -0700, Han Zhou wrote:
> > On Fri, Jun 8, 2018 at 2:59 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > > We're working to make ovn-controller compute more incrementally, to
reduce
> > > CPU usage.  To make it easier to keep track of dependencies, it makes
> > sense
> > > to pass around pointers to fine-grained resources instead of an entire
> > > database at a time.  This commit introduces a way to pass individual
> > tables
> > > around and starts using that feature in ovn-controller.
> > >
> > > CC: Han Zhou <zhouhan at gmail.com>
> > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > ---
> >
> > In this patch although tables are passed individually, the ctx
parameter is
> > still kept for most functions. With ctx, it is still easy for the
function
> > to access any tables than the individual tables passed.
> > I see two reasons why ctx is still kept in this patch:
> > 1) index implementation requires passing the IDL, and I saw this is
> > addressed in next patches by redesigning the index mechanism
> > 2) OVSDB transaction requires passing the ..._idl_txn in the ctx. I
think
> > we can pass in ..._idl_txn directly instead of the ctx. This would avoid
> > the misuse of tables.
> >
> > What do you think about point 2)?
>
> Eliminating ctx is a goal but this series doesn't accomplish it yet.  I
> hope to do that in a followup.  Passing in idl_txn is an easy way to do
> it; I was hoping to somehow distinguish read-only tables from read/write
> ones (maybe via const?) but I haven't gotten that far.

Ok. I think distinguish read (ready-only and read/write) and write-only
makes sense, because write-only tables are not regarded as inputs. However,
I am not sure how to ensure this, i.e. verify that we are not reading the
tables that are declared in the parameter as write-only.

Another point is, how to reflect the indirect dependency, e.g. bfd_run()
doesn't has table "ovsrec_port" as input, but in fact it depends on this
table by referencing from ovsrec_bridge->ports.

I agree that these don't need to be addressed in current patch. so

Acked-by: Han Zhou <hzhou8 at ebay.com>