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