From 66e25c823b0c820a471d3ecd6c258d22e65315bc Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Wed, 8 Apr 2020 17:37:17 +0200 Subject: [PATCH 1/2] roachtest: fix one-off in version-upgrade We weren't checking whether n4 had actually upgraded. Fixes the failure https://github.com/cockroachdb/cockroach/issues/44732#issuecomment-610454808 (but there are others, so not closing). Release note: None --- pkg/cmd/roachtest/versionupgrade.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/roachtest/versionupgrade.go b/pkg/cmd/roachtest/versionupgrade.go index b5601bd164ca..ee651374d7c0 100644 --- a/pkg/cmd/roachtest/versionupgrade.go +++ b/pkg/cmd/roachtest/versionupgrade.go @@ -396,7 +396,7 @@ func waitForUpgradeStep() versionStep { newVersion := u.binaryVersion(ctx, t, 1).String() t.l.Printf("%s: waiting for cluster to auto-upgrade\n", newVersion) - for i := 1; i < c.spec.NodeCount; i++ { + for i := 1; i <= c.spec.NodeCount; i++ { err := retry.ForDuration(30*time.Second, func() error { db := u.conn(ctx, t, i) From d805353b4a5fe95e65bb221d69e82dc5044266ad Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Wed, 8 Apr 2020 17:43:25 +0200 Subject: [PATCH 2/2] roachtest: touch up network/sanity It also had a one-off where it wasn't running against the last node. This was inconsequential but was still fixed, along with an update of the comment about what the test is trying to do. Release note: None --- pkg/cmd/roachtest/network.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/roachtest/network.go b/pkg/cmd/roachtest/network.go index ef5c9285c5bf..f71400f8a4f4 100644 --- a/pkg/cmd/roachtest/network.go +++ b/pkg/cmd/roachtest/network.go @@ -23,7 +23,8 @@ import ( ) // runNetworkSanity is just a sanity check to make sure we're setting up toxiproxy -// correctly. +// correctly. It injects latency between the nodes and verifies that we're not +// seeing the latency on the client connection running `SELECT 1` on each node. func runNetworkSanity(ctx context.Context, t *test, origC *cluster, nodes int) { origC.Put(ctx, cockroach, "./cockroach", origC.All()) c, err := Toxify(ctx, origC, origC.All()) @@ -41,8 +42,6 @@ func runNetworkSanity(ctx context.Context, t *test, origC *cluster, nodes int) { // the upstream connections aren't affected by latency below, but the fixed // cost of starting the binary and processing the query is already close to // 100ms. - // - // NB: last node gets no latency injected, but first node gets cut off below. const latency = 300 * time.Millisecond for i := 1; i <= nodes; i++ { // NB: note that these latencies only apply to connections *to* the node @@ -84,7 +83,7 @@ insert into test.commit values(3,1000), (1,1000), (2,1000); select age, message from [ show trace for session ]; `) - for i := 1; i < origC.spec.NodeCount; i++ { + for i := 1; i <= origC.spec.NodeCount; i++ { if dur := c.Measure(ctx, i, `SELECT 1`); dur > latency { t.Fatalf("node %d unexpectedly affected by latency: select 1 took %.2fs", i, dur.Seconds()) }