From 65d03ab8c153154eabe5c2b7d107f3cc3b4e1381 Mon Sep 17 00:00:00 2001 From: Andrii Vorobiov Date: Mon, 14 Jun 2021 11:58:21 +0300 Subject: [PATCH 1/4] ui: disallow imports from cluster-ui sources Db Console depends on local `cluster-ui` package that and it was possible to specify imports to its modules as a paths to source modules or use exported `index` file which is an entry point for bundled module. `cluster-ui` package has to be built before imports in Db Console because it has its own dependencies and build process which isn't compatible with build process of Db Console. To prevent incorrect imports, this change adds es lint rule to prohibit any imports from `@cockroachlabs/cluster-ui/src/*` path. Release note: none --- pkg/ui/.eslintrc.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/ui/.eslintrc.json b/pkg/ui/.eslintrc.json index 9949741a0888..9a28b421df56 100644 --- a/pkg/ui/.eslintrc.json +++ b/pkg/ui/.eslintrc.json @@ -8,6 +8,9 @@ "react/no-unescaped-entities": "off", "@typescript-eslint/no-unused-vars": "off", "@typescript-eslint/explicit-module-boundary-types": "off", - "@typescript-eslint/no-explicit-any": "off" + "@typescript-eslint/no-explicit-any": "off", + "no-restricted-imports": ["error", { + "patterns": ["@cockroachlabs/cluster-ui/src/*"] + }] } } From 8aae3c9939d0f051e8444c1bc6099aae1be74015 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 16 Jun 2021 14:47:12 +0000 Subject: [PATCH 2/4] backupccl: add setting to write files in SQL This adds a setting -- default off -- to force BACKUP to always ask KV to return files to SQL to write instead of writing them directly. This is currently what it does for tenants but not for the system tenant due to the extra network hop, data copy, cpu and mem overhead. Release note: none. --- pkg/ccl/backupccl/backup_processor.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/ccl/backupccl/backup_processor.go b/pkg/ccl/backupccl/backup_processor.go index 0eff31c7901f..10cfe5df6ced 100644 --- a/pkg/ccl/backupccl/backup_processor.go +++ b/pkg/ccl/backupccl/backup_processor.go @@ -63,6 +63,12 @@ var ( time.Minute*5, settings.NonNegativeDuration, ) + alwaysWriteInProc = settings.RegisterBoolSetting( + "bulkio.backup.proxy_file_writes.enabled", + "return files to the backup coordination processes to write to "+ + "external storage instead of writing them directly from the storage layer", + false, + ) ) const backupProcessorName = "backupDataProcessor" @@ -201,7 +207,7 @@ func runBackupProcessor( exportRequestStoreByLocalityKV := storageConfByLocalityKV // If this is a tenant backup, we need to write the file from the SQL layer. - writeSSTsInProcessor := !flowCtx.Cfg.Codec.ForSystemTenant() + writeSSTsInProcessor := !flowCtx.Cfg.Codec.ForSystemTenant() || alwaysWriteInProc.Get(&clusterSettings.SV) var defaultStore cloud.ExternalStorage if writeSSTsInProcessor { From b0c459d45d84b462fdf55885d4d70cfbc7f59473 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Wed, 16 Jun 2021 16:31:41 +0200 Subject: [PATCH 3/4] roachprod: avoid extra dead event in roachprod monitor Prior to this commit, we'd get an extra dead event when restarting a node: ``` // initially running 1: 5762 // issue roachprod stop 1: dead (exit status 137) // issue roachprod start 1: dead (exit status 0) 1: 6254 ``` This would in turn upset roachtest's `monitor`, as it keeps track of the number of expected death events, and cause bogus test failures. This was introduced in #66414 and is fixed in this commit. As a small extra fix, if we don't observe the exit status of the stopped systemd unit (i.e. if the process cycles rapidly), we now correctly print that we don't know the exit status, where previously we would print the "exit" status of the new running incarnation, i.e. likely zero. I tested this manually by exacerbating the sleeps in the shell snippet: ``` 1: 10885 1: dead (exit status unknown) 1: 11220 ``` I also verfied that repeated restarts produce the expected events in general, i.e. one dead event following one pid event. Closes https://github.com/cockroachdb/cockroach/issues/66522 Closes https://github.com/cockroachdb/cockroach/issues/66528 Release note: None --- pkg/cmd/roachprod/install/cluster_synced.go | 27 +++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/roachprod/install/cluster_synced.go b/pkg/cmd/roachprod/install/cluster_synced.go index 4420a790e225..1c7b34652061 100644 --- a/pkg/cmd/roachprod/install/cluster_synced.go +++ b/pkg/cmd/roachprod/install/cluster_synced.go @@ -366,22 +366,39 @@ if [ ! -f "{{.Store}}/CURRENT" ]; then fi {{- end}} -lastpid="" +# Init with -1 so that when cockroach is initially dead, we print +# a dead event for it. +lastpid=-1 while :; do {{ if .Local }} pid=$(lsof -i :{{.Port}} -sTCP:LISTEN | awk '!/COMMAND/ {print $2}') pid=${pid:-0} # default to 0 - status=unknown + status="unknown" {{- else }} + # When CRDB is not running, this is zero. pid=$(systemctl show cockroach --property MainPID --value) status=$(systemctl show cockroach --property ExecMainStatus --value) {{- end }} + if [[ "${lastpid}" == -1 && "${pid}" != 0 ]]; then + # On the first iteration through the loop, if the process is running, + # don't register a PID change (which would trigger an erroneous dead + # event). + lastpid=0 + fi + # Output a dead event whenever the PID changes from a nonzero value to + # any other value. In particular, we emit a dead event when the node stops + # (lastpid is nonzero, pid is zero), but not when the process then starts + # again (lastpid is zero, pid is nonzero). if [ "${pid}" != "${lastpid}" ]; then - # Output a dead event on every PID change, except if initial PID is live. - if [[ ! ("${lastpid}" == "" && "${pid}" != 0) ]]; then + if [ "${lastpid}" != 0 ]; then + if [ "${pid}" != 0 ]; then + # If the PID changed but neither is zero, then the status refers to + # the new incarnation. We lost the actual exit status of the old PID. + status="unknown" + fi echo "dead (exit status ${status})" - fi + fi if [ "${pid}" != 0 ]; then echo "${pid}" fi From 0e17774659590b082c43e621cf3da81cb4c51431 Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Wed, 16 Jun 2021 10:35:32 -0500 Subject: [PATCH 4/4] build: set ROACHPROD_USER in weekly roachtest script This was lost in 6651a08752080a3594714b9e8de009bf4155fbb1. Release note: None --- build/teamcity-weekly-roachtest.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/build/teamcity-weekly-roachtest.sh b/build/teamcity-weekly-roachtest.sh index d3fa7f1a262c..893728044270 100755 --- a/build/teamcity-weekly-roachtest.sh +++ b/build/teamcity-weekly-roachtest.sh @@ -8,6 +8,7 @@ set -euo pipefail google_credentials="$GOOGLE_EPHEMERAL_CREDENTIALS" source "$(dirname "${0}")/teamcity-support.sh" log_into_gcloud +export ROACHPROD_USER=teamcity set -x