On 4/26/22 12:37, Dumitru Ceara wrote:
> At a first glance, change tracking should never be allowed for
> write-only columns. However, some clients (e.g., ovn-northd) that are
> mostly exclusive writers of a database, use change tracking to avoid
> duplicating the IDL row records into a local cache when implementing
> incremental processing.
>
> The default behavior of the IDL is to automatically turn a write-only
> column into a read-write column whenever the client enables change
> tracking for that column.
>
> For the afore mentioned clients, this becomes a performance issue.
> Commit 1cc618c32524 ("ovsdb-idl: Fix atomicity of writes that don't
> change a column's value.") explains why writes that don't change a
> column's value cannot be optimized out early if the column is
> read/write.
>
> Furthermore, if there is at least one record in any table that
> changed during a transaction, then *all* records that have been
> written are added to the transaction, even if their values didn't
> change. If there are many such rows (e.g., like in ovn-northd's
> case) this incurs a significant overhead because:
> a. the client has to build this large transaction
> b. the transaction has to be sent over the network
> c. the server needs to parse this (mostly) no-op update
>
> We now introduce new IDL APIs allowing users to set a new monitoring
> mode flag, OVSDB_IDL_WRITE_CHANGED_ONLY, to indicate to the IDL that the
> atomicity constraints may be relaxed and written columns that don't
> change value can be skipped from the current transaction.
>
> We benchmarked ovn-northd performance when using this new mode
> against NB and SB databases taken from ovn-kubernetes scale tests.
> We noticed that when a minor change is performed to the Northbound
> database (e.g., NB_Global.nb_cfg is incremented) the time it takes to
> build the Southbound transaction becomes negligible (vs ~1.5 seconds
> before this change).
>
> End-to-end ovn-kubernetes scale tests on 120-node clusters also show
> significant reduction of latency to bring up pods; both average and P99
> latency decreased by ~30%.
>
> Acked-by: Han Zhou <hz...@ovn.org>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
> V4:
> - Added Han's ack and rephrased comments as suggested.
> - Added a comment to ovsdb_idl_set_write_change_only_all().
> - Fixed naming inconsistency ("write_change_only" vs
> "write_changed_only") - pointed out by Ilya privately.
> V3:
> - Addressed Han's comments:
> - Add a new IDL monitor mode flag, OVSDB_IDL_WRITE_CHANGED_ONLY.
> - Rephrased commit log.
> - Changed commit title to reflect the new approach.
> - Old patch (v2) was:
>
> https://patchwork.ozlabs.org/project/openvswitch/patch/20220415142146.13169-1-dce...@redhat.com/
> V2:
> - Addressed Numan's comments:
> - Added APIs to allow per column configuration of modes.
> - Fixed unit tests to actually enable change tracking for write-only
> columns.
> - Fixed ovsdb_idl_row_change() to correctly update row/table/idl change
> seqnos if change tracking is enabled (even for write-only rows).
>
> Note: The OVN counter part change is:
> https://github.com/dceara/ovn/commit/36c9820afe7b998ba6f32b908f21dfdef472839b
> ---
> NEWS | 4 ++++
> lib/ovsdb-idl.c | 60 +++++++++++++++++++++++++++++++++++++++++-----
> lib/ovsdb-idl.h | 16 +++++++++++--
> tests/ovsdb-idl.at | 31 +++++++++++++++++++++++-
> tests/test-ovsdb.c | 18 ++++++++++----
> 5 files changed, 115 insertions(+), 14 deletions(-)
Applied. Thanks!
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
