On Tue, May 3, 2022 at 7:32 PM Han Zhou <hz...@ovn.org> wrote:
>
> On Thu, Apr 28, 2022 at 9:36 AM Dumitru Ceara <dce...@redhat.com> 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 <dce...@redhat.com>
> > ---
> > 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?
+1 from me for the backport to 22.03 and 21.12. I think it's not
applicable to 21.09 and older.
Numan
>
> Thanks,
> Han
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
