Merge cockroachdb#68960

68960: roachprod: fix node environment variable propagation r=tbg a=erikgrinaker In cockroachdb#67363, configured environment variables were no longer propagated to nodes. In particular, the default node envvar `COCKROACH_ENABLE_RPC_COMPRESSION=false` was not passed, which caused a 5% performance regression on certain benchmarks. This patch fixes propagation of environment variables. Resolves cockroachdb#68887. Release note: None /cc @cockroachdb/test-eng --- I'm going on vacation next week, I'd appreciate it if you could get this merged for me. Co-authored-by: Erik Grinaker <[email protected]> Co-authored-by: Steven Danna <[email protected]>
irfansharif · Aug 17, 2021 · b606f08 · b606f08
2 parents ecab37d + 4ddc2dd
commit b606f08
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 8 deletions.
diff --git a/pkg/cmd/roachprod/install/cluster_synced.go b/pkg/cmd/roachprod/install/cluster_synced.go
@@ -69,7 +69,7 @@ type SyncedCluster struct {
 	Nodes          []int
 	Secure         bool
 	CertsDir       string
-	Env            string
+	Env            []string
 	Args           []string
 	Tag            string
 	Impl           ClusterImpl

diff --git a/pkg/cmd/roachprod/install/cockroach.go b/pkg/cmd/roachprod/install/cockroach.go
@@ -436,10 +436,10 @@ func (h *crdbInstallHelper) generateStartCmd(
 		LogDir: h.c.Impl.LogDir(h.c, nodes[nodeIdx]),
 		KeyCmd: h.generateKeyCmd(nodeIdx, extraArgs),
 		Tag:    h.c.Tag,
-		EnvVars: append([]string{
+		EnvVars: append(append([]string{
 			"GOTRACEBACK=crash",
 			"COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=1",
-		}, h.getEnvVars()...),
+		}, h.c.Env...), h.getEnvVars()...),
 		Binary:           cockroachNodeBinary(h.c, nodes[nodeIdx]),
 		StartCmd:         startCmd,
 		Args:             args,

diff --git a/pkg/cmd/roachprod/main.go b/pkg/cmd/roachprod/main.go
@@ -185,7 +185,7 @@ Available clusters:
 	c.Nodes = nodes
 	c.Secure = secure
 	c.CertsDir = certsDir
-	c.Env = strings.Join(nodeEnv, " ")
+	c.Env = nodeEnv
 	c.Args = nodeArgs
 	if tag != "" {
 		c.Tag = "/" + tag

diff --git a/pkg/cmd/roachtest/tests/clearrange.go b/pkg/cmd/roachtest/tests/clearrange.go
@@ -79,7 +79,8 @@ func runClearRange(ctx context.Context, t test.Test, c cluster.Cluster, aggressi
 		//
 		// NB: the below invocation was found to actually make it to the server at the time of writing.
 		opts = append(opts, option.StartArgs(
-			"--env", "COCKROACH_CONSISTENCY_AGGRESSIVE=true COCKROACH_ENFORCE_CONSISTENT_STATS=true",
+			"--env", "COCKROACH_CONSISTENCY_AGGRESSIVE=true",
+			"--env", "COCKROACH_ENFORCE_CONSISTENT_STATS=true",
 		))
 	}
 	c.Start(ctx, opts...)

diff --git a/pkg/cmd/roachtest/tests/kv.go b/pkg/cmd/roachtest/tests/kv.go
@@ -628,9 +628,8 @@ func registerKVSplits(r registry.Registry) {
 				c.Put(ctx, t.Cockroach(), "./cockroach", c.Range(1, nodes))
 				c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.Node(nodes+1))
 				c.Start(ctx, c.Range(1, nodes), option.StartArgs(
-					// NB: this works. Don't change it or only one of the two vars may actually
-					// make it to the server.
-					"--env", "COCKROACH_MEMPROF_INTERVAL=1m COCKROACH_DISABLE_QUIESCENCE="+strconv.FormatBool(!item.quiesce),
+					"--env", "COCKROACH_MEMPROF_INTERVAL=1m",
+					"--env", "COCKROACH_DISABLE_QUIESCENCE="+strconv.FormatBool(!item.quiesce),
 					"--args=--cache=256MiB",
 				))