ovsdb: Don't store rows that didn't change in transaction history.

Transaction history is used to construct database updates for clients.
But if the row didn't change it will never be used for monitor updates,
because ovsdb_monitor_changes_classify() will always return
OVSDB_CHANGES_NO_EFFECT.  So, ovsdb_monitor_history_change_cb()
will never add it to the update.

This condition is very common for rows with references.  While
processing strong references in ovsdb_txn_adjust_atom_refs() the
whole destination row will be cloned into transaction just to update
the reference counter.  If this row will not be changed later in
the transaction, it will just stay in that state and will be added
to the transaction history.  Since the data didn't change, both 'old'
and 'new' datums will be the same and equal to one in the database.
So, we're keeping 2 copies of the same row in memory and we are
never using them.  In this case, we should just not add them to the
transaction history in the first place.

This change should save some space in the transaction history in case
of transactions with rows with big number of strong references.
This should also speed up the processing since we will not clone
these rows for transaction history and will not count their atoms.

Testing shows about 5-10% performance improvement in ovn-heater
test scenarios.

'n_atoms' counter for transaction adjusted to count only changed
rows, so we will have accurate value for a number of atoms in the
history.

Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 17 insertions, 1 deletions

diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index d8d677d31..bb997b45b 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -42,7 +42,7 @@ struct ovsdb_txn {
     struct ovs_list txn_tables; /* Contains "struct ovsdb_txn_table"s. */
     struct ds comment;
     struct uuid txnid; /* For clustered and relay modes.  It is the eid. */
-    size_t n_atoms;    /* Number of atoms in all transaction rows. */
+    size_t n_atoms;    /* Number of atoms in all changed transaction rows. */
     ssize_t n_atoms_diff;  /* Difference between number of added and
                             * removed atoms. */
 };
@@ -987,9 +987,17 @@ static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 count_atoms(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
 {
     struct ovsdb_table *table = txn_row->table;
+    size_t n_columns = shash_count(&table->schema->columns);
     ssize_t n_atoms_old = 0, n_atoms_new = 0;
     struct shash_node *node;
 
+    if (txn_row->old && txn_row->new
+        && bitmap_is_all_zeros(txn_row->changed, n_columns)) {
+        /* This row didn't change, it has nothing to contribute
+         * to atom counters. */
+        return NULL;
+    }
+
     SHASH_FOR_EACH (node, &table->schema->columns) {
         const struct ovsdb_column *column = node->data;
         const struct ovsdb_type *type = &column->type;
@@ -1115,6 +1123,14 @@ ovsdb_txn_clone_for_history(const struct ovsdb_txn *txn)
         struct ovsdb_txn_row *r;
         HMAP_FOR_EACH (r, hmap_node, &t->txn_rows) {
             size_t n_columns = shash_count(&t->table->schema->columns);
+
+            if (r->old && r->new
+                && bitmap_is_all_zeros(r->changed, n_columns)) {
+                /* The row is neither old or new and no columns changed.
+                 * No need to store in the history. */
+                continue;
+            }
+
             struct ovsdb_txn_row *r_cloned =
                 xzalloc(offsetof(struct ovsdb_txn_row, changed)
                         + bitmap_n_bytes(n_columns));