On Thu, May 18, 2023 at 02:14:25PM +0200, Ilya Maximets wrote:
> In most cases, after the condition change request, the new condition
> is the same as old one plus minus a few clauses. Today, ovsdb-server
> will evaluate every database row against all the old clauses and then
> against all the new clauses in order to tell if an update should be
> generated.
>
> For example, every time a new port is added, ovn-controller adds two
> new clauses to conditions for a Port_Binding table. And this condition
> may grow significantly in size making addition of every new port
> heavier on the server side.
>
> The difference between conditions is not larger and, likely,
> significantly smaller than old and new conditions combined. And if
> the row doesn't match clauses that are different between old and new
> conditions, that row should not be part of the update. It either
> matches both old and new, or it doesn't match either of them.
>
> If the row matches some clauses in the difference, then we need to
> perform a full match against old and new in order to tell if it
> should be added/removed/modified. This is necessary because different
> clauses may select same rows.
>
> Let's generate the condition difference and use it to avoid evaluation
> of all the clauses for rows not affected by the condition change.
>
> Testing shows 70% reduction in total CPU time in ovn-heater's 120-node
> density-light test with conditional monitoring. Average CPU usage
> during the test phase went down from frequent 100% spikes to just 6-8%.
>
> Note: This will not help with new connections, or re-connections,
> or new monitor requests after database conversion. ovsdb-server will
> still evaluate every database row against every clause in the condition
> in these cases. So, it's still important to not have too many clauses
> in conditions for large tables.
>
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
> ovsdb/condition.c | 57 +++++++++++++++++++++++++++++++++++++
> ovsdb/condition.h | 9 ++++++
> ovsdb/monitor.c | 71 ++++++++++++++++++++++++++++++++---------------
> 3 files changed, 115 insertions(+), 22 deletions(-)
>
> diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> index d0016fa7f..c2d5363f5 100644
> --- a/ovsdb/condition.c
> +++ b/ovsdb/condition.c
> @@ -497,6 +497,63 @@ ovsdb_condition_cmp_3way(const struct ovsdb_condition *a,
> return 0;
> }
>
> +
nit: one blank line seems enough?
> +/* Given conditions 'a' and 'b', composes a new condition 'diff' that
> contains
> + * clauses that are present in one of the conditions, but not in the other.
> + *
> + * If some data doesn't match the resulted 'diff' condition, that means one
> of:
> + * 1. The data matches both 'a' and 'b'.
> + * 2. The data does not match either 'a' or 'b'.
> + *
> + * However, that is not true if one of the original conditions is a trivial
> + * True or False. In this case the function will currently just return an
> + * empty (True) condition. */
> +void
> +ovsdb_condition_diff(struct ovsdb_condition *diff,
> + const struct ovsdb_condition *a,
> + const struct ovsdb_condition *b)
> +{
> + size_t i, j;
> + int cmp;
> +
> + ovsdb_condition_init(diff);
> +
> + if (ovsdb_condition_is_trivial(a) || ovsdb_condition_is_trivial(b)) {
> + return;
> + }
> +
> + diff->clauses = xzalloc((a->n_clauses + b->n_clauses)
> + * sizeof *diff->clauses);
nit: maybe xcalloc()
...
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
