If a single transaction exceeds the size of the whole database (e.g.,
a lot of rows got removed and new ones added), transaction history will
be drained.  This leads to sending UUID_ZERO to the clients as the last
transaction id in the next monitor update, because monitor doesn't
know what was the actual last transaction id.  In case of a re-connect
that will cause re-downloading of the whole database, since the
client's last_id will be out of sync.

One solution would be to store the last transaction ID separately
from the actual transactions, but that will require a careful
management in cases where database gets reset and the history needs
to be cleared.  Keeping the one last transaction instead to avoid
the problem.  That should not be a big concern in terms of memory
consumption, because this last transaction will be removed from the
history once the next transaction appeared.  This is also not a concern
for a fast re-sync, because this last transaction will not be used
for the monitor reply; it's either client already has it, so no need
to send, or it's a history miss.

The test updated to not check the number of atoms if there is only
one transaction in the history.

Fixes: 317b1bfd7dd3 ("ovsdb: Don't let transaction history grow larger than the 
database.")
Signed-off-by: Ilya Maximets <[email protected]>
---
 ovsdb/transaction.c   |  6 ++++--
 tests/ovsdb-server.at | 16 +++++++++-------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 88e052800..db86d847c 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -1595,8 +1595,10 @@ ovsdb_txn_history_run(struct ovsdb *db)
     /* Remove old histories to limit the size of the history.  Removing until
      * the number of ovsdb atoms in history becomes less than the number of
      * atoms in the database, because it will be faster to just get a database
-     * snapshot than re-constructing changes from the history that big. */
-    while (db->n_txn_history &&
+     * snapshot than re-constructing changes from the history that big.
+     * Keeping at least one transaction to avoid sending UUID_ZERO as a last id
+     * if all entries got removed due to the size limit. */
+    while (db->n_txn_history > 1 &&
            (db->n_txn_history > 100 ||
             db->n_txn_history_atoms > db->n_atoms)) {
         struct ovsdb_txn_history_node *txn_h_node = CONTAINER_OF(
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 2b742f78b..876cb836c 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -1250,10 +1250,11 @@ dnl the total database size, so the transaction history 
will not be able
 dnl to hold all of them.
 dnl
 dnl The test verifies that the number of atoms in the transaction history
-dnl is always less than the number of atoms in the database.
-get_n_atoms () {
+dnl is always less than the number of atoms in the database, except for
+dnl a case where there is only one transaction in a history.
+get_memory_value () {
     n=$(ovs-appctl -t ovsdb-server memory/show dnl
-            | tr ' ' '\n' | grep atoms | grep "^$1:" | cut -d ':' -f 2)
+            | tr ' ' '\n' | grep "^$1:" | cut -d ':' -f 2)
     if test X"$n" == "X"; then
         n=0
     fi
@@ -1261,8 +1262,9 @@ get_n_atoms () {
 }
 
 check_atoms () {
-    n_db_atoms=$(get_n_atoms atoms)
-    n_txn_history_atoms=$(get_n_atoms txn-history-atoms)
+    if test $(get_memory_value txn-history) -eq 1; then return; fi
+    n_db_atoms=$(get_memory_value atoms)
+    n_txn_history_atoms=$(get_memory_value txn-history-atoms)
     echo "n_db_atoms:          $n_db_atoms"
     echo "n_txn_history_atoms: $n_txn_history_atoms"
     AT_CHECK([test $n_txn_history_atoms -le $n_db_atoms])
@@ -1274,7 +1276,7 @@ add_ports () {
     done
 }
 
-initial_db_atoms=$(get_n_atoms atoms)
+initial_db_atoms=$(get_memory_value atoms)
 
 for i in $(seq 1 100); do
     cmd=$(add_ports $i $(($i / 4 + 1)))
@@ -1289,7 +1291,7 @@ done
 
 dnl After removing all the bridges, the number of atoms in the database
 dnl should return to its initial value.
-AT_CHECK([test $(get_n_atoms atoms) -eq $initial_db_atoms])
+AT_CHECK([test $(get_memory_value atoms) -eq $initial_db_atoms])
 
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 AT_CLEANUP
-- 
2.31.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to