On Sun, Dec 19, 2021 at 6:09 AM Ilya Maximets <[email protected]> wrote: > > 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 >
Acked-by: Han Zhou <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
