Skip to content

Commit

Permalink
ovsdb: transaction: Fix weak reference leak.
Browse files Browse the repository at this point in the history
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: 4dbff9f ("ovsdb: transaction: Incremental reassessment of weak refs.")
Signed-off-by: Han Zhou <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
hzhou8 authored and igsilya committed Nov 4, 2022
1 parent 9a63804 commit c8a08db
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions ovsdb/transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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];
Expand Down

0 comments on commit c8a08db

Please sign in to comment.