From patchwork Thu Apr 1 23:21:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1461456 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::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (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 ozlabs.org (Postfix) with ESMTPS id 4FBK5X5l87z9sV5 for ; Fri, 2 Apr 2021 10:24:32 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id B2DDB84D68; Thu, 1 Apr 2021 23:24:30 +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 T_9eDgNu8TI2; Thu, 1 Apr 2021 23:24:29 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTP id CC99884BA8; Thu, 1 Apr 2021 23:24:28 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9F16BC000B; Thu, 1 Apr 2021 23:24:28 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2A004C000A for ; Thu, 1 Apr 2021 23:24:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 7211E4065A for ; Thu, 1 Apr 2021 23:22:12 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1PI4CXtwj1Wx for ; Thu, 1 Apr 2021 23:22:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp2.osuosl.org (Postfix) with ESMTPS id D4CBD40E56 for ; Thu, 1 Apr 2021 23:21:59 +0000 (UTC) X-Originating-IP: 75.54.222.30 Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 2229B1C0004; Thu, 1 Apr 2021 23:21:56 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 1 Apr 2021 16:21:07 -0700 Message-Id: <20210401232108.3902274-26-blp@ovn.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210401232108.3902274-1-blp@ovn.org> References: <20210401232108.3902274-1-blp@ovn.org> MIME-Version: 1.0 Cc: Leonid Ryzhyk , Ben Pfaff Subject: [ovs-dev] [PATCH v2 25/26] ovn-northd-ddlog: Remove Router.static_routes. 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" From: Leonid Ryzhyk This is another instance of a performance bug when a change to a router object forces a cascade of changes to relations that reference the object. This time around the problem was caused by the `Router.static_routes` field, which is copied from `nb::Logical_Router`. Luckily, this field was only used in one rule and was easy to remove. Here is how we diagnosed the issue (this may be useful for posterity): - We started with a benchmark that executed several hundreds of similar transactions (in this case, these transactions were adding new router ports). We recorded execution of the benchmark in a DDlog command file (replay.txt) and added `timestamp;` commands after each transaction in the file. - Run `make NORTHD_CLI=1` to generate the ovn_northd_cli executable and use it to execute the command file: ``` ./ovn_northd_ddlog/target/release/ovn_northd_cli -w 1 < replay.txt > replay.dump ``` - Extract only the timestamps from replay.dump and plot differences between successive timestamps (i.e., individual transaction times). I use gnumeric. It would be nice to have some automation for this in the future. We observe that one of the transactions in the benchmark loop slows down linearly as the size of the network topology grows: https://gist.github.com/ryzhyk/16a5607b280ed9cd09b176d6816cb4f0 Clearly some of the rules in the program are getting more expensive as the number of ports goes up. Another interesting observation is that the size of the delta output at each iteration of the benchmark remains constant (the delta mostly consists of new network flows). This suggests that whatever extra work DDlog is doing at each iteration might be redundant. - To identify where the wasted work happens, we re-compile the program passing the `--output-internal-relations` flag to DDlog, which tells it to dump changes to all intermediate relations, not just output relation. We replay the trace again. We locate the expensive transaction in the log and compare its output from one of the first iterations vs one from the end of the log. We now see a whole bunch of intermediate relations that only had a few modified records in the first transaction versus hundreds in the second one. We further observe that all of these changes simply update the `static_routes` field (as explained above). - After removing the field, we observe that changes to intermediate relations no longer grow with the topology, and transaction timing increases much more slowly: https://gist.github.com/ryzhyk/d02784b9088d82f8549ea1b2ebdf095e Signed-off-by: Leonid Ryzhyk Signed-off-by: Ben Pfaff --- northd/lrouter.dl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/northd/lrouter.dl b/northd/lrouter.dl index 38a19e7ea324..7ed5358c9431 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -434,7 +434,6 @@ typedef Router = Router { /* Fields copied from nb::Logical_Router. */ _uuid: uuid, name: string, - static_routes: Set, policies: Set, enabled: Option, nat: Set, @@ -459,7 +458,6 @@ relation Router[Intern] Router[Router{ ._uuid = lr._uuid, .name = lr.name, - .static_routes = lr.static_routes, .policies = lr.policies, .enabled = lr.enabled, .nat = lr.nat, @@ -730,7 +728,8 @@ RouterStaticRoute_(.router = router, .nexthop = route.nexthop, .output_port = route.output_port, .ecmp_symmetric_reply = route.ecmp_symmetric_reply) :- - router in &Router(.static_routes = routes), + router in &Router(), + nb::Logical_Router(._uuid = router._uuid, .static_routes = routes), var route_id = FlatMap(routes), route in &StaticRoute(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}).