On Thu, Apr 28, 2022 at 9:36 AM Dumitru Ceara <[email protected]> wrote: > > This uses the newly added ovsdb_idl_set_write_changed_only_all() to > explicitly disable atomicity checks for read/write columns and to enable > optimizing the process of building the SB transaction. > > Bump OVS submodule to pick up d94cd0d3eec3 ("ovsdb-idl: Support > write-only-changed IDL monitor mode."). > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2069623 > Signed-off-by: Dumitru Ceara <[email protected]> > --- > NOTE: this bumps the OVS submodule to a revision that's not yet part of > a stable branch. > --- > northd/ovn-northd.c | 10 +++++++++- > ovs | 2 +- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 45b120697..0a0f85010 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -723,10 +723,18 @@ main(int argc, char *argv[]) > unixctl_command_register("nb-connection-status", "", 0, 0, > ovn_conn_show, ovnnb_idl_loop.idl); > > - /* We want to detect all changes to the ovn-sb db. */ > + /* We want to detect all changes to the ovn-sb db so enable change > + * tracking but, for performance reasons, and because northd > + * reconciles all database changes, also configure the IDL to only > + * write columns that actually change value. > + */ > struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( > ovsdb_idl_create(ovnsb_db, &sbrec_idl_class, true, true)); > ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); > + ovsdb_idl_set_write_changed_only_all(ovnsb_idl_loop.idl, true); > + > + /* Disable alerting for pure write-only columns. */ > + ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_nb_cfg); > > unixctl_command_register("sb-connection-status", "", 0, 0, > ovn_conn_show, ovnsb_idl_loop.idl); > diff --git a/ovs b/ovs > index ba159ee0f..d94cd0d3e 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit ba159ee0f97ed770c244cd6710d34fe20595541d > +Subproject commit d94cd0d3eec33e4290d7ca81918f5ac61444886e > -- > 2.27.0 >
Thanks Dumitru. I applied this to main. I tested with a scale test and the extra SB writing cost is gone. Also, the recompute triggered by handling SB SB_Global:nb_cfg notification is gone. However, compared with the version before "ovn-northd: Enable change tracking for all SB tables." (but after I-P northd), it still has an extra recompute triggered by change notifications for the SB changes made by ovn-northd itself. I think we can probably still avoid this cost by carefully evaluating and omitting (without tracking) columns that are really "write-only". This can be separate patches of improvements. Since this patch is a fix for a performance regression, I am wondering if we should backport to 22.03 (or even older branches). It may be tricky since this depends on the latest OVS change (not on any branch). Any thoughts on this, Mark, Numan? Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
