From patchwork Wed Aug 5 15:27:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1341281 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=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IvpFJpql; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BMFrH1mcdz9sRK for ; Thu, 6 Aug 2020 01:28:14 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9C91188221; Wed, 5 Aug 2020 15:28:12 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oqqOWtayPu6o; Wed, 5 Aug 2020 15:28:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 7C8C688189; Wed, 5 Aug 2020 15:28:09 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 602C6C0051; Wed, 5 Aug 2020 15:28:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 43C27C004C for ; Wed, 5 Aug 2020 15:28:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 2851B87886 for ; Wed, 5 Aug 2020 15:28:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SnaZ4pZXNkqn for ; Wed, 5 Aug 2020 15:28:05 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by whitealder.osuosl.org (Postfix) with ESMTPS id F0F728788C for ; Wed, 5 Aug 2020 15:28:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1596641283; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type; bh=gHMq3R9gcbFs6IU+suBCX2HuJrO4SHgAKF3Wk7ZdvZk=; b=IvpFJpqloGj4EKqCLcwQK5zcq0dcF0vyxBpW4Z9pPOkLFRcIuEzFvR7rrGHmknJzdBEHFP xsCPzDomHZfOttO9XWOX25Bymf9JETaqBVJihLzYcUfr+v5GvPG8oYjlUco9CFhSHYQj5f hmTgryYSvfm16Iylgicpa90023Luebk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-277-RqdDuSMdOpO-DZbKa5Zgtg-1; Wed, 05 Aug 2020 11:28:00 -0400 X-MC-Unique: RqdDuSMdOpO-DZbKa5Zgtg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4A21B19057A1; Wed, 5 Aug 2020 15:27:59 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-80.ams2.redhat.com [10.36.114.80]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5CE0C19C71; Wed, 5 Aug 2020 15:27:58 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Wed, 5 Aug 2020 17:27:53 +0200 Message-Id: <1596641273-5748-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: Han Zhou Subject: [ovs-dev] [PATCH] ovsdb-server: Replace in-memory DB contents at raft install_snapshot. 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Every time a follower has to install a snapshot received from the leader, it should also replace the data in memory. Right now this only happens when snapshots are installed that also change the schema. This can lead to inconsistent DB data on follower nodes and the snapshot may fail to get applied. CC: Han Zhou Fixes: bda1f6b60588 ("ovsdb-server: Don't disconnect clients after raft install_snapshot.") Signed-off-by: Dumitru Ceara Acked-by: Han Zhou --- ovsdb/ovsdb-server.c | 21 +++++++++++++-------- tests/idltest.ovsschema | 9 +++++++++ tests/ovsdb-cluster.at | 28 +++++++++++++++++++++++++--- tests/ovsdb-idl.at | 1 + 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index ef4e996..fd7891a 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -543,13 +543,14 @@ parse_txn(struct server_config *config, struct db *db, const struct ovsdb_schema *schema, const struct json *txn_json, const struct uuid *txnid) { - if (schema && (!db->db->schema || strcmp(schema->version, - db->db->schema->version))) { + if (schema) { /* We're replacing the schema (and the data). Destroy the database * (first grabbing its storage), then replace it with the new schema. * The transaction must also include the replacement data. * - * Only clustered database schema changes go through this path. */ + * Only clustered database schema changes and snapshot installs + * go through this path. + */ ovs_assert(txn_json); ovs_assert(ovsdb_storage_is_clustered(db->db->storage)); @@ -559,11 +560,15 @@ parse_txn(struct server_config *config, struct db *db, return error; } - ovsdb_jsonrpc_server_reconnect( - config->jsonrpc, false, - (db->db->schema - ? xasprintf("database %s schema changed", db->db->name) - : xasprintf("database %s connected to storage", db->db->name))); + if (!db->db->schema || + strcmp(schema->version, db->db->schema->version)) { + ovsdb_jsonrpc_server_reconnect( + config->jsonrpc, false, + (db->db->schema + ? xasprintf("database %s schema changed", db->db->name) + : xasprintf("database %s connected to storage", + db->db->name))); + } ovsdb_replace(db->db, ovsdb_create(ovsdb_schema_clone(schema), NULL)); diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema index e02b975..e04755e 100644 --- a/tests/idltest.ovsschema +++ b/tests/idltest.ovsschema @@ -54,6 +54,15 @@ }, "isRoot" : true }, + "indexed": { + "columns": { + "i": { + "type": "integer" + } + }, + "indexes": [["i"]], + "isRoot" : true + }, "simple": { "columns": { "b": { diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at index 9714545..06d27c9 100644 --- a/tests/ovsdb-cluster.at +++ b/tests/ovsdb-cluster.at @@ -332,15 +332,31 @@ for i in `seq $n`; do AT_CHECK([ovsdb_client_wait unix:s$i.ovsdb $schema_name connected]) done +AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest", + {"op": "insert", + "table": "indexed", + "row": {"i": 1}}]]'], [0], [ignore], [ignore]) + # Kill one follower (s2) and write some data to cluster, so that the follower is falling behind printf "\ns2: stopping\n" OVS_APP_EXIT_AND_WAIT_BY_TARGET([`pwd`/s2], [s2.pid]) +# Delete "i":1 and readd it to get a different UUID for it. +AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest", + {"op": "delete", + "table": "indexed", + "where": [["i", "==", 1]]}]]'], [0], [ignore], [ignore]) + AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest", {"op": "insert", - "table": "simple", + "table": "indexed", "row": {"i": 1}}]]'], [0], [ignore], [ignore]) +AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest", + {"op": "insert", + "table": "indexed", + "row": {"i": 2}}]]'], [0], [ignore], [ignore]) + # Compact leader online to generate snapshot AT_CHECK([ovs-appctl -t "`pwd`"/s1 ovsdb-server/compact]) @@ -355,8 +371,14 @@ AT_CHECK([ovsdb_client_wait unix:s2.ovsdb $schema_name connected]) # succeed. AT_CHECK([ovsdb-client transact unix:s2.ovsdb '[["idltest", {"op": "insert", - "table": "simple", - "row": {"i": 1}}]]'], [0], [ignore], [ignore]) + "table": "indexed", + "row": {"i": 3}}]]'], [0], [ignore], [ignore]) + +# The snapshot should overwrite the in-memory contents of the DB on S2 +# without generating any constraint violations. +AT_CHECK([grep 'Transaction causes multiple rows in "indexed" table to have identical values (1) for index on column "i"' -c s2.log], [1], [dnl +0 +]) for i in `seq $n`; do OVS_APP_EXIT_AND_WAIT_BY_TARGET([`pwd`/s$i], [s$i.pid]) diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 4efed88..789ae23 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -954,6 +954,7 @@ AT_CHECK([sort stdout | uuidfilt], [0], # Check that ovsdb-idl figured out that table link2 and column l2 are missing. AT_CHECK([grep ovsdb_idl stderr | sort], [0], [dnl +test-ovsdb|ovsdb_idl|idltest database lacks indexed table (database needs upgrade?) test-ovsdb|ovsdb_idl|idltest database lacks link2 table (database needs upgrade?) test-ovsdb|ovsdb_idl|idltest database lacks simple5 table (database needs upgrade?) test-ovsdb|ovsdb_idl|idltest database lacks singleton table (database needs upgrade?)