On 4/28/22 18:07, Ilya Maximets wrote:
> 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 <[email protected]>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>> 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/[email protected]/
>> 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.
>
Thank you!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
