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 <[email protected]>
Acked-by: Dumitru Ceara <[email protected]>

ovsdb/row.c

@@ -38,8 +38,7 @@ allocate_row(const struct ovsdb_table *table)
     struct ovsdb_row *row = xmalloc(row_size);
     row->table = CONST_CAST(struct ovsdb_table *, table);
     row->txn_row = NULL;
-    ovs_list_init(&row->src_refs);
-    ovs_list_init(&row->dst_refs);
+    hmap_init(&row->dst_refs);
     row->n_refs = 0;
     return row;
 }
@@ -61,6 +60,78 @@ ovsdb_row_create(const struct ovsdb_table *table)
     return row;
 }
 
+static struct ovsdb_weak_ref *
+ovsdb_weak_ref_clone(struct ovsdb_weak_ref *src)
+{
+    struct ovsdb_weak_ref *weak = xzalloc(sizeof *weak);
+
+    hmap_node_nullify(&weak->dst_node);
+    ovs_list_init(&weak->src_node);
+    weak->src_table = src->src_table;
+    weak->src = src->src;
+    weak->dst_table = src->dst_table;
+    weak->dst = src->dst;
+    ovsdb_atom_clone(&weak->key, &src->key, src->type.key.type);
+    if (src->type.value.type != OVSDB_TYPE_VOID) {
+        ovsdb_atom_clone(&weak->value, &src->value, src->type.value.type);
+    }
+    ovsdb_type_clone(&weak->type, &src->type);
+    weak->column_idx = src->column_idx;
+    weak->by_key = src->by_key;
+    return weak;
+}
+
+uint32_t
+ovsdb_weak_ref_hash(const struct ovsdb_weak_ref *weak)
+{
+    return uuid_hash(&weak->src);
+}
+
+static bool
+ovsdb_weak_ref_equals(const struct ovsdb_weak_ref *a,
+                      const struct ovsdb_weak_ref *b)
+{
+    if (a == b) {
+        return true;
+    }
+    return a->src_table == b->src_table
+           && a->dst_table == b->dst_table
+           && uuid_equals(&a->src, &b->src)
+           && uuid_equals(&a->dst, &b->dst)
+           && a->column_idx == b->column_idx
+           && a->by_key == b->by_key
+           && ovsdb_atom_equals(&a->key, &b->key, a->type.key.type);
+}
+
+struct ovsdb_weak_ref *
+ovsdb_row_find_weak_ref(const struct ovsdb_row *row,
+                        const struct ovsdb_weak_ref *ref)
+{
+    struct ovsdb_weak_ref *weak;
+    HMAP_FOR_EACH_WITH_HASH (weak, dst_node,
+                             ovsdb_weak_ref_hash(ref), &row->dst_refs) {
+        if (ovsdb_weak_ref_equals(weak, ref)) {
+            return weak;
+        }
+    }
+    return NULL;
+}
+
+void
+ovsdb_weak_ref_destroy(struct ovsdb_weak_ref *weak)
+{
+    if (!weak) {
+        return;
+    }
+    ovs_assert(ovs_list_is_empty(&weak->src_node));
+    ovsdb_atom_destroy(&weak->key, weak->type.key.type);
+    if (weak->type.value.type != OVSDB_TYPE_VOID) {
+        ovsdb_atom_destroy(&weak->value, weak->type.value.type);
+    }
+    ovsdb_type_destroy(&weak->type);
+    free(weak);
+}
+
 struct ovsdb_row *
 ovsdb_row_clone(const struct ovsdb_row *old)
 {
@@ -75,6 +146,13 @@ ovsdb_row_clone(const struct ovsdb_row *old)
                           &old->fields[column->index],
                           &column->type);
     }
+
+    struct ovsdb_weak_ref *weak, *clone;
+    HMAP_FOR_EACH (weak, dst_node, &old->dst_refs) {
+        clone = ovsdb_weak_ref_clone(weak);
+        hmap_insert(&new->dst_refs, &clone->dst_node,
+                    ovsdb_weak_ref_hash(clone));
+    }
     return new;
 }
 
