[ovs-dev] [PATCH] bridge: Rate-limit updates to "instant stats".

Ben Pfaff blp at nicira.com
Tue Mar 19 21:46:46 UTC 2013


Not sure I follow.  The goal of instant_stats_could_have_changed is to
track whether the source data that we want to feed into the database
could have changed since we last composed a transaction to write it.
Whether the database itself has changed is not an issue.

Can you expand on your proposal?

On Tue, Mar 19, 2013 at 02:43:21PM -0700, Ethan Jackson wrote:
> The instant_stats_could_have_changed variable seems a bit of a kludge to
> me.  It seems it would be cleaner if instant_stats_run() maintained the
> idl_seqno at the time of the 'instant_txn' creation.  If that sequence
> number is out of date, we know we need to wait until instant_next_txn.
>  Thoughts?
> 
> Ethan
> 
> 
> On Tue, Mar 19, 2013 at 2:04 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Some information in the database must be kept as up-to-date as
> > possible to allow controllers to respond rapidly to network outages.
> > We call these statistics "instant" stats.
> >
> > Until now, the instant stats have been updated on every trip through
> > the main loop.  This work scales with the number of interfaces that
> > ovs-vswitchd manages.  With CFM enabled on 5000 interfaces, even with
> > a low transmission rate, we see ovs-vswitchd using 100% CPU just to
> > maintain statistics, even with no actual changes.
> >
> > This commit rate-limits updates to instant stats to at most 10 times
> > per second.  Earlier tests I did with similar patches showed a major
> > reduction in CPU usage.  I have not rerun those tests with this patch,
> > but I expect that the CPU usage should similarly decline.
> >
> > CC: Ram Jothikumar <rjothikumar at nicira.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  vswitchd/bridge.c |   75
> > +++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 files changed, 67 insertions(+), 8 deletions(-)
> >
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index 9449879..6dc3db2 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -2019,17 +2019,61 @@ refresh_controller_status(void)
> >
> >      ofproto_free_ofproto_controller_info(&info);
> >  }
> > +
> > +/* "Instant" stats.
> > + *
> > + * Some information in the database must be kept as up-to-date as
> > possible to
> > + * allow controllers to respond rapidly to network outages.  We call these
> > + * statistics "instant" stats.
> > + *
> > + * We wish to update these statistics every INSTANT_INTERVAL_MSEC
> > milliseconds,
> > + * assuming that they've changed.  The only means we have to determine
> > whether
> > + * they have changed are:
> > + *
> > + *   - Try to commit changes to the database.  If nothing changed, then
> > + *     ovsdb_idl_txn_commit() returns TXN_UNCHANGED, otherwise some other
> > + *     value.
> > + *
> > + *   - instant_stats_run() is called late in the run loop, after anything
> > that
> > + *     might change any of the instant stats.
> > + *
> > + * We use these two facts together to avoid waking the process up every
> > + * INSTANT_INTERVAL_MSEC whether there is any change or not.
> > + */
> > +
> > +/* Minimum interval between writing updates to the instant stats to the
> > + * database. */
> > +#define INSTANT_INTERVAL_MSEC 100
> > +
> > +/* Current instant stats database transaction, NULL if there is no ongoing
> > + * transaction. */
> > +static struct ovsdb_idl_txn *instant_txn;
> > +
> > +/* Next time (in msec on monotonic clock) at which we will update the
> > instant
> > + * stats.  */
> > +static long long int instant_next_txn = LLONG_MIN;
> > +
> > +/* True if the run loop has run since we last saw that the instant stats
> > were
> > + * unchanged, that is, this is true if we need to wake up at
> > 'instant_next_txn'
> > + * to refresh the instant stats. */
> > +static bool instant_stats_could_have_changed;
> >
> >  static void
> > -refresh_instant_stats(void)
> > +instant_stats_run(void)
> >  {
> > -    static struct ovsdb_idl_txn *txn = NULL;
> > +    enum ovsdb_idl_txn_status status;
> >
> > -    if (!txn) {
> > +    instant_stats_could_have_changed = true;
> > +
> > +    if (!instant_txn) {
> >          struct bridge *br;
> >
> > -        txn = ovsdb_idl_txn_create(idl);
> > +        if (time_msec() < instant_next_txn) {
> > +            return;
> > +        }
> > +        instant_next_txn = time_msec() + INSTANT_INTERVAL_MSEC;
> >
> > +        instant_txn = ovsdb_idl_txn_create(idl);
> >          HMAP_FOR_EACH (br, node, &all_bridges) {
> >              struct iface *iface;
> >              struct port *port;
> > @@ -2078,12 +2122,26 @@ refresh_instant_stats(void)
> >          }
> >      }
> >
> > -    if (ovsdb_idl_txn_commit(txn) != TXN_INCOMPLETE) {
> > -        ovsdb_idl_txn_destroy(txn);
> > -        txn = NULL;
> > +    status = ovsdb_idl_txn_commit(instant_txn);
> > +    if (status != TXN_INCOMPLETE) {
> > +        ovsdb_idl_txn_destroy(instant_txn);
> > +        instant_txn = NULL;
> > +    }
> > +    if (status == TXN_UNCHANGED) {
> > +        instant_stats_could_have_changed = false;
> >      }
> >  }
> >
> > +static void
> > +instant_stats_wait(void)
> > +{
> > +    if (instant_txn) {
> > +        ovsdb_idl_txn_wait(instant_txn);
> > +    } else if (instant_stats_could_have_changed) {
> > +        poll_timer_wait_until(instant_next_txn);
> > +    }
> > +}
> > +
> >  /* Performs periodic activity required by bridges that needs to be done
> > with
> >   * the least possible latency.
> >   *
> > @@ -2254,7 +2312,7 @@ bridge_run(void)
> >      }
> >
> >      run_system_stats();
> > -    refresh_instant_stats();
> > +    instant_stats_run();
> >  }
> >
> >  void
> > @@ -2286,6 +2344,7 @@ bridge_wait(void)
> >      }
> >
> >      system_stats_wait();
> > +    instant_stats_wait();
> >  }
> >
> >  /* Adds some memory usage statistics for bridges into 'usage', for use
> > with
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >



More information about the dev mailing list