ovsdb: transaction: Fix weak reference leak.

When a row is deleted, if the row has weak references to other rows, the
weak reference nodes attached to the destination rows (through
weak->dst_node hmap) are not destroyed.

Deleting weak references is properly handled when a row is modified. The
removed references are taken care by:
1. assess_weak_refs() figures out the deleted references from the row
   and add them to txn_row->deleted_refs.
2. before commit, in ovsdb_txn_update_weak_refs() it finds the
   destination row for each item in txn_row->deleted_refs (from step 1),
   and destroy the corresponding weak references of the destination row.

However, when the row is deleted, the step 1 in assess_weak_refs() is
missing. It directly returns without adding the deleted references to
txn_row->deleted_refs. So, the destination nodes will keep those weak
references although the source side of the references are already
deleted.  When such rows that originating weak references are created
and deleted, more and more such useless weak reference structures
accumulate in the memory, and can stay there until the destination rows
are deleted. It is possible that the destination row is never deleted,
and in such case the ovsdb-server memory keeps growing (although it is
not strictly memory leak, because the structures are still referenced).

This problem has an impact to applications like OVN SB DB - the memory
grows very fast in long-running deployments and finally causes OOM.

This patch fixes it by generating deleted_refs for deleted rows in
assess_weak_refs().

Fixes: 4dbff9f0a685 ("ovsdb: transaction: Incremental reassessment of weak refs.")
Signed-off-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 10 insertions, 2 deletions

diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 679688056..5d7c70a51 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -666,7 +666,7 @@ static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
 {
     struct ovsdb_weak_ref *weak;
-    struct ovsdb_table *table;
+    struct ovsdb_table *table = txn_row->table;
     struct shash_node *node;
 
     if (txn_row->old && !txn_row->new) {
@@ -688,6 +688,15 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
             ovs_assert(ovs_list_is_empty(&weak->src_node));
             ovs_list_insert(&src_txn_row->deleted_refs, &weak->src_node);
         }
+
+        /* Creating refs that needs to be removed on commit. */
+        SHASH_FOR_EACH (node, &table->schema->columns) {
+            const struct ovsdb_column *column = node->data;
+            struct ovsdb_datum *datum = &txn_row->old->fields[column->index];
+
+            find_and_add_weak_refs(txn_row->old, datum, column,
+                                   &txn_row->deleted_refs, NULL, NULL);
+        }
     }
 
     if (!txn_row->new) {
@@ -698,7 +707,6 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
         return NULL;
     }
 
-    table = txn_row->table;
     SHASH_FOR_EACH (node, &table->schema->columns) {
         const struct ovsdb_column *column = node->data;
         struct ovsdb_datum *datum = &txn_row->new->fields[column->index];