From patchwork Sun Dec 19 14:09:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1570730 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JH4Ns0LGWz9s3q for ; Mon, 20 Dec 2021 01:10:04 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 207E441F66; Sun, 19 Dec 2021 14:10:02 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id d6Ymtwjz3eV3; Sun, 19 Dec 2021 14:10:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id B20E441EEE; Sun, 19 Dec 2021 14:09:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 828B0C001E; Sun, 19 Dec 2021 14:09:59 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 93993C0038 for ; Sun, 19 Dec 2021 14:09:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7820684C15 for ; Sun, 19 Dec 2021 14:09:58 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7GGdcC0aQIp8 for ; Sun, 19 Dec 2021 14:09:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp1.osuosl.org (Postfix) with ESMTPS id D5A5884C16 for ; Sun, 19 Dec 2021 14:09:55 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 64AEBFF807; Sun, 19 Dec 2021 14:09:53 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Sun, 19 Dec 2021 15:09:39 +0100 Message-Id: <20211219140941.2279071-4-i.maximets@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211219140941.2279071-1-i.maximets@ovn.org> References: <20211219140941.2279071-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH 3/4] ovsdb: relay: Add transaction history support. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Even though relays can be scaled to the big number of servers to handle a lot more clients, lack of transaction history may cause significant load if clients are re-connecting. E.g. in case of the upgrade of a large-scale OVN deployment, relays can be taken down one by one forcing all the clients of one relay to jump to other ones. And all these clients will download the database from scratch from a new relay. Since relay itself supports monitor_cond_since connection to the main cluster, it receives the last transaction id along with each update. Since these transaction ids are 'eid's of actual transactions, they can be used by relay for a transaction history. Relay may not receive all the transaction ids, because the main cluster may combine several changes into a single monitor update. However, all relays will, likely, receive same updates with the same transaction ids, so the case where transaction id can not be found after re-connection between relays should not be very common. If some id is missing on the relay (i.e. this update was merged with some other update and newer id was used) the client will just re-download the database as if there was a normal transaction history miss. OVSDB client synchronization module updated to provide the last transaction id along with the update. Relay module updated to use these ids as a transaction id. If ids are zero, relay decides that the main server doesn't support transaction ids and disables the transaction history accordingly. Using ovsdb_txn_replay_commit() instead of ovsdb_txn_propose_commit_block(), so transactions are added to the history. This can be done, because relays has no file storage, so there is no need to write anything. Relay tests modified to test both standalone and clustered database as a main server. Checks added to ensure that all servers receive the same transaction ids in monitor updates. Signed-off-by: Ilya Maximets Acked-by: Mike Pattrick Acked-by: Han Zhou --- NEWS | 3 +++ lib/ovsdb-cs.c | 1 + lib/ovsdb-cs.h | 1 + ovsdb/ovsdb-server.c | 8 +++++--- ovsdb/relay.c | 28 ++++++++++++++++++++++----- ovsdb/transaction.c | 9 +++++---- tests/ovsdb-server.at | 44 +++++++++++++++++++++++++++++++------------ 7 files changed, 70 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index bc4a1cfac..e37ed97de 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,9 @@ Post-v2.16.0 * Default selection method for select groups with up to 256 buckets is now dp_hash. Previously this was limited to 64 buckets. This change is mainly for the benefit of OVN load balancing configurations. + - OVSDB: + * 'relay' service model now supports transaction history, i.e. honors the + 'last-txn-id' field in 'monitor_cond_since' requests from clients. v2.16.0 - 16 Aug 2021 diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c index 2d2b77026..f9acda419 100644 --- a/lib/ovsdb-cs.c +++ b/lib/ovsdb-cs.c @@ -1529,6 +1529,7 @@ ovsdb_cs_db_add_update(struct ovsdb_cs_db *db, .clear = clear, .monitor_reply = monitor_reply, .version = version, + .last_id = db->last_id, }; } diff --git a/lib/ovsdb-cs.h b/lib/ovsdb-cs.h index 03bbd7ee1..5d5b58f0a 100644 --- a/lib/ovsdb-cs.h +++ b/lib/ovsdb-cs.h @@ -99,6 +99,7 @@ struct ovsdb_cs_event { bool monitor_reply; struct json *table_updates; int version; + struct uuid last_id; } update; /* The "result" member from a transaction reply. The transaction is diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 9fe90592e..b975c17fc 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -729,9 +729,11 @@ open_db(struct server_config *config, const char *filename) db->db = ovsdb_create(schema, storage); ovsdb_jsonrpc_server_add_db(config->jsonrpc, db->db); - /* Enable txn history for clustered mode. It is not enabled for other mode - * for now, since txn id is available for clustered mode only. */ - ovsdb_txn_history_init(db->db, ovsdb_storage_is_clustered(storage)); + /* Enable txn history for clustered and relay modes. It is not enabled for + * other modes for now, since txn id is available for clustered and relay + * modes only. */ + ovsdb_txn_history_init(db->db, + is_relay || ovsdb_storage_is_clustered(storage)); read_db(config, db); diff --git a/ovsdb/relay.c b/ovsdb/relay.c index ef0e44d34..2df393403 100644 --- a/ovsdb/relay.c +++ b/ovsdb/relay.c @@ -222,7 +222,8 @@ ovsdb_relay_process_row_update(struct ovsdb_table *table, static struct ovsdb_error * ovsdb_relay_parse_update__(struct ovsdb *db, - const struct ovsdb_cs_db_update *du) + const struct ovsdb_cs_db_update *du, + const struct uuid *last_id) { struct ovsdb_error *error = NULL; struct ovsdb_txn *txn; @@ -254,8 +255,17 @@ exit: ovsdb_txn_abort(txn); return error; } else { - /* Commit transaction. */ - error = ovsdb_txn_propose_commit_block(txn, false); + if (uuid_is_zero(last_id)) { + /* The relay source doesn't support unique transaction ids, + * disabling transaction history for relay. */ + ovsdb_txn_history_destroy(db); + ovsdb_txn_history_init(db, false); + } else { + ovsdb_txn_set_txnid(last_id, txn); + } + /* Commit transaction. + * There is no storage, so ovsdb_txn_replay_commit() can be used. */ + error = ovsdb_txn_replay_commit(txn); } return error; @@ -266,6 +276,7 @@ ovsdb_relay_clear(struct ovsdb *db) { struct ovsdb_txn *txn = ovsdb_txn_create(db); struct shash_node *table_node; + struct ovsdb_error *error; SHASH_FOR_EACH (table_node, &db->tables) { struct ovsdb_table *table = table_node->data; @@ -276,7 +287,14 @@ ovsdb_relay_clear(struct ovsdb *db) } } - return ovsdb_txn_propose_commit_block(txn, false); + /* There is no storage, so ovsdb_txn_replay_commit() can be used. */ + error = ovsdb_txn_replay_commit(txn); + + /* Clearing the transaction history, and re-enabling it. */ + ovsdb_txn_history_destroy(db); + ovsdb_txn_history_init(db, true); + + return error; } static void @@ -304,7 +322,7 @@ ovsdb_relay_parse_update(struct relay_ctx *ctx, error = ovsdb_relay_clear(ctx->db); } if (!error) { - error = ovsdb_relay_parse_update__(ctx->db, du); + error = ovsdb_relay_parse_update__(ctx->db, du, &update->last_id); } } ovsdb_cs_db_update_destroy(du); diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index db86d847c..090068603 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -40,7 +40,7 @@ struct ovsdb_txn { struct ovsdb *db; struct ovs_list txn_tables; /* Contains "struct ovsdb_txn_table"s. */ struct ds comment; - struct uuid txnid; /* For clustered mode only. It is the eid. */ + struct uuid txnid; /* For clustered and relay modes. It is the eid. */ size_t n_atoms; /* Number of atoms in all transaction rows. */ ssize_t n_atoms_diff; /* Difference between number of added and * removed atoms. */ @@ -1143,9 +1143,10 @@ ovsdb_txn_complete(struct ovsdb_txn *txn) /* Applies 'txn' to the internal representation of the database. This is for * transactions that don't need to be written to storage; probably, they came - * from storage. These transactions shouldn't ordinarily fail because storage - * should contain only consistent transactions. (One exception is for database - * conversion in ovsdb_convert().) */ + * from storage or from relay. These transactions shouldn't ordinarily fail + * because storage should contain only consistent transactions. (One exception + * is for database conversion in ovsdb_convert().) Transactions from relay + * should also be consistent, since relay source should have verified them. */ struct ovsdb_error * OVS_WARN_UNUSED_RESULT ovsdb_txn_replay_commit(struct ovsdb_txn *txn) { diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index 876cb836c..a3b4051b2 100644 --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -1482,11 +1482,13 @@ EXECUTION_EXAMPLES AT_BANNER([OVSDB -- ovsdb-server relay]) -# OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS]) +# OVSDB_CHECK_EXECUTION_RELAY(MODEL, TITLE, SCHEMA, TRANSACTIONS, +# OUTPUT, [KEYWORDS]) # -# Creates a database with the given SCHEMA and starts an ovsdb-server on -# it. Also starts a daisy chain of ovsdb-servers in relay mode where the -# first relay server is connected to the main non-relay ovsdb-server. +# Creates a clustered or standalone (MODEL) database with the given SCHEMA +# and starts an ovsdb-server on it. Also starts a daisy chain of +# ovsdb-servers in relay mode where the first relay server is connected to +# the main non-relay ovsdb-server. # # Runs each of the TRANSACTIONS (which should be a quoted list of # quoted strings) against one of relay servers in the middle with @@ -1508,17 +1510,21 @@ AT_BANNER([OVSDB -- ovsdb-server relay]) # If a given UUID appears more than once it is always replaced by the # same marker. # -# Checks that the dump of all databases is the same. +# Checks that the dump of all databases and transaction ids are the same. # # TITLE is provided to AT_SETUP and KEYWORDS to AT_KEYWORDS. -m4_define([OVSDB_CHECK_EXECUTION], - [AT_SETUP([$1]) - AT_KEYWORDS([ovsdb server tcp relay $5]) +m4_define([OVSDB_CHECK_EXECUTION_RELAY], + [AT_SETUP([$2 - relay - $1]) + AT_KEYWORDS([ovsdb server tcp relay $6 $1]) n_servers=6 target=4 - $2 > schema + $3 > schema schema_name=`ovsdb-tool schema-name schema` - AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore]) + AT_CHECK([if test $1 = standalone; then + ovsdb-tool create db1 schema + else + ovsdb-tool create-cluster db1 schema unix:s1.raft + fi], [0], [stdout], [ignore]) on_exit 'kill `cat *.pid`' AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log dnl @@ -1534,13 +1540,13 @@ m4_define([OVSDB_CHECK_EXECUTION], ], [0], [ignore], [ignore]) done - m4_foreach([txn], [$3], + m4_foreach([txn], [$4], [AT_CHECK([ovsdb-client transact unix:db${target}.sock 'txn'], [0], [stdout], [ignore]) cat stdout >> output ]) - AT_CHECK([uuidfilt output], [0], [$4], [ignore]) + AT_CHECK([uuidfilt output], [0], [$5], [ignore]) AT_CHECK([ovsdb-client dump unix:db1.sock], [0], [stdout], [ignore]) for i in $(seq 2 ${n_servers}); do @@ -1548,12 +1554,26 @@ m4_define([OVSDB_CHECK_EXECUTION], diff stdout dump${i}]) done + dnl Check that transaction ids in notifications are the same on all relays. + last_id_pattern='s/\(.*"monid","[[a-z]]*".,"\)\(.*\)\(",{".*\)/\2/' + AT_CHECK([grep 'received notification, method="update3"' ovsdb-server2.log dnl + | sed $last_id_pattern > txn_ids2]) + for i in $(seq 3 ${n_servers}); do + AT_CHECK([grep 'received notification, method="update3"' ovsdb-server$i.log dnl + | sed $last_id_pattern > txn_ids$i]) + AT_CHECK([diff txn_ids2 txn_ids$i]) + done + OVSDB_SERVER_SHUTDOWN for i in $(seq 2 ${n_servers}); do OVSDB_SERVER_SHUTDOWN_N([$i]) done AT_CLEANUP]) +m4_define([OVSDB_CHECK_EXECUTION], + [OVSDB_CHECK_EXECUTION_RELAY(standalone, $@) + OVSDB_CHECK_EXECUTION_RELAY(clustered, $@)]) + EXECUTION_EXAMPLES AT_BANNER([OVSDB -- ovsdb-server replication])