On 5/25/23 14:27, Simon Horman wrote:
> 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 <i.maxim...@ovn.org>
>> ---
>> 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?
Sure.
>
>> +/* 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()
Good point. Will reduce the number of parenthesis. I'll post v2.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
