From patchwork Thu May 7 11:20:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1285161 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.136; helo=silver.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=COIMJqAg; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49HrcK0hPxz9sSG for ; Thu, 7 May 2020 21:20:49 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 653FE26348; Thu, 7 May 2020 11:20:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id l+Ho9xuTbJpK; Thu, 7 May 2020 11:20:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id DB72420031; Thu, 7 May 2020 11:20:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B8B87C0890; Thu, 7 May 2020 11:20:42 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 33725C07FF for ; Thu, 7 May 2020 11:20:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 16D4C87145 for ; Thu, 7 May 2020 11:20:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qzM8NxfcaITA for ; Thu, 7 May 2020 11:20:40 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 3F774873FD for ; Thu, 7 May 2020 11:20:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588850439; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hp4TzgnNzr61oXt8EPFXJXPPUySiwDqRFJm6tKVg80k=; b=COIMJqAgvcioi6u1i1fMuS+UnV5IsTPQq6EQLq0z9dkzovONt8KTPJ2PsZIMIlrgfo3L1k NS9BBgeRtJlVNXusrqpqVG2gdLYh6nAx8ko5fmh/hv40ZVvHGDuo/s/tuDUlWwj2hBZLnD 93W0wOVewut/ldM64MWMxFL4eHpyBUo= 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-356-2gqLtycDOl6DeU1K_mSf1Q-1; Thu, 07 May 2020 07:20:37 -0400 X-MC-Unique: 2gqLtycDOl6DeU1K_mSf1Q-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9E304835B42; Thu, 7 May 2020 11:20:36 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-102.ams2.redhat.com [10.36.114.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id BFFD160C81; Thu, 7 May 2020 11:20:35 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 7 May 2020 13:20:31 +0200 Message-Id: <20200507112028.31976.20915.stgit@dceara.remote.csb> In-Reply-To: <20200507112013.31976.54079.stgit@dceara.remote.csb> References: <20200507112013.31976.54079.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org, i.maximets@ovn.org Subject: [ovs-dev] [PATCH v5 1/3] ovsdb-server.7: Mention update3 as replies to monitor_cond_change. 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" Monitor_cond_change might trigger updates to be sent to clients as results to condition changes. These updates can be either update2 (for monitor_cond monitors) or update3 (for monitor_cond_since monitors). The documentation used to mention only update2. Signed-off-by: Dumitru Ceara Acked-by: Han Zhou --- Documentation/ref/ovsdb-server.7.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/ref/ovsdb-server.7.rst b/Documentation/ref/ovsdb-server.7.rst index 967761b..0441435 100644 --- a/Documentation/ref/ovsdb-server.7.rst +++ b/Documentation/ref/ovsdb-server.7.rst @@ -316,9 +316,9 @@ monitor request, will contain any matched rows by old condition and not matched by the new condition. Changes according to the new conditions are automatically sent to the client -using the ``update2`` monitor notification. An update, if any, as a result of -a condition change, will be sent to the client before the reply to the -``monitor_cond_change`` request. +using the ``update2`` or ``update3`` monitor notification depending on the +monitor method. An update, if any, as a result of a condition change, will +be sent to the client before the reply to the ``monitor_cond_change`` request. 4.1.14 Update2 notification --------------------------- From patchwork Thu May 7 11:20:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1285162 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=Fj037UmH; 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 49Hrcd58v6z9sSG for ; Thu, 7 May 2020 21:21:05 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9FFB3893E9; Thu, 7 May 2020 11:21:03 +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 LW5skvBwV3cg; Thu, 7 May 2020 11:21:01 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id C4AB38937B; Thu, 7 May 2020 11:21:01 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B7B6DC0865; Thu, 7 May 2020 11:21:01 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8251CC0865 for ; Thu, 7 May 2020 11:21:00 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 672BF26248 for ; Thu, 7 May 2020 11:21:00 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 44Ej08Gfpl3p for ; Thu, 7 May 2020 11:20:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by silver.osuosl.org (Postfix) with ESMTPS id 4A3032043E for ; Thu, 7 May 2020 11:20:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588850455; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=z1BbrH5K0md2SPLLPTV3qMYn4H1psu9JHiQq484YCK0=; b=Fj037UmHrUKvu9t9MirXQToPvNz3LZcc4Gdhe3BqsWjjyEAaC6o5Ag4qiVWwZQL0hi+Mrz F87QtC3KU7qiJa7NLesa1xmaekRMxQqdMg0ZinVm08+8IRgKBqL2XV6gZQ/I+Lvmpy8Xbg 2EzxEk0UEkve9jdrXD9J3C8sZUXu3pE= 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-30-Rt_6RPaHMwKk3f6_DM-s6A-1; Thu, 07 May 2020 07:20:50 -0400 X-MC-Unique: Rt_6RPaHMwKk3f6_DM-s6A-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E0DE98015D2; Thu, 7 May 2020 11:20:49 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-102.ams2.redhat.com [10.36.114.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id E0B625C1D4; Thu, 7 May 2020 11:20:48 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 7 May 2020 13:20:46 +0200 Message-Id: <20200507112041.31976.7419.stgit@dceara.remote.csb> In-Reply-To: <20200507112013.31976.54079.stgit@dceara.remote.csb> References: <20200507112013.31976.54079.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org, i.maximets@ovn.org Subject: [ovs-dev] [PATCH v5 2/3] ovsdb-idl: Avoid inconsistent IDL state with OVSDB_MONITOR_V3. 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" Assuming an ovsdb client connected to a database using OVSDB_MONITOR_V3 (i.e., "monitor_cond_since" method) with the initial monitor condition MC1. Assuming the following two transactions are executed on the ovsdb-server: TXN1: "insert record R1 in table T1" TXN2: "insert record R2 in table T2" If the client's monitor condition MC1 for table T2 matches R2 then the client will receive the following update3 message: method="update3", "insert record R2 in table T2", last-txn-id=TXN2 At this point, if the presence of the new record R2 in the IDL triggers the client to update its monitor condition to MC2 and add a clause for table T1 which matches R1, a monitor_cond_change message is sent to the server: method="monitor_cond_change", "clauses from MC2" In normal operation the ovsdb-server will reply with a new update3 message of the form: method="update3", "insert record R1 in table T1", last-txn-id=TXN2 However, if the connection drops in the meantime, this last update might get lost. It might happen that during the reconnect a new transaction happens that modifies the original record R1: TXN3: "modify record R1 in table T1" When the client reconnects, it will try to perform a fast resync by sending: method="monitor_cond_since", "clauses from MC2", last-txn-id=TXN2 Because TXN2 is still in the ovsdb-server transaction history, the server replies with the changes from the most recent transactions only, i.e., TXN3: result="true", last-txbb-id=TXN3, "modify record R1 in table T1" This causes the IDL on the client in to end up in an inconsistent state because it has never seen the update that created R1. Such a scenario is described in: https://bugzilla.redhat.com/show_bug.cgi?id=1808580#c22 To avoid this issue, the IDL will now maintain (up to) 3 different types of conditions for each DB table: - new_cond: condition that has been set by the IDL client but has not yet been sent to the server through monitor_cond_change. - req_cond: condition that has been sent to the server but the reply acknowledging the change hasn't been received yet. - ack_cond: condition that has been acknowledged by the server. Whenever the IDL FSM is restarted (e.g., voluntary or involuntary disconnect): - if there is a known last_id txn-id the code ensures that new_cond will contain the most recent condition set by the IDL client (either req_cond if there was a request in flight, or new_cond if the IDL client set a condition while the IDL was disconnected) - if there is no known last_id txn-id the code ensures that ack_cond will contain the most recent conditions set by the IDL client regardless whether they were acked by the server or not. When monitor_cond_since/monitor_cond requests are sent they will always include ack_cond and if new_cond is not NULL a follow up monitor_cond_change will be generated afterwards. On the other hand ovsdb_idl_db_set_condition() will always modify new_cond. This ensures that updates of type "insert" that happened before the last transaction known by the IDL but didn't match old monitor conditions are sent upon reconnect if the monitor condition has changed to include them in the meantime. CC: Han Zhou Fixes: 403a6a0cb003 ("ovsdb-idl: Fast resync from server when connection reset.") Signed-off-by: Dumitru Ceara --- lib/ovsdb-idl-provider.h | 8 ++ lib/ovsdb-idl.c | 148 +++++++++++++++++++++++++++++++++++++++------- tests/ovsdb-idl.at | 56 +++++++++++++++++ 3 files changed, 187 insertions(+), 25 deletions(-) diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h index 30d1d08..00497d9 100644 --- a/lib/ovsdb-idl-provider.h +++ b/lib/ovsdb-idl-provider.h @@ -122,8 +122,12 @@ struct ovsdb_idl_table { unsigned int change_seqno[OVSDB_IDL_CHANGE_MAX]; struct ovs_list indexes; /* Contains "struct ovsdb_idl_index"s */ struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). */ - struct ovsdb_idl_condition condition; - bool cond_changed; + struct ovsdb_idl_condition *ack_cond; /* Last condition acked by the + * server. */ + struct ovsdb_idl_condition *req_cond; /* Last condition requested to the + * server. */ + struct ovsdb_idl_condition *new_cond; /* Latest condition set by the IDL + * client. */ }; struct ovsdb_idl_class { diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 1535ad7..557f61c 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -240,6 +240,10 @@ static void ovsdb_idl_send_monitor_request(struct ovsdb_idl *, struct ovsdb_idl_db *, enum ovsdb_idl_monitor_method); static void ovsdb_idl_db_clear(struct ovsdb_idl_db *db); +static void ovsdb_idl_db_ack_condition(struct ovsdb_idl_db *db); +static void ovsdb_idl_db_sync_condition(struct ovsdb_idl_db *db); +static void ovsdb_idl_condition_move(struct ovsdb_idl_condition **dst, + struct ovsdb_idl_condition **src); struct ovsdb_idl { struct ovsdb_idl_db server; @@ -422,9 +426,11 @@ ovsdb_idl_db_init(struct ovsdb_idl_db *db, const struct ovsdb_idl_class *class, = table->change_seqno[OVSDB_IDL_CHANGE_MODIFY] = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; table->db = db; - ovsdb_idl_condition_init(&table->condition); - ovsdb_idl_condition_add_clause_true(&table->condition); - table->cond_changed = false; + table->ack_cond = NULL; + table->req_cond = NULL; + table->new_cond = xmalloc(sizeof *table->new_cond); + ovsdb_idl_condition_init(table->new_cond); + ovsdb_idl_condition_add_clause_true(table->new_cond); } db->monitor_id = json_array_create_2(json_string_create("monid"), json_string_create(class->database)); @@ -556,12 +562,15 @@ ovsdb_idl_set_shuffle_remotes(struct ovsdb_idl *idl, bool shuffle) static void ovsdb_idl_db_destroy(struct ovsdb_idl_db *db) { + struct ovsdb_idl_condition *null_cond = NULL; ovs_assert(!db->txn); ovsdb_idl_db_txn_abort_all(db); ovsdb_idl_db_clear(db); for (size_t i = 0; i < db->class_->n_tables; i++) { struct ovsdb_idl_table *table = &db->tables[i]; - ovsdb_idl_condition_destroy(&table->condition); + ovsdb_idl_condition_move(&table->ack_cond, &null_cond); + ovsdb_idl_condition_move(&table->req_cond, &null_cond); + ovsdb_idl_condition_move(&table->new_cond, &null_cond); ovsdb_idl_destroy_indexes(table); shash_destroy(&table->columns); hmap_destroy(&table->rows); @@ -690,6 +699,12 @@ ovsdb_idl_send_request(struct ovsdb_idl *idl, struct jsonrpc_msg *request) static void ovsdb_idl_restart_fsm(struct ovsdb_idl *idl) { + /* Resync data DB table conditions to avoid missing updates due to + * conditions that were in flight or changed locally while the connection + * was down. + */ + ovsdb_idl_db_sync_condition(&idl->data); + ovsdb_idl_send_schema_request(idl, &idl->server); ovsdb_idl_transition(idl, IDL_S_SERVER_SCHEMA_REQUESTED); idl->data.monitoring = OVSDB_IDL_NOT_MONITORING; @@ -797,7 +812,9 @@ ovsdb_idl_process_response(struct ovsdb_idl *idl, struct jsonrpc_msg *msg) * do, it's a "monitor_cond_change", which means that the conditional * monitor clauses were updated. * - * If further condition changes were pending, send them now. */ + * Mark the last requested conditions and acked and if further + * condition changes were pending, send them now. */ + ovsdb_idl_db_ack_condition(&idl->data); ovsdb_idl_send_cond_change(idl); idl->data.cond_seqno++; break; @@ -1493,30 +1510,60 @@ ovsdb_idl_condition_equals(const struct ovsdb_idl_condition *a, } static void -ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dst, +ovsdb_idl_condition_clone(struct ovsdb_idl_condition **dst, const struct ovsdb_idl_condition *src) { - ovsdb_idl_condition_init(dst); + if (*dst) { + ovsdb_idl_condition_destroy(*dst); + } else { + *dst = xmalloc(sizeof **dst); + } + ovsdb_idl_condition_init(*dst); - dst->is_true = src->is_true; + (*dst)->is_true = src->is_true; const struct ovsdb_idl_clause *clause; HMAP_FOR_EACH (clause, hmap_node, &src->clauses) { - ovsdb_idl_condition_add_clause__(dst, clause, clause->hmap_node.hash); + ovsdb_idl_condition_add_clause__(*dst, clause, clause->hmap_node.hash); } } +static void +ovsdb_idl_condition_move(struct ovsdb_idl_condition **dst, + struct ovsdb_idl_condition **src) +{ + if (*dst) { + ovsdb_idl_condition_destroy(*dst); + free(*dst); + } + *dst = *src; + *src = NULL; +} + static unsigned int ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db, const struct ovsdb_idl_table_class *tc, const struct ovsdb_idl_condition *condition) { + struct ovsdb_idl_condition *table_cond; struct ovsdb_idl_table *table = ovsdb_idl_db_table_from_class(db, tc); unsigned int seqno = db->cond_seqno; - if (!ovsdb_idl_condition_equals(condition, &table->condition)) { - ovsdb_idl_condition_destroy(&table->condition); - ovsdb_idl_condition_clone(&table->condition, condition); - db->cond_changed = table->cond_changed = true; + + /* Compare the new condition to the last known condition which can be + * either "new" (not sent yet), "requested" or "acked", in this order. + */ + if (table->new_cond) { + table_cond = table->new_cond; + } else if (table->req_cond) { + table_cond = table->req_cond; + } else { + table_cond = table->ack_cond; + } + ovs_assert(table_cond); + + if (!ovsdb_idl_condition_equals(condition, table_cond)) { + ovsdb_idl_condition_clone(&table->new_cond, condition); + db->cond_changed = true; poll_immediate_wake(); return seqno + 1; } @@ -1561,9 +1608,8 @@ ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd) } static struct json * -ovsdb_idl_create_cond_change_req(struct ovsdb_idl_table *table) +ovsdb_idl_create_cond_change_req(const struct ovsdb_idl_condition *cond) { - const struct ovsdb_idl_condition *cond = &table->condition; struct json *monitor_cond_change_request = json_object_create(); struct json *cond_json = ovsdb_idl_condition_to_json(cond); @@ -1583,8 +1629,9 @@ ovsdb_idl_db_compose_cond_change(struct ovsdb_idl_db *db) for (size_t i = 0; i < db->class_->n_tables; i++) { struct ovsdb_idl_table *table = &db->tables[i]; - if (table->cond_changed) { - struct json *req = ovsdb_idl_create_cond_change_req(table); + if (table->new_cond) { + struct json *req = + ovsdb_idl_create_cond_change_req(table->new_cond); if (req) { if (!monitor_cond_change_requests) { monitor_cond_change_requests = json_object_create(); @@ -1593,7 +1640,11 @@ ovsdb_idl_db_compose_cond_change(struct ovsdb_idl_db *db) table->class_->name, json_array_create_1(req)); } - table->cond_changed = false; + /* Mark the new condition as requested by moving it to req_cond. + * If there's already requested condition that's a bug. + */ + ovs_assert(table->req_cond == NULL); + ovsdb_idl_condition_move(&table->req_cond, &table->new_cond); } } @@ -1608,6 +1659,60 @@ ovsdb_idl_db_compose_cond_change(struct ovsdb_idl_db *db) return jsonrpc_create_request("monitor_cond_change", params, NULL); } +/* Marks all requested table conditions in 'db' as acked by the server. + * It should be called when the server replies to monitor_cond_change + * requests. + */ +static void +ovsdb_idl_db_ack_condition(struct ovsdb_idl_db *db) +{ + for (size_t i = 0; i < db->class_->n_tables; i++) { + struct ovsdb_idl_table *table = &db->tables[i]; + + if (table->req_cond) { + ovsdb_idl_condition_move(&table->ack_cond, &table->req_cond); + } + } +} + +/* Should be called when the IDL fsm is restarted and resyncs table conditions + * based on the state the DB is in: + * - if a non-zero last_id is available for the DB then upon reconnect, + * if the IDL will use monitor_cond_since, it should first request acked + * conditions to avoid missing updates about records that were added before + * the transaction with txn-id == last_id. If there were requested + * condition changes in flight (i.e., req_cond not NULL) and the IDL + * client didn't set new conditions (i.e., new_cond is NULL) then move + * req_cond to new_cond to trigger a follow up cond_change request. + * - if there's no last_id available for the DB then it's safe to use the + * latest conditions set by the IDL client even if they weren't acked yet. + */ +static void +ovsdb_idl_db_sync_condition(struct ovsdb_idl_db *db) +{ + bool ack_all = uuid_is_zero(&db->last_id); + + db->cond_changed = false; + for (size_t i = 0; i < db->class_->n_tables; i++) { + struct ovsdb_idl_table *table = &db->tables[i]; + + if (ack_all) { + if (table->new_cond) { + ovsdb_idl_condition_move(&table->req_cond, &table->new_cond); + } + + if (table->req_cond) { + ovsdb_idl_condition_move(&table->ack_cond, &table->req_cond); + } + } else { + if (table->req_cond && !table->new_cond) { + ovsdb_idl_condition_move(&table->new_cond, &table->req_cond); + db->cond_changed = true; + } + } + } +} + static void ovsdb_idl_send_cond_change(struct ovsdb_idl *idl) { @@ -2062,13 +2167,12 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl, struct ovsdb_idl_db *db, monitor_request = json_object_create(); json_object_put(monitor_request, "columns", columns); - const struct ovsdb_idl_condition *cond = &table->condition; + const struct ovsdb_idl_condition *cond = table->ack_cond; if ((monitor_method == OVSDB_IDL_MM_MONITOR_COND || monitor_method == OVSDB_IDL_MM_MONITOR_COND_SINCE) && - !ovsdb_idl_condition_is_true(cond)) { + cond && !ovsdb_idl_condition_is_true(cond)) { json_object_put(monitor_request, "where", ovsdb_idl_condition_to_json(cond)); - table->cond_changed = false; } json_object_put(monitor_requests, tc->name, json_array_create_1(monitor_request)); @@ -2076,8 +2180,6 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl, struct ovsdb_idl_db *db, } free_schema(schema); - db->cond_changed = false; - struct json *params = json_array_create_3( json_string_create(db->class_->database), json_clone(db->monitor_id), diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index b5cbee7..4efed88 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -1828,3 +1828,59 @@ m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY], OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL connects to leader], 3, ['remote']) OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL reconnects to leader], 3, ['remote' '+remotestop' 'remote']) + +# same as OVSDB_CHECK_IDL but uses C IDL implementation with tcp +# with multiple remotes. +m4_define([OVSDB_CHECK_CLUSTER_IDL_C], + [AT_SETUP([$1 - C - tcp]) + AT_KEYWORDS([ovsdb server idl positive tcp socket $5]) + m4_define([LPBK],[127.0.0.1]) + AT_CHECK([ovsdb_cluster_start_idltest $2 "ptcp:0:"LPBK]) + PARSE_LISTENING_PORT([s1.log], [TCP_PORT_1]) + PARSE_LISTENING_PORT([s2.log], [TCP_PORT_2]) + PARSE_LISTENING_PORT([s3.log], [TCP_PORT_3]) + remotes=tcp:LPBK:$TCP_PORT_1,tcp:LPBK:$TCP_PORT_2,tcp:LPBK:$TCP_PORT_3 + + m4_if([$3], [], [], + [AT_CHECK([ovsdb-client transact $remotes $3], [0], [ignore], [ignore])]) + AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 idl tcp:LPBK:$TCP_PORT_1 $4], + [0], [stdout], [ignore]) + AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]), + [0], [$5]) + AT_CLEANUP]) + +# Checks that monitor_cond_since works fine when disconnects happen +# with cond_change requests in flight (i.e., IDL is properly updated). +OVSDB_CHECK_CLUSTER_IDL_C([simple idl, monitor_cond_since, cluster disconnect], + 3, + [['["idltest", + {"op": "insert", + "table": "simple", + "row": {"i": 1, + "r": 1.0, + "b": true}}, + {"op": "insert", + "table": "simple", + "row": {"i": 2, + "r": 1.0, + "b": true}}]']], + [['condition simple []' \ + 'condition simple [["i","==",2]]' \ + 'condition simple [["i","==",1]]' \ + '+reconnect' \ + '["idltest", + {"op": "update", + "table": "simple", + "where": [["i", "==", 1]], + "row": {"r": 2.0 }}]']], + [[000: change conditions +001: empty +002: change conditions +003: i=2 r=1 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +004: change conditions +005: reconnect +006: i=2 r=1 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +007: {"error":null,"result":[{"count":1}]} +008: i=1 r=2 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> +009: done +]]) From patchwork Thu May 7 11:21:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1285163 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.136; helo=silver.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=WdkfSWld; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49Hrct1TsMz9sSG for ; Thu, 7 May 2020 21:21:18 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id AAC292639B; Thu, 7 May 2020 11:21:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bZ098VDJtOof; Thu, 7 May 2020 11:21:14 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 9BDD826316; Thu, 7 May 2020 11:21:14 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 906F7C088E; Thu, 7 May 2020 11:21:14 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 49597C0865 for ; Thu, 7 May 2020 11:21:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 33AB32043E for ; Thu, 7 May 2020 11:21:13 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AqcKWYnMqUf6 for ; Thu, 7 May 2020 11:21:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by silver.osuosl.org (Postfix) with ESMTPS id 198D51FC94 for ; Thu, 7 May 2020 11:21:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588850471; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gj+YQjlHhgwPuClWReehOsFneHxDZbVCY/Hp9cOQ4WA=; b=WdkfSWldIFREYqXSgyhk+gb4vTMFN/V3dbQ5ofHFkuJoUOPoSOYUlLXFNaEov71Va+Cb/s 7F5OBUcTuusbIjI1HxbqSmhdRV92mnSjlv4lgo2nuP+ATNL9AnTSjyuwkr77/WTD9kC57t EkokaQA+oKQopWIDg3Nt4XapaA54etA= 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-360-_OmqKZ5DOmyKZrtqdMTlaQ-1; Thu, 07 May 2020 07:21:09 -0400 X-MC-Unique: _OmqKZ5DOmyKZrtqdMTlaQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7072218FE860; Thu, 7 May 2020 11:21:08 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-102.ams2.redhat.com [10.36.114.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6812561527; Thu, 7 May 2020 11:21:07 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 7 May 2020 13:21:04 +0200 Message-Id: <20200507112055.31976.98059.stgit@dceara.remote.csb> In-Reply-To: <20200507112013.31976.54079.stgit@dceara.remote.csb> References: <20200507112013.31976.54079.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org, i.maximets@ovn.org Subject: [ovs-dev] [PATCH v5 3/3] ovsdb-idl: Force IDL retry when missing updates encountered. 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" Adds a generic recovery mechanism which triggers an IDL retry with fast resync disabled in case the IDL has detected that it ended up in an inconsistent state due to other bugs in the ovsdb-server/ovsdb-idl implementation. CC: Andy Zhou Fixes: db2b5757328c ("lib: add monitor2 support in ovsdb-idl.") Signed-off-by: Dumitru Ceara --- lib/ovsdb-idl.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 557f61c..076f0a1 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -328,7 +328,8 @@ static bool ovsdb_idl_process_update(struct ovsdb_idl_table *, static bool ovsdb_idl_process_update2(struct ovsdb_idl_table *, const struct uuid *, const char *operation, - const struct json *row); + const struct json *row, + bool *inconsistent); static void ovsdb_idl_insert_row(struct ovsdb_idl_row *, const struct json *); static void ovsdb_idl_delete_row(struct ovsdb_idl_row *); static bool ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct json *); @@ -2420,10 +2421,26 @@ ovsdb_idl_db_parse_update__(struct ovsdb_idl_db *db, row = shash_find_data(json_object(row_update), operation); if (row) { + bool inconsistent = false; + if (ovsdb_idl_process_update2(table, &uuid, operation, - row)) { + row, &inconsistent)) { db->change_seqno++; } + + /* If the db is in an inconsistent state, clear the + * db->last_id and retry to get the complete view of + * the database. + */ + if (inconsistent) { + memset(&db->last_id, 0, sizeof db->last_id); + ovsdb_idl_retry(db->idl); + return ovsdb_error(NULL, + " received for " + "inconsistent IDL: " + "reconnecting IDL with " + "fast resync disabled"); + } break; } } @@ -2547,16 +2564,26 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table, } /* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false - * otherwise. */ + * otherwise. + * + * NOTE: When processing the "modify" updates, the IDL can determine that + * previous updates were missed (e.g., due to bugs) and that rows that don't + * exist locally should be updated. This indicates that the + * IDL is in an inconsistent state and, unlike in ovsdb_idl_process_update(), + * the missing rows cannot be inserted. If this is the case, 'inconsistent' + * is set to true to indicate the catastrophic failure. + */ static bool ovsdb_idl_process_update2(struct ovsdb_idl_table *table, const struct uuid *uuid, const char *operation, - const struct json *json_row) + const struct json *json_row, + bool *inconsistent) { struct ovsdb_idl_row *row; row = ovsdb_idl_get_row(table, uuid); + *inconsistent = false; if (!strcmp(operation, "delete")) { /* Delete row. */ if (row && !ovsdb_idl_row_is_orphan(row)) { @@ -2588,11 +2615,13 @@ ovsdb_idl_process_update2(struct ovsdb_idl_table *table, VLOG_WARN_RL(&semantic_rl, "cannot modify missing but " "referenced row "UUID_FMT" in table %s", UUID_ARGS(uuid), table->class_->name); + *inconsistent = true; return false; } } else { VLOG_WARN_RL(&semantic_rl, "cannot modify missing row "UUID_FMT" " "in table %s", UUID_ARGS(uuid), table->class_->name); + *inconsistent = true; return false; } } else {