[ovs-dev] [PATCH 2/3] jsonrpc: Treat draining data from send queue as activity.

[Ben Pfaff]

On Wed, Sep 05, 2012 at 11:48:26PM -0700, Ansis Atteka wrote:
> On Wed, Aug 8, 2012 at 4:14 PM, Ben Pfaff <blp at nicira.com> wrote:
> > Until now, the jsonrpc module has used messages received from the
> > remote peer as the sole means to determine that the JSON-RPC
> > connection is up.  This could in theory interact badly with a
> > remote peer that stops reading and processing messages from the
> > receive queue when there is a backlog in the send queue for a
> > given connection (ovsdb-server is an example of a program that
> > behaves this way).  This commit fixes the problem by expanding
> > the definition of "activity" to include successfully sending
> > JSON-RPC data that was previously queued.
> >
> > The above change is exactly analogous to the similar change
> > made to the rconn library in commit 133f2dc95454 (rconn: Treat
> > draining a message from the send queue as activity.).
> >
> > Bug #12789.
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/jsonrpc.c         |   13 +++++++++++++
> >  python/ovs/jsonrpc.py |   11 ++++++++++-
> >  2 files changed, 23 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> > index c4d7dd2..9c0aaa9 100644
> > --- a/lib/jsonrpc.c
> > +++ b/lib/jsonrpc.c
> > @@ -880,9 +880,22 @@ jsonrpc_session_run(struct jsonrpc_session *s)
> >      }
> >
> >      if (s->rpc) {
> > +        size_t backlog;
> >          int error;
> >
> > +        backlog = jsonrpc_get_backlog(s->rpc);
> >          jsonrpc_run(s->rpc);
> > +        if (jsonrpc_get_backlog(s->rpc) < backlog) {
> > +            /* Data previously caught in a queue was successfully sent.
> Is this comment accurate that "data was sent successfully"? I suppose
> that, if jsonrpc_get_backlog(s->rpc) returned 0, then that might mean
> an error in jsonrpc_run(s->rpc) too?

Yes, that's also a possibility.  In that case, we'll catch the error
and disconnect in the jsonrpc_get_status() test just below:

> > +             *
> > +             * We don't count data that is successfully sent immediately as
> > +             * activity, because there's a lot of queuing downstream from us,
> > +             * which means that we can push a lot of data into a connection
> > +             * that has stalled and won't ever recover.
> > +             */
> > +            reconnect_activity(s->reconnect, time_msec());
> > +        }
> > +
> >          error = jsonrpc_get_status(s->rpc);
> >          if (error) {
> >              reconnect_disconnected(s->reconnect, time_msec(), error);

If you like, I'll make the comment mention that case.