Currently, the transaction history keeps a fixed number of transactions, hardcoded to 100. On a busy system this may cover only a short period of time, so a client that disconnects has a high chance of not finding its last transaction in the history, which leads to a download of the whole database.
This patch replaces the fixed number of transactions with a time based limit. Transactions are removed from the history once they are older than the limit, regardless of how many of them are in the history. The limit defaults to 60 seconds and can be changed with a new database configuration option 'transaction-history-time-limit' for clustered and relay databases. The value is in seconds. The limit on the number of atoms, which prevents the transaction history from growing larger than the database itself, still applies. Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2026-April/431593.html Signed-off-by: Tomasz Wałaszek <[email protected]> --- ovsdb/ovsdb-server.c | 44 +++++++++++++++++++++++++++- ovsdb/ovsdb.h | 3 ++ ovsdb/transaction.c | 25 +++++++++++++--- ovsdb/transaction.h | 2 ++ tests/ovsdb-server.at | 68 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 137 insertions(+), 5 deletions(-) diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index af217476f..2b765686a 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -156,8 +156,15 @@ struct db_config { bool backup; /* If true, the database is read-only and receives * updates from the 'source'. */ } ab; + + /* Valid for SM_CLUSTERED or SM_RELAY. */ + long long int txn_history_time_max; /* Keep history entries younger than + * this many seconds. */ }; +/* Default value for 'txn_history_time_max', in seconds. */ +#define TXN_HISTORY_TIME_MAX_DEFAULT 60 + struct db { struct ovsdb *db; char *filename; @@ -459,6 +466,7 @@ db_config_clone(const struct db_config *c) conf->options = ovsdb_jsonrpc_options_clone(c->options); } conf->ab.sync_exclude = nullable_xstrdup(c->ab.sync_exclude); + conf->txn_history_time_max = c->txn_history_time_max; return conf; } @@ -486,6 +494,8 @@ add_database_config(struct shash *db_conf, const char *opt, struct db_config *conf = xzalloc(sizeof *conf); char *filename = NULL; + conf->txn_history_time_max = TXN_HISTORY_TIME_MAX_DEFAULT; + if (parse_relay_args(opt, &filename, &conf->source)) { conf->model = SM_RELAY; conf->options = get_jsonrpc_options(conf->source, conf->model); @@ -569,6 +579,11 @@ database_update_config(struct server_config *server_config, replication_set_db(db->db, conf->source, conf->ab.sync_exclude, server_uuid, &conf->options->rpc); } + + if (conf->model == SM_CLUSTERED || conf->model == SM_RELAY) { + ovsdb_txn_history_update(db->db, + conf->txn_history_time_max * 1000); + } } static bool @@ -1175,6 +1190,10 @@ open_db(struct server_config *server_config, * modes only. */ ovsdb_txn_history_init(db->db, model == SM_RELAY || model == SM_CLUSTERED); + if (model == SM_RELAY || model == SM_CLUSTERED) { + ovsdb_txn_history_update(db->db, conf->txn_history_time_max * 1000); + } + read_db(server_config, db); error = (db->db->name[0] == '_' @@ -2901,6 +2920,12 @@ db_config_to_json(const struct db_config *conf) } json_object_put(json, "backup", json_boolean_create(conf->ab.backup)); } + + if (conf->txn_history_time_max != TXN_HISTORY_TIME_MAX_DEFAULT) { + json_object_put(json, "transaction-history-time-limit", + json_integer_create(conf->txn_history_time_max)); + } + return json; } @@ -3024,12 +3049,13 @@ remotes_from_json(struct shash *remotes, const struct json *json) static struct db_config * db_config_from_json(const char *name, const struct json *json) { - const struct json *model, *source, *sync_exclude, *backup; + const struct json *model, *source, *sync_exclude, *backup, *txn_time_limit; struct db_config *conf = xzalloc(sizeof *conf); struct ovsdb_parser parser; struct ovsdb_error *error; conf->model = SM_UNDEFINED; + conf->txn_history_time_max = TXN_HISTORY_TIME_MAX_DEFAULT; ovs_assert(json); if (json->type == JSON_NULL) { @@ -3105,6 +3131,22 @@ db_config_from_json(const char *name, const struct json *json) } } + txn_time_limit = ovsdb_parser_member(&parser, + "transaction-history-time-limit", + OP_INTEGER | OP_OPTIONAL); + if (txn_time_limit) { + if (json_integer(txn_time_limit) < 0) { + ovsdb_parser_raise_error(&parser, + "transaction-history-time-limit must not be negative"); + } else if (json_integer(txn_time_limit) > INT_MAX) { + ovsdb_parser_raise_error(&parser, + "transaction-history-time-limit must not be greater than %d", + INT_MAX); + } else { + conf->txn_history_time_max = json_integer(txn_time_limit); + } + } + error = ovsdb_parser_finish(&parser); if (error) { char *s = ovsdb_error_to_string_free(error); diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h index 325900bc6..3b027fe64 100644 --- a/ovsdb/ovsdb.h +++ b/ovsdb/ovsdb.h @@ -71,6 +71,7 @@ bool ovsdb_is_valid_version(const char *); struct ovsdb_txn_history_node { struct ovs_list node; /* Element in struct ovsdb's txn_history list */ struct ovsdb_txn *txn; + long long int timestamp; /* Time of addition to the history, in msec. */ }; struct ovsdb_compaction_state { @@ -108,6 +109,8 @@ struct ovsdb { /* History trasanctions for incremental monitor transfer. */ bool need_txn_history; /* Need to maintain history of transactions. */ + long long int txn_history_time_max; /* Keep entries younger than this + * many msec. */ unsigned int n_txn_history; /* Current number of history transactions. */ unsigned int n_txn_history_atoms; /* Total number of atoms in history. */ struct ovs_list txn_history; /* Contains "struct ovsdb_txn_history_node. */ diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 0d0d27b61..ef0c668a2 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -33,6 +33,7 @@ #include "row.h" #include "storage.h" #include "table.h" +#include "timeval.h" #include "uuid.h" #include "util.h" @@ -1189,6 +1190,7 @@ ovsdb_txn_add_to_history(struct ovsdb_txn *txn) if (txn->db->need_txn_history) { struct ovsdb_txn_history_node *node = xzalloc(sizeof *node); node->txn = ovsdb_txn_clone_for_history(txn); + node->timestamp = time_msec(); ovs_list_push_back(&txn->db->txn_history, &node->node); txn->db->n_txn_history++; txn->db->n_txn_history_atoms += txn->n_atoms; @@ -1683,15 +1685,22 @@ ovsdb_txn_history_run(struct ovsdb *db) * 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. + * Entries older than 'txn_history_time_max' are removed as well. * 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)) { + long long int now = time_msec(); + + while (db->n_txn_history > 1) { struct ovsdb_txn_history_node *txn_h_node = CONTAINER_OF( - ovs_list_pop_front(&db->txn_history), + ovs_list_front(&db->txn_history), struct ovsdb_txn_history_node, node); + bool expired = now - txn_h_node->timestamp > db->txn_history_time_max; + + if (!expired && db->n_txn_history_atoms <= db->n_atoms) { + break; + } + ovs_list_remove(&txn_h_node->node); db->n_txn_history_atoms -= txn_h_node->txn->n_atoms; ovsdb_txn_destroy_cloned(txn_h_node->txn); free(txn_h_node); @@ -1725,3 +1734,11 @@ ovsdb_txn_history_destroy(struct ovsdb *db) db->n_txn_history = 0; db->n_txn_history_atoms = 0; } + +void +ovsdb_txn_history_update(struct ovsdb *db, long long int txn_history_time_max) +{ + ovs_assert(txn_history_time_max >= 0); + db->txn_history_time_max = txn_history_time_max; + ovsdb_txn_history_run(db); +} diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h index d94205414..914cfdac5 100644 --- a/ovsdb/transaction.h +++ b/ovsdb/transaction.h @@ -77,5 +77,7 @@ const char *ovsdb_txn_get_comment(const struct ovsdb_txn *); void ovsdb_txn_history_run(struct ovsdb *); void ovsdb_txn_history_init(struct ovsdb *, bool need_txn_history); void ovsdb_txn_history_destroy(struct ovsdb *); +void ovsdb_txn_history_update(struct ovsdb *, + long long int txn_history_time_max); #endif /* ovsdb/transaction.h */ diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index 7042b1fd3..509cf953c 100644 --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -1640,6 +1640,46 @@ dnl still has a reasonable size. check_atoms AT_CHECK([test $(get_memory_value atoms) -eq $db_atoms_before_conversion]) +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP + +AT_SETUP([ovsdb-server transaction history time limit]) +AT_KEYWORDS([ovsdb server transaction]) +on_exit 'kill `cat *.pid`' +AT_CHECK([ovsdb-tool create-cluster db dnl + $abs_top_srcdir/vswitchd/vswitch.ovsschema unix:s1.raft], + [0], [ignore], [ignore]) +AT_DATA([config.json], [ +{"remotes": {"punix:db.sock": {}}, + "databases": {"db": {"service-model": "clustered"}}} +]) +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file dnl + --config-file=config.json], [0], [ignore], [ignore]) + +dnl Make the database large enough to not limit the history by atoms. +AT_CHECK([ovs-vsctl --no-wait init -- add-br br0 dnl + $(for i in $(seq 250); do echo -- add-port br0 p$i; done)]) +AT_CHECK([for i in $(seq 110); do + ovs-vsctl --no-wait set port p1 external_ids:k=$i || exit 1 + done]) + +n_txns () { + ovs-appctl -t ovsdb-server memory/show | tr ' ' '\n' | sed -n 's/^txn-history://p' +} + +dnl All transactions are younger than the default limit, so all are kept. +AT_CHECK([test $(n_txns) -ge 110]) + +dnl Set a short limit, so all the entries expire. The last one is always kept. +sleep 2 +AT_DATA([config.json], [ +{"remotes": {"punix:db.sock": {}}, + "databases": {"db": {"service-model": "clustered", + "transaction-history-time-limit": 1}}} +]) +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/reload]) +AT_CHECK([test $(n_txns) -eq 1]) + OVSDB_SERVER_SHUTDOWN AT_CLEANUP @@ -3052,6 +3092,34 @@ WARN|syntax "{"dscp":42,"inactivity-probe":10000,"max-backoff":8000,"role":"My-R syntax error: Parsing JSON-RPC options failed: Member 'role' is present but not allowed here. ]) +TEST_CONFIG_FILE([negative transaction-history-time-limit], [ +{ + "remotes": { "punix:db.sock": {} }, + "databases": { + "db": { "service-model": "clustered", + "transaction-history-time-limit": -1 } + } +} +], [1], [dnl +WARN|syntax "{"service-model":"clustered","transaction-history-time-limit":-1}": dnl +syntax error: Parsing database db failed: dnl +transaction-history-time-limit must not be negative +WARN|config: failed to parse 'databases']) + +TEST_CONFIG_FILE([too large transaction-history-time-limit], [ +{ + "remotes": { "punix:db.sock": {} }, + "databases": { + "db": { "service-model": "clustered", + "transaction-history-time-limit": 2147483648 } + } +} +], [1], [dnl +WARN|syntax "{"service-model":"clustered","transaction-history-time-limit":2147483648}": dnl +syntax error: Parsing database db failed: dnl +transaction-history-time-limit must not be greater than 2147483647 +WARN|config: failed to parse 'databases']) + TEST_CONFIG_FILE([unknown config], [ { "remotes": { "punix:db.sock": {} }, -- 2.53.0
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
