On Thu, Sep 8, 2022 at 8:46 AM Ilya Maximets <[email protected]> wrote: > > northd doesn't process changes incrementally, so it makes sense to > accumulate more database updates and process them in bulk, so we can > cover everything in a single recompute. > > ovsdb-server has a mechanism to start accumulating changes if the > client doesn't receive them fast enough, but it relies on the receive > buffer size, which is a few hundreds of KB on a typical system. > Unfortunately, that is enough to queue up several hundreds of small > updates, and it takes northd a lot of time to process them if poll > intervals are large, receiving at most 50 messages on each iteration > (half of which are updates for a _Server database). > > Calling ovsdb_idl_run() as long as something changes. This allows to > quickly process large bursts of database updates. For example, it > takes only 30-40 seconds for 'ovn-nbctl --wait=hv sync' to finish on > a 500-node cluster after the startup phase of the density-heavy > ovn-heater test, instead of 6-8 minutes without this change. > > 500 ms seems like a reasonable hard limit to avoid spinning for too > long if the database is changed constantly at a fast pace. > > Very long polling is also logged at INFO level to notify users. > Not using WARN or higher because it may happen under normal conditions, > e.g. on the initial connection to a large database or another type > of a single large update. Other notable polling attempts are logged > at debug level. > > Signed-off-by: Ilya Maximets <[email protected]> > --- > northd/ovn-northd.c | 38 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 34 insertions(+), 4 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index bd35802ed..96f17f15f 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -685,6 +685,36 @@ get_probe_interval(const char *db, const struct nbrec_nb_global *nb) > return interval; > } > > +static struct ovsdb_idl_txn * > +run_idl_loop(struct ovsdb_idl_loop *idl_loop, const char *name) > +{ > + unsigned long long duration, start = time_msec(); > + unsigned int seqno = UINT_MAX; > + struct ovsdb_idl_txn *txn; > + int n = 0; > + > + /* Accumulate database changes as long as there are some, > + * but no longer than half a second. */ > + while (seqno != ovsdb_idl_get_seqno(idl_loop->idl) > + && time_msec() - start < 500) { > + seqno = ovsdb_idl_get_seqno(idl_loop->idl); > + ovsdb_idl_run(idl_loop->idl); > + n++; > + } > + > + txn = ovsdb_idl_loop_run(idl_loop); > + > + duration = time_msec() - start; > + /* ovsdb_idl_run() is called at least 2 times. Once directly and > + * once in the ovsdb_idl_loop_run(). n > 2 means that we received > + * data on at least 2 subsequent calls. */ > + if (n > 2 || duration > 100) { > + VLOG(duration > 500 ? VLL_INFO : VLL_DBG, > + "%s IDL run: %d iterations in %lld ms", name, n + 1, duration); > + } > + return txn; > +} > + > int > main(int argc, char *argv[]) > { > @@ -821,8 +851,8 @@ main(int argc, char *argv[]) > ovsdb_idl_set_lock(ovnsb_idl_loop.idl, "ovn_northd"); > } > > - struct ovsdb_idl_txn *ovnnb_txn = > - ovsdb_idl_loop_run(&ovnnb_idl_loop); > + struct ovsdb_idl_txn *ovnnb_txn = run_idl_loop(&ovnnb_idl_loop, > + "OVN_Northbound"); > unsigned int new_ovnnb_cond_seqno = > ovsdb_idl_get_condition_seqno(ovnnb_idl_loop.idl); > if (new_ovnnb_cond_seqno != ovnnb_cond_seqno) { > @@ -833,8 +863,8 @@ main(int argc, char *argv[]) > ovnnb_cond_seqno = new_ovnnb_cond_seqno; > } > > - struct ovsdb_idl_txn *ovnsb_txn = > - ovsdb_idl_loop_run(&ovnsb_idl_loop); > + struct ovsdb_idl_txn *ovnsb_txn = run_idl_loop(&ovnsb_idl_loop, > + "OVN_Southbound"); > unsigned int new_ovnsb_cond_seqno = > ovsdb_idl_get_condition_seqno(ovnsb_idl_loop.idl); > if (new_ovnsb_cond_seqno != ovnsb_cond_seqno) { > -- > 2.34.3 >
Thanks Ilya for the great improvement! Applied to main. It is not a bug fix but it seems a good candidate for backporting to 22.09 given that the change is small and we haven't released yet. @Mark Michelson <[email protected]> @Numan Siddique <[email protected]> what do you think? Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