@@ -85,20 +163,13 @@ ovsdb_row_destroy(struct ovsdb_row *row)
 {
     if (row) {
         const struct ovsdb_table *table = row->table;
-        struct ovsdb_weak_ref *weak, *next;
+        struct ovsdb_weak_ref *weak;
         const struct shash_node *node;
 
-        LIST_FOR_EACH_SAFE (weak, next, dst_node, &row->dst_refs) {
-            ovs_list_remove(&weak->src_node);
-            ovs_list_remove(&weak->dst_node);
-            free(weak);
-        }
-
-        LIST_FOR_EACH_SAFE (weak, next, src_node, &row->src_refs) {
-            ovs_list_remove(&weak->src_node);
-            ovs_list_remove(&weak->dst_node);
-            free(weak);
+        HMAP_FOR_EACH_POP (weak, dst_node, &row->dst_refs) {
+            ovsdb_weak_ref_destroy(weak);
         }
+        hmap_destroy(&row->dst_refs);
 
         SHASH_FOR_EACH (node, &table->schema->columns) {
             const struct ovsdb_column *column = node->data;

ovsdb/row.h

@@ -36,11 +36,28 @@ struct ovsdb_column_set;
  * ovsdb_weak_ref" structures are created for them.
  */
 struct ovsdb_weak_ref {
-    struct ovs_list src_node;      /* In src->src_refs list. */
-    struct ovs_list dst_node;      /* In destination row's dst_refs list. */
-    struct ovsdb_row *src;         /* Source row. */
-    struct ovsdb_table *dst_table; /* Destination table. */
+    struct hmap_node dst_node;     /* In ovsdb_row's 'dst_refs' hmap. */
+    struct ovs_list src_node;      /* In txn_row's 'deleted/added_refs'. */
+
+    struct ovsdb_table *src_table; /* Source row table. */
+    struct uuid src;               /* Source row uuid. */
+
+    struct ovsdb_table *dst_table; /* Destination row table. */
     struct uuid dst;               /* Destination row uuid. */
+
+    /* Source row's key-value pair that created this reference.
+     * This information is needed in order to find and delete the reference
+     * from the source row.  We need both key and value in order to avoid
+     * accidential deletion of an updated data, i.e. if value in datum got
+     * updated and the reference was created by the old value.
+     * Storing column index in order to remove references from the correct
+     * column.   'by_key' flag allows to distinguish 2 references in a corner
+     * case where key and value are the same. */
+    union ovsdb_atom key;
+    union ovsdb_atom value;
+    struct ovsdb_type type;        /* Datum type of the key-value pair. */
+    unsigned int column_idx;       /* Row column index for this pair. */
+    bool by_key;                   /* 'true' if reference is a 'key'. */
 };
 
 /* A row in a database table. */
@@ -50,8 +67,7 @@ struct ovsdb_row {
     struct ovsdb_txn_row *txn_row; /* Transaction that row is in, if any. */
 
     /* Weak references.  Updated and checked only at transaction commit. */
-    struct ovs_list src_refs;   /* Weak references from this row. */
-    struct ovs_list dst_refs;   /* Weak references to this row. */
+    struct hmap dst_refs;          /* Weak references to this row. */
 
     /* Number of strong refs to this row from other rows, in this table or
      * other tables, through 'uuid' columns that have a 'refTable' constraint
@@ -69,6 +85,12 @@ struct ovsdb_row {
      * index 'i' is contained in hmap table->indexes[i].  */
 };
 
+uint32_t ovsdb_weak_ref_hash(const struct ovsdb_weak_ref *);
+struct ovsdb_weak_ref * ovsdb_row_find_weak_ref(const struct ovsdb_row *,
+                                                const struct ovsdb_weak_ref *);
+void ovsdb_weak_ref_destroy(struct ovsdb_weak_ref *);
+
+
 struct ovsdb_row *ovsdb_row_create(const struct ovsdb_table *);
 struct ovsdb_row *ovsdb_row_clone(const struct ovsdb_row *);
 void ovsdb_row_destroy(struct ovsdb_row *);