Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

The FDP team is no longer accepting new bugs in Bugzilla. Please report your issues under FDP project in Jira. Thanks.

Bug 2005958

Summary:	[RFE] ovsdb-server: Use column diffs for weak reference counting during transaction precommit
Product:	Red Hat Enterprise Linux Fast Datapath	Reporter:	Ilya Maximets <i.maximets>
Component:	ovsdb2.16	Assignee:	Ilya Maximets <i.maximets>
Status:	CLOSED ERRATA	QA Contact:	Jianlin Shi <jishi>
Severity:	high	Docs Contact:
Priority:	high
Version:	RHEL 8.0	CC:	ctrautma, dceara, jhsiao, jishi, ovs-qe, ovs-team, ralongi, tredaelli, trozet
Target Milestone:	---	Keywords:	FutureFeature
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	openvswitch2.16-2.16.0-26.el8fdp	Doc Type:	Enhancement
Doc Text:		Story Points:	---
Clone Of:	2003203	Environment:
Last Closed:	2022-03-30 16:28:58 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Ilya Maximets 2021-09-20 14:47:29 UTC

+++ This bug was initially created as a clone of Bug #2003203 +++

Assuming we have a row with a column that contains a set of UUIDs.
And these UUIDs are strong or weak references to rows in some other
database table.

If client will request addition of one new UUID to the set, currently,
ovsdb-server will re-count references for all UUIDs in that set.
If the referenced row will happen to have references to some other
rows, they will be re-counted too.  This is done to keep referential
integrity of the database and for garbage collection, i.e. to delete
all non-root rows that no longer has any references.
Because of this behavior time to execute transaction linearly grows
with increase of the number of elements in the set.

However, in reality, it only need to re-count references for UUIDs
that was added/removed to/from a set, and not for others.
OVSDB knows the new state of a row and the old one, so the difference
could be calculated.  Even more, currently we're storing column
diffs in a database storage, so the 'new' row is already calculated
from the 'old' and 'diff'.  We can just try to keep the diff after
reading from the storage and use it during transaction commit to avoid
unnecessary work.

This should make transaction commit less dependent on the number of
elements in the set by re-calculating only what really changed by the
transaction.

Related functions:
- ovsdb_txn_precommit()
- update_ref_counts()
- assess_weak_refs()

--- Additional comment from Ilya Maximets on 2021-09-17 18:31:28 UTC ---

Patch for strong references sent for review:
  https://patchwork.ozlabs.org/project/openvswitch/patch/20210916201522.3693567-1-i.maximets@ovn.org/


---------------------------------------------------------------------

BZ 2003203 covered strong references (update_ref_counts), while this
one is to cover weak references, i.e. assess_weak_refs() case.

Performance of weak references is very important for OVN load balancers.

Comment 1 Ilya Maximets 2021-10-18 10:08:04 UTC

Patch posted for review:
  https://patchwork.ozlabs.org/project/openvswitch/patch/20211016012023.1637214-1-i.maximets@ovn.org/

Comment 2 OvS team 2021-11-09 19:17:00 UTC

* Tue Nov 09 2021 Ilya Maximets <i.maximets> - 2.16.0-26
- ovsdb: transaction: Incremental reassessment of weak refs. [RH git: e8a363db49] (#2005958)
commit 4dbff9f0a68579241ac1a040726be3906afb8fe9
Author: Ilya Maximets <i.maximets>
Date: Sat Oct 16 03:20:23 2021 +0200

ovsdb: transaction: Incremental reassessment of weak refs.

The main idea is to not store list of weak references in the source
row, so they all don't need to be re-checked/updated on every
modification of that source row. The point is that source row already
knows UUIDs of all destination rows stored in the data, so there is no
much profit in storing this information somewhere else. If needed,
destination row can be looked up and reference can be looked up in the
destination row. For the fast lookup, destination row now stores
references in a hash map.

Weak reference structure now contains the table and uuid of a source
row instead of a direct pointer. This allows to replace/update the
source row without breaking any weak references stored in destination
rows.

Structure also now contains the key-value pair of atoms that triggered
creation of this reference. These atoms can be used to quickly
subtract removed references from a source row. During reassessment,
ovsdb now only needs to care about new added or removed atoms, and
atoms that got removed due to removal of the destination rows, but
these are marked for reassessment by the destination row.

ovsdb_datum_subtract() is used to remove atoms that points to removed
or incorrect rows, so there is no need to re-sort datum in the end.

Results of an OVN load-balancer benchmark that adds 3K load-balancers
to each of 120 logical switches and 120 logical routers in the OVN
sandbox with clustered Northbound database and then removes them:

Before:

%CPU CPU Time CMD
86.8 00:16:05 ovsdb-server nb1.db
44.1 00:08:11 ovsdb-server nb2.db
43.2 00:08:00 ovsdb-server nb3.db

After:

%CPU CPU Time CMD
54.9 00:02:58 ovsdb-server nb1.db
33.3 00:01:48 ovsdb-server nb2.db
32.2 00:01:44 ovsdb-server nb3.db

So, on a cluster leader the processing time dropped by 5.4x, on
followers - by 4.5x. More load-balancers - larger the performance
difference. There is a slight increase of memory usage, because new
reference structure is larger, but the difference is not significant.

Signed-off-by: Ilya Maximets <i.maximets>
Acked-by: Dumitru Ceara <dceara>

Reported-at: https://bugzilla.redhat.com/2005958
Signed-off-by: Ilya Maximets <i.maximets>

Comment 5 Jianlin Shi 2022-03-14 02:10:48 UTC

per https://bugzilla.redhat.com/show_bug.cgi?id=2003203#c8, set SanityOnly

Comment 7 errata-xmlrpc 2022-03-30 16:28:58 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (openvswitch2.16 bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:1146