[ovs-dev] [PATCH 5/5] ovsdb-idl: Wake up ovsdb_idl_loop when a transaction commits.

[Russell Bryant]

On Fri, Jul 29, 2016 at 1:46 AM, Ben Pfaff <blp at ovn.org> wrote:

> On Thu, Jul 28, 2016 at 10:24:43PM -0500, Ryan Moats wrote:
> > "dev" <dev-bounces at openvswitch.org> wrote on 07/27/2016 02:03:25 AM:
> >
> > > From: Ben Pfaff <blp at ovn.org>
> > > To: dev at openvswitch.org
> > > Cc: Ben Pfaff <blp at ovn.org>
> > > Date: 07/27/2016 02:04 AM
> > > Subject: [ovs-dev] [PATCH 5/5] ovsdb-idl: Wake up ovsdb_idl_loop
> > > when a transaction commits.
> > > Sent by: "dev" <dev-bounces at openvswitch.org>
> > >
> > > There is a fair amount of code that defers modifying the database when
> a
> > > transaction cannot be created (because there is already one
> outstanding).
> > > This code tends to assume that the main loop will wake up again when it
> > > becomes possible again to modify the database, but the actual
> > ovsdb_id_loop
> > > implementation only did this if the database had changed.  This is too
> > > conservative a policy and may account for some failures I've seen in
> > tests.
> > >
> > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > ---
> >
> > Ben, while I can't tell you to hold up on this if somebody else acks it,
> > my first thought (after looking at it) is that checking it against the
> > unit tests cases answers only half of the questions that it needs to
> > answer.
> > Since it is changing the cycle time, there is the question of whether it
> > provides a measurable change in E2E performance. Therefore, with your
> > indulgence, I'm going to try and throw it into an openstack cloud
> tomorrow
> > and run some rally tests against it to see if indeed does result in a
> > measurable change in performance.
>
> This commit is not supposed to be a performance fix, it's supposed to be
> a correctness fix.
>
> I don't have a really good example of the kind of thing that it fixes so
> I'm happy to sit on it for a while until I do.


It makes sense to me.

Acked-by: Russell Bryant <russell at ovn.org>

-- 
Russell Bryant