From 393120d8b227590c5fd3a997d1bf20b17b8a5a6f Mon Sep 17 00:00:00 2001 From: Aaron Zinger Date: Wed, 13 Jul 2022 12:52:03 -0400 Subject: [PATCH 1/6] test: enforce skip.UnderStress[Race] locally Previously, skip.UnderStress only checked NightlyStress(), which is only set during nightly stress test runs, not by ./dev test --stress or make stress. This lead to developers checking in tests that flake or reliably fail under stress or stressrace. This PR modifies dev and make to set a COCKROACH_STRESS environment variable, and modifies skip to honor both that flag and the one set by nightly stress. A follow-up might be to default to not skipping when a filter matches only one test, as there the intent was almost certainly to actually run it. Release note: None --- Makefile | 4 ++-- pkg/cmd/dev/test.go | 1 + pkg/cmd/dev/testdata/datadriven/test | 2 +- pkg/cmd/dev/testdata/datadriven/testlogic | 10 +++++----- pkg/testutils/skip/skip.go | 4 ++-- pkg/testutils/skip/stress.go | 12 ++++++++++-- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 4577fec542a1..434298c816ca 100644 --- a/Makefile +++ b/Makefile @@ -1154,10 +1154,10 @@ check test testshort testrace testdeadlock bench benchshort: .PHONY: stress stressrace stress: ## Run tests under stress. $(info $(yellow)[WARNING] Use `dev test --stress $(subst ./,,$(PKG))$(if $(filter . -,$(TESTS)),, --filter $(TESTS))$(if $(TESTFLAGS), --test-args "$(TESTFLAGS)")$(if $(findstring 60m,$(TESTTIMEOUT)),, --timeout $(TESTTIMEOUT))` instead.$(term-reset)) - $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) + COCKROACH_STRESS=true $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) stressrace: ## Run tests under stress with the race detector enabled. $(info $(yellow)[WARNING] Use `dev test --stress --race $(subst ./,,$(PKG))$(if $(filter . -,$(TESTS)),, --filter $(TESTS))$(if $(TESTFLAGS), --test-args "$(TESTFLAGS)")$(if $(findstring 60m,$(TESTTIMEOUT)),, --timeout $(TESTTIMEOUT))` instead.$(term-reset)) - $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) + COCKROACH_STRESS=true $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) .PHONY: roachprod-stress roachprod-stressrace roachprod-stress roachprod-stressrace: bin/roachprod-stress diff --git a/pkg/cmd/dev/test.go b/pkg/cmd/dev/test.go index 8320ca086c17..998198d0d3d6 100644 --- a/pkg/cmd/dev/test.go +++ b/pkg/cmd/dev/test.go @@ -343,6 +343,7 @@ func (d *dev) getStressArgs(stressCmdArg string, timeout time.Duration) []string if stressCmdArg != "" { stressCmdArgs = append(stressCmdArgs, stressCmdArg) } + stressArgs = append(stressArgs, "--test_env=COCKROACH_STRESS=true") stressArgs = append(stressArgs, "--run_under", // NB: Run with -bazel, which propagates `TEST_TMPDIR` to `TMPDIR`, // and -shardable-artifacts set such that we can merge the XML output diff --git a/pkg/cmd/dev/testdata/datadriven/test b/pkg/cmd/dev/testdata/datadriven/test index d6b85f800507..ad141df3b317 100644 --- a/pkg/cmd/dev/testdata/datadriven/test +++ b/pkg/cmd/dev/testdata/datadriven/test @@ -26,7 +26,7 @@ bazel test pkg/util/tracing:all --nocache_test_results --test_env=GOTRACEBACK=al exec dev test --stress pkg/util/tracing --filter TestStartChild* --cpus=12 --timeout=25s ---- -bazel test --local_cpu_resources=12 pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed +bazel test --local_cpu_resources=12 pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed exec dev test //pkg/testutils --timeout=10s diff --git a/pkg/cmd/dev/testdata/datadriven/testlogic b/pkg/cmd/dev/testdata/datadriven/testlogic index eec452ebdfae..fee15e2a633c 100644 --- a/pkg/cmd/dev/testdata/datadriven/testlogic +++ b/pkg/cmd/dev/testdata/datadriven/testlogic @@ -50,12 +50,12 @@ bazel test --test_env=GOTRACEBACK=all --test_arg -test.count=5 --test_arg -show- exec dev testlogic base --files=auto_span_config_reconciliation --stress ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --timeout 1m --cpus 8 ---- -bazel test --test_env=GOTRACEBACK=all --local_cpu_resources=8 --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=120 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=1m0s -p=8' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --local_cpu_resources=8 --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=120 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=1m0s -p=8' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic ccl --rewrite --show-logs -v --files distsql_automatic_stats --config 3node-tenant @@ -66,14 +66,14 @@ bazel test --test_env=GOTRACEBACK=all --nocache_test_results --test_arg -test.v exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed diff --git a/pkg/testutils/skip/skip.go b/pkg/testutils/skip/skip.go index 6be83ff2bb5f..3b1bf65f9193 100644 --- a/pkg/testutils/skip/skip.go +++ b/pkg/testutils/skip/skip.go @@ -114,7 +114,7 @@ func UnderShort(t SkippableTest, args ...interface{}) { // UnderStress skips this test when running under stress. func UnderStress(t SkippableTest, args ...interface{}) { t.Helper() - if NightlyStress() { + if Stress() { t.Skip(append([]interface{}{"disabled under stress"}, args...)) } } @@ -123,7 +123,7 @@ func UnderStress(t SkippableTest, args ...interface{}) { // run under stress with the -race flag. func UnderStressRace(t SkippableTest, args ...interface{}) { t.Helper() - if NightlyStress() && util.RaceEnabled { + if Stress() && util.RaceEnabled { t.Skip(append([]interface{}{"disabled under stressrace"}, args...)) } } diff --git a/pkg/testutils/skip/stress.go b/pkg/testutils/skip/stress.go index 668d3efde668..4368746ca431 100644 --- a/pkg/testutils/skip/stress.go +++ b/pkg/testutils/skip/stress.go @@ -12,10 +12,18 @@ package skip import "github.com/cockroachdb/cockroach/pkg/util/envutil" -var stress = envutil.EnvOrDefaultBool("COCKROACH_NIGHTLY_STRESS", false) +var nightlyStress = envutil.EnvOrDefaultBool("COCKROACH_NIGHTLY_STRESS", false) + +var stress = envutil.EnvOrDefaultBool("COCKROACH_STRESS", false) // NightlyStress returns true iff the process is running as part of CockroachDB's // nightly stress tests. func NightlyStress() bool { - return stress + return nightlyStress +} + +// Stress returns true iff the process is running under any instance of the stress +// harness, including the nightly one. +func Stress() bool { + return stress || nightlyStress } From 265667cab6fa3c98548d58f4f87521e4d53b7541 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 17 Jul 2022 00:22:15 +0200 Subject: [PATCH 2/6] cli,sdnotify: improve the handling of unix socket directories tldr: This patch makes it easier for users to understand when CockroachDB has a problem using/creating unix sockets, and what they can do to avert the situation. Example, before; ``` $ cockroach start-single-node --insecure --background ... listen unixgram /data/home/kena/cockroach/this-is-a-very-long-directory-name-the-point-is-to-be-more-than-one-hundred-and-twenty-three-characters/sdnotify2139543236/notify.sock: bind: invalid argument ``` Example, after: ``` $ cockroach start-single-node --insecure --background ERROR: no suitable directory found for the --background notify socket HINT: Avoid using --background altogether (preferred), or use a shorter directory name for --socket-dir, TMPDIR or the current directory. ``` **Context:** CockroachDB uses unix sockets both to handle `--background` and to create a postgres-compatible SQL client socket via `--socket-dir`. For `--background`, the logic was previously hardwired to always use `os.TempDir()`, i.e. either `$TMPDIR` or `/tmp`. On Unix, socket objects are commonly limited to 128 bytes; that's e.g. 104 path characters including final NUL on BSD, 108 on glibc. When the actual length is larger, creating the socket fails with a confusing error `bind: invalid argument`. **The problem:** Users have reported issues in the following circumstances: - when setting their CWD/$TMPDIR to a long directory name, they expected `--socket-dir` to also influence the socket name used for `--background`. Instead, `--background` failed with a hard-to-understand error. - when running tests inside a container/sandbox with a long prefix, they were confused that `--background` / `--socket-dir=.` did not work properly. - they use macOS which has very large directory names in $TMPDIR. To improve user experience, this patch changes the behavior as described in the release notes below below. Release note (cli change): CockroachDB now produces a clearer error when the path specified via `--socket-dir` is too long. Release note (cli change): When the flag `--background` is specified, CockroachDB now makes 3 attempts to find a suitable directory to create the notification socket: the value of `--socket-dir` if specified, the value of `$TMPDIR` (or /tmp if the env var is empty), and the current working directory. If none of these directories has a name short enough, an explanatory error is printed. --- pkg/cli/flags.go | 12 +++- .../interactive_tests/test_socket_name.tcl | 65 +++++++++++++++++++ pkg/cli/start_unix.go | 65 ++++++++++++++++++- pkg/util/sdnotify/sdnotify.go | 4 +- pkg/util/sdnotify/sdnotify_unix.go | 8 +-- pkg/util/sdnotify/sdnotify_unix_test.go | 3 +- pkg/util/sdnotify/sdnotify_windows.go | 2 +- 7 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 pkg/cli/interactive_tests/test_socket_name.tcl diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index e2b9c59dfde9..224f8c915b9a 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -1009,7 +1009,17 @@ func extraServerFlagInit(cmd *cobra.Command) error { if serverSocketDir == "" { serverCfg.SocketFile = "" } else { - serverCfg.SocketFile = filepath.Join(serverSocketDir, ".s.PGSQL."+serverListenPort) + socketName := ".s.PGSQL." + serverListenPort + // On BSD, binding to a socket is limited to a path length of 104 characters + // (including the NUL terminator). In glibc, this limit is 108 characters. + // Otherwise, the bind operation fails with "invalid parameter". + if len(serverSocketDir) >= 104-1-len(socketName) { + return errors.WithHintf( + errors.Newf("value of --%s is too long: %s", cliflags.SocketDir.Name, serverSocketDir), + "The socket directory name must be shorter than %d characters.", + 104-1-len(socketName)) + } + serverCfg.SocketFile = filepath.Join(serverSocketDir, socketName) } } diff --git a/pkg/cli/interactive_tests/test_socket_name.tcl b/pkg/cli/interactive_tests/test_socket_name.tcl new file mode 100644 index 000000000000..3f02a42997fe --- /dev/null +++ b/pkg/cli/interactive_tests/test_socket_name.tcl @@ -0,0 +1,65 @@ +#! /usr/bin/env expect -f +# +source [file join [file dirname $argv0] common.tcl] + +set longname "this-is-a-very-long-directory-name-the-point-is-to-be-more-than-one-hundred-and-twenty-three-characters/and-we-also-need-to-break-it-into-two-parts" + +spawn /bin/bash +send "PS1=':''/# '\r" +eexpect ":/# " + +send "mkdir -p $longname\r" +eexpect ":/# " + +start_test "Check that the socket-dir flag checks the length of the directory." +send "$argv start-single-node --insecure --socket-dir=$longname\r" +eexpect "value of --socket-dir is too long" +eexpect "socket directory name must be shorter" +eexpect ":/# " +end_test + +set crdb [file normalize $argv] +send "export BASEDIR=\$PWD\r" +eexpect ":/# " +send "export PREVTMP=\$TMPDIR\r" +eexpect ":/# " + +start_test "Check that --background complains about the directory name if there is no default." +send "cd $longname\r" +eexpect ":/# " +send "export TMPDIR=\$PWD\r" +eexpect ":/# " +send "$crdb start-single-node --insecure --background\r" +eexpect "no suitable directory found for the --background notify socket" +eexpect "use a shorter directory name" +eexpect ":/# " +end_test + +start_test "Check that --background can use --socket-name if specified and set to sane default." +send "$crdb start-single-node --insecure --background --socket-dir=\$BASEDIR --pid-file=\$BASEDIR/server_pid\r" +eexpect ":/# " +# check the server is running. +system "$crdb sql --insecure -e 'select 1'" +stop_server $crdb +end_test + +start_test "Check that --background can use TMPDIR if specified and set to sane default." +# NB: we use a single-command override of TMPDIR (as opposed to using 'export') so that +# the following test below can reuse the value set above. +send "TMPDIR=\$PREVTMP $crdb start-single-node --insecure --background --pid-file=\$BASEDIR/server_pid\r" +eexpect ":/# " +# check the server is running. +system "$crdb sql --insecure -e 'select 1'" +stop_server $crdb +end_test + +start_test "Check that --background can use cwd if TMPDIR is invalid." +# NB: at this point TMPDIR is still long, as per previous test. +send "cd \$BASEDIR\r" +eexpect ":/# " +send "$crdb start-single-node --insecure --background --pid-file=server_pid\r" +eexpect ":/# " +# check the server is running. +system "$crdb sql --insecure -e 'select 1'" +stop_server $crdb +end_test diff --git a/pkg/cli/start_unix.go b/pkg/cli/start_unix.go index 73e856cc6479..d81e27a7ce29 100644 --- a/pkg/cli/start_unix.go +++ b/pkg/cli/start_unix.go @@ -20,9 +20,11 @@ import ( "os/signal" "strings" + "github.com/cockroachdb/cockroach/pkg/cli/cliflags" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/sdnotify" "github.com/cockroachdb/cockroach/pkg/util/sysutil" + "github.com/cockroachdb/errors" "golang.org/x/sys/unix" ) @@ -67,8 +69,69 @@ func handleSignalDuringShutdown(sig os.Signal) { const backgroundFlagDefined = true +// findGoodNotifyDir determines a good target directory +// to create the unix socket used to signal successful +// background startup (via sdnotify). +// A directory is "good" if it seems writable and its +// name is short enough. +func findGoodNotifyDir() (string, error) { + goodEnough := func(s string) bool { + if len(s) >= 104-1-len("sdnotify/notify.sock")-10 { + // On BSD, binding to a socket is limited to a path length of 104 characters + // (including the NUL terminator). In glibc, this limit is 108 characters. + // macOS also has a tendency to produce very long temporary directory names. + return false + } + st, err := os.Stat(s) + if err != nil { + return false + } + if !st.IsDir() || st.Mode()&0222 == 0 /* any write bits? */ { + // Note: we're confident the directory is unsuitable if none of the + // write bits are set, however there could be a false positive + // if some write bits are set. + // + // For example, if the process runs as a UID that does not match + // the directory owner UID or GID, and the write mode is 0220 or + // less, the sd socket creation will still fail. + // As such, the mode check here is merely a heuristic. We're + // OK with that: the actual write failure will produce a clear + // error message. + return false + } + return true + } + + // Was --socket-dir configured? Try to use that. + if serverSocketDir != "" && goodEnough(serverSocketDir) { + return serverSocketDir, nil + } + // Do we have a temp directory? Try to use that. + if tmpDir := os.TempDir(); goodEnough(tmpDir) { + return tmpDir, nil + } + // Can we perhaps use the current directory? + if cwd, err := os.Getwd(); err == nil && goodEnough(cwd) { + return cwd, nil + } + + // Note: we do not attempt to use the configured on-disk store + // directory(ies), because they may point to a filesystem that does + // not support unix sockets. + + return "", errors.WithHintf( + errors.Newf("no suitable directory found for the --background notify socket"), + "Avoid using --%s altogether (preferred), or use a shorter directory name "+ + "for --socket-dir, TMPDIR or the current directory.", cliflags.Background.Name) +} + func maybeRerunBackground() (bool, error) { if startBackground { + notifyDir, err := findGoodNotifyDir() + if err != nil { + return true, err + } + args := make([]string, 0, len(os.Args)) foundBackground := false for _, arg := range os.Args { @@ -88,7 +151,7 @@ func maybeRerunBackground() (bool, error) { // Notify to ourselves that we're restarting. _ = os.Setenv(backgroundEnvVar, "1") - return true, sdnotify.Exec(cmd) + return true, sdnotify.Exec(cmd, notifyDir) } return false, nil } diff --git a/pkg/util/sdnotify/sdnotify.go b/pkg/util/sdnotify/sdnotify.go index c8295e301f0f..244fe988a282 100644 --- a/pkg/util/sdnotify/sdnotify.go +++ b/pkg/util/sdnotify/sdnotify.go @@ -29,6 +29,6 @@ func Ready() error { // either exited or signaled that it is ready. If the command exits // with a non-zero status before signaling readiness, returns an // exec.ExitError. -func Exec(cmd *exec.Cmd) error { - return bgExec(cmd) +func Exec(cmd *exec.Cmd, socketDir string) error { + return bgExec(cmd, socketDir) } diff --git a/pkg/util/sdnotify/sdnotify_unix.go b/pkg/util/sdnotify/sdnotify_unix.go index c4a5a04a8675..a2a3e77e41bf 100644 --- a/pkg/util/sdnotify/sdnotify_unix.go +++ b/pkg/util/sdnotify/sdnotify_unix.go @@ -55,8 +55,8 @@ func notify(path, msg string) error { return err } -func bgExec(cmd *exec.Cmd) error { - l, err := listen() +func bgExec(cmd *exec.Cmd, socketDir string) error { + l, err := listen(socketDir) if err != nil { return err } @@ -97,8 +97,8 @@ type listener struct { conn *net.UnixConn } -func listen() (listener, error) { - dir, err := ioutil.TempDir("", "sdnotify") +func listen(socketDir string) (listener, error) { + dir, err := ioutil.TempDir(socketDir, "sdnotify") if err != nil { return listener{}, err } diff --git a/pkg/util/sdnotify/sdnotify_unix_test.go b/pkg/util/sdnotify/sdnotify_unix_test.go index 595913114657..f219c2b65a23 100644 --- a/pkg/util/sdnotify/sdnotify_unix_test.go +++ b/pkg/util/sdnotify/sdnotify_unix_test.go @@ -14,13 +14,14 @@ package sdnotify import ( + "os" "testing" _ "github.com/cockroachdb/cockroach/pkg/util/log" // for flags ) func TestSDNotify(t *testing.T) { - l, err := listen() + l, err := listen(os.TempDir()) if err != nil { t.Fatal(err) } diff --git a/pkg/util/sdnotify/sdnotify_windows.go b/pkg/util/sdnotify/sdnotify_windows.go index 54c764e33a26..3f3c1f4afc23 100644 --- a/pkg/util/sdnotify/sdnotify_windows.go +++ b/pkg/util/sdnotify/sdnotify_windows.go @@ -20,6 +20,6 @@ func ready() error { return nil } -func bgExec(*exec.Cmd) error { +func bgExec(*exec.Cmd, string) error { return errors.New("not implemented") } From b7af1849c695c5aea938ca6a105310f1a72f5872 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 17 Jul 2022 00:34:31 +0200 Subject: [PATCH 3/6] cli/start: remove noise when shutting down after `--backround` Release note (bug fix): The server process does not any more announce that it is shutting down on stdout when running with `--background`. --- pkg/cli/start.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/cli/start.go b/pkg/cli/start.go index cbb1aac8dfe0..ba2efbce5b16 100644 --- a/pkg/cli/start.go +++ b/pkg/cli/start.go @@ -758,8 +758,10 @@ func waitForShutdown( // INFO because a single interrupt is rather innocuous. severity.INFO, ) - msgDouble := "Note: a second interrupt will skip graceful shutdown and terminate forcefully" - fmt.Fprintln(os.Stdout, msgDouble) + if !startCtx.inBackground { + msgDouble := "Note: a second interrupt will skip graceful shutdown and terminate forcefully" + fmt.Fprintln(os.Stdout, msgDouble) + } } // Start the draining process in a separate goroutine so that it @@ -843,7 +845,9 @@ func waitForShutdown( const msgDrain = "initiating graceful shutdown of server" log.Ops.Info(shutdownCtx, msgDrain) - fmt.Fprintln(os.Stdout, msgDrain) + if !startCtx.inBackground { + fmt.Fprintln(os.Stdout, msgDrain) + } // Notify the user every 5 second of the shutdown progress. go func() { @@ -895,12 +899,16 @@ func waitForShutdown( case <-stopper.IsStopped(): const msgDone = "server drained and shutdown completed" log.Ops.Infof(shutdownCtx, msgDone) - fmt.Fprintln(os.Stdout, msgDone) + if !startCtx.inBackground { + fmt.Fprintln(os.Stdout, msgDone) + } case <-stopWithoutDrain: const msgDone = "too early to drain; used hard shutdown instead" log.Ops.Infof(shutdownCtx, msgDone) - fmt.Fprintln(os.Stdout, msgDone) + if !startCtx.inBackground { + fmt.Fprintln(os.Stdout, msgDone) + } } break } From 82d92c5617b5069731263b68779850c1e31d1492 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 17 Jul 2022 01:01:25 +0200 Subject: [PATCH 4/6] pgwire,sdnotify: shortcut to /tmp in tests when necessary This commit changes `TestAuthenticationAndHBARules` (pgwire) and `TestSDNotify` (sdnotify) to use `/tmp` as socket directory if the default temporary directory name is too long. Release note: None --- pkg/sql/pgwire/auth_test.go | 16 ++++++++++------ pkg/util/sdnotify/BUILD.bazel | 13 +++++++++++++ pkg/util/sdnotify/sdnotify_unix_test.go | 18 +++++++++++++++--- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/pkg/sql/pgwire/auth_test.go b/pkg/sql/pgwire/auth_test.go index dbca836e43cd..111e19839dcb 100644 --- a/pkg/sql/pgwire/auth_test.go +++ b/pkg/sql/pgwire/auth_test.go @@ -48,6 +48,7 @@ import ( "github.com/cockroachdb/errors/stdstrings" "github.com/cockroachdb/redact" "github.com/lib/pq" + "github.com/stretchr/testify/require" ) // TestAuthenticationAndHBARules exercises the authentication code @@ -138,6 +139,7 @@ func makeSocketFile(t *testing.T) (socketDir, socketFile string, cleanupFn func( // Unix sockets not supported on windows. return "", "", func() {} } + socketName := ".s.PGSQL." + socketConnVirtualPort // We need a temp directory in which we'll create the unix socket. // // On BSD, binding to a socket is limited to a path length of 104 characters @@ -145,17 +147,19 @@ func makeSocketFile(t *testing.T) (socketDir, socketFile string, cleanupFn func( // // macOS has a tendency to produce very long temporary directory names, so // we are careful to keep all the constants involved short. - baseTmpDir := "" - if runtime.GOOS == "darwin" || strings.Contains(runtime.GOOS, "bsd") { + baseTmpDir := os.TempDir() + if len(baseTmpDir) >= 104-1-len(socketName)-1-len("TestAuth")-10 { + t.Logf("temp dir name too long: %s", baseTmpDir) + t.Logf("using /tmp instead.") + // Note: /tmp might fail in some systems, that's why we still prefer + // os.TempDir() if available. baseTmpDir = "/tmp" } tempDir, err := ioutil.TempDir(baseTmpDir, "TestAuth") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // ".s.PGSQL.NNNN" is the standard unix socket name supported by pg clients. return tempDir, - filepath.Join(tempDir, ".s.PGSQL."+socketConnVirtualPort), + filepath.Join(tempDir, socketName), func() { _ = os.RemoveAll(tempDir) } } diff --git a/pkg/util/sdnotify/BUILD.bazel b/pkg/util/sdnotify/BUILD.bazel index a776becebd87..a9d9ca50c9e7 100644 --- a/pkg/util/sdnotify/BUILD.bazel +++ b/pkg/util/sdnotify/BUILD.bazel @@ -26,42 +26,55 @@ go_test( deps = select({ "@io_bazel_rules_go//go/platform:aix": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:android": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:darwin": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:dragonfly": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:freebsd": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:illumos": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:ios": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:js": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:netbsd": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:openbsd": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:plan9": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:solaris": [ "//pkg/util/log", + "@com_github_stretchr_testify//require", ], "//conditions:default": [], }), diff --git a/pkg/util/sdnotify/sdnotify_unix_test.go b/pkg/util/sdnotify/sdnotify_unix_test.go index f219c2b65a23..7b4063bdead9 100644 --- a/pkg/util/sdnotify/sdnotify_unix_test.go +++ b/pkg/util/sdnotify/sdnotify_unix_test.go @@ -18,13 +18,25 @@ import ( "testing" _ "github.com/cockroachdb/cockroach/pkg/util/log" // for flags + "github.com/stretchr/testify/require" ) func TestSDNotify(t *testing.T) { - l, err := listen(os.TempDir()) - if err != nil { - t.Fatal(err) + tmpDir := os.TempDir() + // On BSD, binding to a socket is limited to a path length of 104 characters + // (including the NUL terminator). In glibc, this limit is 108 characters. + // macOS also has a tendency to produce very long temporary directory names. + if len(tmpDir) >= 104-1-len("sdnotify/notify.sock")-10 { + // Perhaps running inside a sandbox? + t.Logf("default temp dir name is too long: %s", tmpDir) + t.Logf("forcing use of /tmp instead") + // Note: Using /tmp may fail on some systems; this is why we + // prefer os.TempDir() by default. + tmpDir = "/tmp" } + + l, err := listen(tmpDir) + require.NoError(t, err) defer func() { _ = l.close() }() ch := make(chan error) From 7377c2bd181bad5738de256ec4eaa1c1be445a2f Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Mon, 18 Jul 2022 14:57:29 -0400 Subject: [PATCH 5/6] cluster-ui: allow dynamic timescale window from key buttons fixes #77007 Previously, controlling the time scale window using the arrow key buttons and 'Now' button would disable the dynamically moving window even when the end time = now. This commit makes it so that when the time window is changed to include the current time (i.e. endTime = now), the time window will be changed to dynamic to ensure the window is keep up to date with the current time. This commit also fixes the object construction of the time scale from session storage and corrects the 'Now' button tooltip label from 'past 1 day' to 'Most recent interval', with the other labels in the control panel ('next time interval' and 'previous time interval' being capitalized. Release note (bug fix): changing the time window using arrow buttons and 'Now' button will now properly turn the timeframe into a moving window when endTime = now. --- .../timeScaleDropdown/timeFrameControls.tsx | 6 ++-- .../timeScaleDropdown/timeScaleDropdown.tsx | 12 +++++--- .../cluster-ui/src/timeScaleDropdown/utils.ts | 1 - .../db-console/src/redux/globalTimeScale.ts | 27 ++++++++++++++--- .../src/redux/statements/statementsActions.ts | 2 +- .../db-console/src/redux/timeScale.spec.ts | 4 +-- .../db-console/src/redux/timeScale.ts | 30 +------------------ .../cluster/containers/nodeGraphs/index.tsx | 7 +++-- .../containers/raftMessages/index.tsx | 7 ++--- .../reports/containers/customChart/index.tsx | 12 ++++---- .../shared/components/metricQuery/index.tsx | 4 +-- 11 files changed, 53 insertions(+), 59 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeFrameControls.tsx b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeFrameControls.tsx index d3c67bb85a4b..224d265c702b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeFrameControls.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeFrameControls.tsx @@ -47,7 +47,7 @@ export const TimeFrameControls = ({ @@ -62,7 +62,7 @@ export const TimeFrameControls = ({ @@ -78,7 +78,7 @@ export const TimeFrameControls = ({ diff --git a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx index 8db97d5f8c38..6ae0a4031a24 100644 --- a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx @@ -163,7 +163,10 @@ export const TimeScaleDropdown: React.FC = ({ let selected = {}; let key = currentScale.key; let endTime = moment.utc(currentWindow.end); + // Dynamic moving window should be off unless the window extends to the current time. + let isMoving = false; + const now = moment.utc(); switch (direction) { case ArrowDirection.RIGHT: endTime = endTime.add(seconds, "seconds"); @@ -173,7 +176,8 @@ export const TimeScaleDropdown: React.FC = ({ break; case ArrowDirection.CENTER: // CENTER is used to set the time window to the current time. - endTime = moment.utc(); + endTime = now; + isMoving = true; break; default: console.error("Unknown direction: ", direction); @@ -181,9 +185,8 @@ export const TimeScaleDropdown: React.FC = ({ // If the timescale extends into the future then fallback to a default // timescale. Otherwise set the key to "Custom" so it appears correctly. - // The first `!endTime` part of the if clause seems unnecessary since endTime is always a specific time. // If endTime + windowValid > now. Unclear why this uses windowValid instead of windowSize. - if (!endTime || endTime > moment.utc().subtract(currentScale.windowValid)) { + if (endTime.isSameOrAfter(now.subtract(currentScale.windowValid))) { const foundTimeScale = Object.entries(options).find( // eslint-disable-next-line @typescript-eslint/no-unused-vars ([_, value]) => value.windowSize.asSeconds() === windowSize.asSeconds(), @@ -197,6 +200,7 @@ export const TimeScaleDropdown: React.FC = ({ * not disabled, but the clause doesn't seem to be true. */ selected = { key: foundTimeScale[0], ...foundTimeScale[1] }; + isMoving = true; } else { // This code might not be possible to hit, due to the right arrow being disabled key = "Custom"; @@ -207,7 +211,7 @@ export const TimeScaleDropdown: React.FC = ({ let timeScale: TimeScale = { ...currentScale, - fixedWindowEnd: endTime, + fixedWindowEnd: isMoving ? false : endTime, windowSize, key, ...selected, diff --git a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts index e90fa706f0c5..04c47c6286e0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts @@ -11,7 +11,6 @@ import moment from "moment"; import { TimeScale, TimeScaleOption, TimeScaleOptions } from "./timeScaleTypes"; import { dateFormat, timeFormat } from "./timeScaleDropdown"; -import React from "react"; /** * timeScale1hMinOptions is a preconfigured set of time scales with 1h minimum that can be diff --git a/pkg/ui/workspaces/db-console/src/redux/globalTimeScale.ts b/pkg/ui/workspaces/db-console/src/redux/globalTimeScale.ts index 871dc90c11d1..7b557e8b5e2d 100644 --- a/pkg/ui/workspaces/db-console/src/redux/globalTimeScale.ts +++ b/pkg/ui/workspaces/db-console/src/redux/globalTimeScale.ts @@ -8,13 +8,32 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +import moment from "moment"; import { LocalSetting } from "./localsettings"; import { AdminUIState } from "./state"; import { TimeScale, defaultTimeScaleSelected } from "@cockroachlabs/cluster-ui"; const localSettingsSelector = (state: AdminUIState) => state.localSettings; -export const globalTimeScaleLocalSetting = new LocalSetting< - AdminUIState, - TimeScale ->("timeScale/SQLActivity", localSettingsSelector, defaultTimeScaleSelected); +const setting = new LocalSetting( + "timeScale/SQLActivity", + localSettingsSelector, + defaultTimeScaleSelected, +); + +export const globalTimeScaleLocalSetting = { + ...setting, + selector: (state: AdminUIState): TimeScale => { + const val = setting.selector(state); + if (typeof val.windowSize !== "string") { + return val; + } + return { + key: val.key, + windowSize: val.windowSize && moment.duration(val.windowSize), + windowValid: val.windowValid && moment.duration(val.windowValid), + sampleSize: val.sampleSize && moment.duration(val.sampleSize), + fixedWindowEnd: val.fixedWindowEnd, + }; + }, +}; diff --git a/pkg/ui/workspaces/db-console/src/redux/statements/statementsActions.ts b/pkg/ui/workspaces/db-console/src/redux/statements/statementsActions.ts index bc3cc69c21cf..e7e9c4289dbe 100644 --- a/pkg/ui/workspaces/db-console/src/redux/statements/statementsActions.ts +++ b/pkg/ui/workspaces/db-console/src/redux/statements/statementsActions.ts @@ -12,7 +12,7 @@ import { Action } from "redux"; import { PayloadAction } from "src/interfaces/action"; import { google } from "@cockroachlabs/crdb-protobuf-client"; import IDuration = google.protobuf.IDuration; -import { TimeScale } from "src/redux/timeScale"; +import { TimeScale } from "@cockroachlabs/cluster-ui"; export const CREATE_STATEMENT_DIAGNOSTICS_REPORT = "cockroachui/statements/CREATE_STATEMENT_DIAGNOSTICS_REPORT"; diff --git a/pkg/ui/workspaces/db-console/src/redux/timeScale.spec.ts b/pkg/ui/workspaces/db-console/src/redux/timeScale.spec.ts index e5845eace2fa..289a3bb5a22d 100644 --- a/pkg/ui/workspaces/db-console/src/redux/timeScale.spec.ts +++ b/pkg/ui/workspaces/db-console/src/redux/timeScale.spec.ts @@ -8,7 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import { defaultTimeScaleOptions } from "@cockroachlabs/cluster-ui"; +import { defaultTimeScaleOptions, TimeScale } from "@cockroachlabs/cluster-ui"; import * as timeScale from "./timeScale"; import moment from "moment"; @@ -30,7 +30,7 @@ describe("time scale reducer", function () { }); it("should create the correct SET_SCALE action to set time window settings", function () { - const payload: timeScale.TimeScale = { + const payload: TimeScale = { windowSize: moment.duration(10, "s"), windowValid: moment.duration(10, "s"), sampleSize: moment.duration(10, "s"), diff --git a/pkg/ui/workspaces/db-console/src/redux/timeScale.ts b/pkg/ui/workspaces/db-console/src/redux/timeScale.ts index f90c2acdd6e4..d5ef68250d2d 100644 --- a/pkg/ui/workspaces/db-console/src/redux/timeScale.ts +++ b/pkg/ui/workspaces/db-console/src/redux/timeScale.ts @@ -16,7 +16,7 @@ import { Action } from "redux"; import { PayloadAction } from "src/interfaces/action"; import _ from "lodash"; -import { defaultTimeScaleOptions } from "@cockroachlabs/cluster-ui"; +import { defaultTimeScaleOptions, TimeScale } from "@cockroachlabs/cluster-ui"; import moment from "moment"; export const SET_SCALE = "cockroachui/timewindow/SET_SCALE"; @@ -34,34 +34,6 @@ export interface TimeWindow { end: moment.Moment; } -/** - * TimeScale describes the requested dimensions of TimeWindows; it - * prescribes a length for the window, along with a period of time that a - * newly created TimeWindow will remain valid. - */ -export interface TimeScale { - /** - * The key used to index in to the defaultTimeScaleOptions collection. - * The key is "Custom" when specifying a custom time that is not one of the default options - */ - key?: string; - // The size of a global time window. Default is ten minutes. - windowSize: moment.Duration; - // The length of time the global time window is valid. The current time window - // is invalid if now > (metricsTime.currentWindow.end + windowValid). Default is ten - // seconds. If fixedWindowEnd is set this is ignored. - windowValid?: moment.Duration; - // The expected duration of individual samples for queries at this time scale. - sampleSize: moment.Duration; - /** - * The fixed end time of the window, or false if it should be a dynamically moving "now". - * Typically, when the `key` property is a default option, `fixedWindowEnd` should be false. - * And when the `key` property is "Custom" `fixedWindowEnd` should be a specific Moment. - * It is unclear if there are legitimate reasons for the two being out of sync. - */ - fixedWindowEnd: moment.Moment | false; -} - export class TimeScaleState { // Currently selected scale. scale: TimeScale; diff --git a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx index 361936c232a8..250c1e37835e 100644 --- a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx @@ -66,11 +66,14 @@ import { PayloadAction } from "src/interfaces/action"; import { setMetricsFixedWindow, TimeWindow, - TimeScale, adjustTimeScale, } from "src/redux/timeScale"; import { InlineAlert } from "src/components"; -import { Anchor, TimeScaleDropdown } from "@cockroachlabs/cluster-ui"; +import { + Anchor, + TimeScaleDropdown, + TimeScale, +} from "@cockroachlabs/cluster-ui"; import { reduceStorageOfTimeSeriesDataOperationalFlags } from "src/util/docs"; import moment from "moment"; import { diff --git a/pkg/ui/workspaces/db-console/src/views/devtools/containers/raftMessages/index.tsx b/pkg/ui/workspaces/db-console/src/views/devtools/containers/raftMessages/index.tsx index 58cac83382ab..9a5e0d3a6098 100644 --- a/pkg/ui/workspaces/db-console/src/views/devtools/containers/raftMessages/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/devtools/containers/raftMessages/index.tsx @@ -39,11 +39,8 @@ import { MetricsDataProvider } from "src/views/shared/containers/metricDataProvi import messagesDashboard from "./messages"; import { getMatchParamByName } from "src/util/query"; import { PayloadAction } from "src/interfaces/action"; -import { - TimeWindow, - TimeScale, - setMetricsFixedWindow, -} from "src/redux/timeScale"; +import { TimeWindow, setMetricsFixedWindow } from "src/redux/timeScale"; +import { TimeScale } from "@cockroachlabs/cluster-ui"; interface NodeGraphsOwnProps { refreshNodes: typeof refreshNodes; diff --git a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx index 81412da97f71..3d98b8ac3e5c 100644 --- a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx @@ -22,7 +22,11 @@ import { LineGraph } from "src/views/cluster/components/linegraph"; import { DropdownOption } from "src/views/shared/components/dropdown"; import { MetricsDataProvider } from "src/views/shared/containers/metricDataProvider"; import { Metric, Axis } from "src/views/shared/components/metricQuery"; -import { AxisUnits, TimeScaleDropdown } from "@cockroachlabs/cluster-ui"; +import { + AxisUnits, + TimeScale, + TimeScaleDropdown, +} from "@cockroachlabs/cluster-ui"; import { PageConfig, PageConfigItem, @@ -37,11 +41,7 @@ import { CustomChartState, CustomChartTable } from "./customMetric"; import "./customChart.styl"; import { queryByName } from "src/util/query"; import { PayloadAction } from "src/interfaces/action"; -import { - TimeWindow, - TimeScale, - setMetricsFixedWindow, -} from "src/redux/timeScale"; +import { TimeWindow, setMetricsFixedWindow } from "src/redux/timeScale"; import { setGlobalTimeScaleAction } from "src/redux/statements"; import { globalTimeScaleLocalSetting } from "src/redux/globalTimeScale"; diff --git a/pkg/ui/workspaces/db-console/src/views/shared/components/metricQuery/index.tsx b/pkg/ui/workspaces/db-console/src/views/shared/components/metricQuery/index.tsx index 5b2fdafeea22..03ac6b5c7c4e 100644 --- a/pkg/ui/workspaces/db-console/src/views/shared/components/metricQuery/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/shared/components/metricQuery/index.tsx @@ -37,9 +37,9 @@ import TimeSeriesQueryAggregator = protos.cockroach.ts.tspb.TimeSeriesQueryAggre import TimeSeriesQueryDerivative = protos.cockroach.ts.tspb.TimeSeriesQueryDerivative; import Long from "long"; import { History } from "history"; -import { TimeWindow, TimeScale } from "src/redux/timeScale"; +import { TimeWindow } from "src/redux/timeScale"; import { PayloadAction } from "src/interfaces/action"; -import { AxisUnits } from "@cockroachlabs/cluster-ui"; +import { AxisUnits, TimeScale } from "@cockroachlabs/cluster-ui"; /** * AxisProps represents the properties of an Axis being specified as part of a From 024b27acde05ff94fb94322b0643e3d64cb108fa Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Tue, 19 Jul 2022 13:49:33 -0500 Subject: [PATCH 6/6] ci: only build `roachprod` on Unix This fails to build on Windows which was breaking CI. Release note: None --- build/teamcity/cockroach/ci/builds/build_impl.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/build/teamcity/cockroach/ci/builds/build_impl.sh b/build/teamcity/cockroach/ci/builds/build_impl.sh index c7e940fb4b32..98d414f4f667 100755 --- a/build/teamcity/cockroach/ci/builds/build_impl.sh +++ b/build/teamcity/cockroach/ci/builds/build_impl.sh @@ -10,8 +10,9 @@ fi CONFIG="$1" -# Extra targets to build on Linux x86_64 only. EXTRA_TARGETS= + +# Extra targets to build on Linux x86_64 only. if [ "$CONFIG" == "crosslinux" ] then DOC_TARGETS=$(grep '^//' docs/generated/bazel_targets.txt) @@ -20,10 +21,15 @@ then EXTRA_TARGETS="$DOC_TARGETS $GO_TARGETS $BINARY_TARGETS" fi +# Extra targets to build on Unix only. +if [ "$CONFIG" != "crosswindows" ] +then + EXTRA_TARGETS="$EXTRA_TARGETS //pkg/cmd/roachprod" +fi + bazel build //pkg/cmd/bazci --config=ci $(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci --compilation_mode opt \ --config "$CONFIG" --config ci --config with_ui \ build //pkg/cmd/cockroach-short //pkg/cmd/cockroach \ //pkg/cmd/cockroach-sql \ - //pkg/cmd/roachprod \ //pkg/cmd/cockroach-oss //c-deps:libgeos $EXTRA_TARGETS