On Sun, 2021-12-19 at 15:09 +0100, Ilya Maximets 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(-)
>
Acked-by: Mike Pattrick <[email protected]>
> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
