Merge cockroachdb#107498 cockroachdb#107513 cockroachdb#107514 cockroachdb#107543 cockroachdb#107560

craig[bot] · mgartner · RaduBerinde · craig[bot] · commit 8961a3139ad9 · 2023-07-25T19:50:36.000Z
107498: TEAMS: use workflow for SQL Queries issue triage r=mgartner a=mgartner The SQL Queries team now uses Github Projects v2. This commit updates `TEAMS.yaml` and `.github/workflows/add-issues-to-project.yml` so that issues are assigned to the correct project for triage. Epic: None Release note: None 107513: go.mod: bump Pebble to 7ef7553fd9e1 r=RaduBerinde a=RaduBerinde 7ef7553f objstorage: rename 'shared' to 'remote' 4afa98e7 record: increase record.SyncConcurrency 05db8784 objstorage: rename sharedobjcat to remoteobjcat 2e84c30f record: pool LogWriter blocks f79908fb record: allow unbounded queued blocks in LogWriter fcaeb47a db: fix estimatedCompactionDebt metric 91c76637 sstable: split Iterator and helpers out of reader.go 64a6c5dd sstable: split single-level iter out of reader.go 7d2ca3be sstable: split two-level iter out of reader.go 55801b4b sstable: split VirtualReader out of reader.go 2291eb0a sstable: split Layout out of reader.go Release note: none Epic: none 107514: sqlsmith: a couple of minor fixes r=yuzefovich a=yuzefovich Fixes: cockroachdb#107458. 107543: dev,ui: make external symlink detection more resilient r=rickystewart a=sjbarag Previously, a pre-build assertion (that no JS dependencies are provided by symlinks outside the Bazel workspace) would return errors when it encountered a non-symlink file. While pnpm only produces symlinks for packages it installs, a stale node_modules/ tree could result in unintended errors. Test if a file is a symlink before attempting to call os.Readlink on it. Release note: None Epic: none 107560: ui: Update pnpm lockfile and version of pnpm r=nathanstilwell a=nathanstilwell Potentially due to a recent bug in pnpm[^1] where the order of the dependencies would effect the checksum, there is an error currently[^2] in the Cluster UI publishing workflow. To resolve this, this commit did the following: 1. With an updated version of pnpm (version 8.6.10), ran `pnpm install` which updated the `pnpm-lock.yaml` 1. Added and `engines` entry in `pkg/ui/package.json` to ensure that at least pnpm version 8.6.10 is used. 1. Updated the version of pnpm used in cluster ui release actions to 8.6.10 [^1]: pnpm/pnpm#6824 [^2]: https://go.crdb.dev/p/zhjq-wq6u-hnfk Release note (build change): fixing a bug in Cluster UI publishing due to pnpm version. Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com> Co-authored-by: Radu Berinde <radu@cockroachlabs.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Sean Barag <barag@cockroachlabs.com> Co-authored-by: Nathan Stilwell <nathanstilwell@cockroachlabs.com>
diff --git a/.github/workflows/add-issues-to-project.yml b/.github/workflows/add-issues-to-project.yml
@@ -30,3 +30,8 @@ jobs:
           project-url: https://github.com/orgs/cockroachdb/projects/35
           github-token: ${{ secrets.ADD_TO_PROJECT_PAT }}
           labeled: T-jobs
+      - uses: actions/add-to-project@v0.4.0
+        with:
+          project-url: https://github.com/orgs/cockroachdb/projects/45
+          github-token: ${{ secrets.ADD_TO_PROJECT_PAT }}
+          labeled: T-sql-queries
diff --git a/.github/workflows/cluster-ui-release-next.yml b/.github/workflows/cluster-ui-release-next.yml
@@ -30,7 +30,7 @@ jobs:
 
     - uses: pnpm/action-setup@v2
       with:
-        version: 8
+        version: ">=8.6.10"
 
     - name: Setup NodeJS
       uses: actions/setup-node@v3
diff --git a/.github/workflows/cluster-ui-release.yml b/.github/workflows/cluster-ui-release.yml
@@ -29,7 +29,7 @@ jobs:
 
     - uses: pnpm/action-setup@v2
       with:
-        version: 8
+        version: ">=8.6.10"
 
     - name: Setup NodeJS
       uses: actions/setup-node@v3
diff --git a/DEPS.bzl b/DEPS.bzl
@@ -1585,10 +1585,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "9175ac8aaf2f5e7a475fbebecb5b05ed013c1dd7fce1f1ec0504cb873bf71744",
-        strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230721205111-94f91669304b",
+        sha256 = "34017dde1a11e1792a7f46420cf683e7615e689c6da982405bc7e80271ffcb58",
+        strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230724234444-7ef7553fd9e1",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230721205111-94f91669304b.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230724234444-7ef7553fd9e1.zip",
         ],
     )
     go_repository(
diff --git a/TEAMS.yaml b/TEAMS.yaml
@@ -32,7 +32,9 @@ cockroachdb/sql-queries:
   aliases:
     cockroachdb/sql-optimizer: other
     cockroachdb/sql-opt-prs: other
-  triage_column_id: 13549252
+  # SQL Queries team uses GH projects v2, which doesn't have a REST API, so
+  # there is no triage column ID.
+  # See .github/workflows/add-issues-to-project.yml.
   label: T-sql-queries
 cockroachdb/cluster-observability:
   triage_column_id: 12618343
diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl
@@ -319,7 +319,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230721205111-94f91669304b.zip": "9175ac8aaf2f5e7a475fbebecb5b05ed013c1dd7fce1f1ec0504cb873bf71744",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230724234444-7ef7553fd9e1.zip": "34017dde1a11e1792a7f46420cf683e7615e689c6da982405bc7e80271ffcb58",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",
diff --git a/dev b/dev
@@ -8,7 +8,7 @@ fi
 set -euo pipefail
 
 # Bump this counter to force rebuilding `dev` on all machines.
-DEV_VERSION=81
+DEV_VERSION=82
 
 THIS_DIR=$(cd "$(dirname "$0")" && pwd)
 BINARY_DIR=$THIS_DIR/bin/dev-versions
diff --git a/go.mod b/go.mod
@@ -116,7 +116,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
-	github.com/cockroachdb/pebble v0.0.0-20230721205111-94f91669304b
+	github.com/cockroachdb/pebble v0.0.0-20230724234444-7ef7553fd9e1
 	github.com/cockroachdb/redact v1.1.5
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b
diff --git a/go.sum b/go.sum
@@ -490,8 +490,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy
 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
-github.com/cockroachdb/pebble v0.0.0-20230721205111-94f91669304b h1:QU4vwZQ2DkcrMMB9wnLKa2uQZnzlye8kSCwnmDSPUrQ=
-github.com/cockroachdb/pebble v0.0.0-20230721205111-94f91669304b/go.mod h1:FN5O47SBEz5+kO9fG8UTR64g2WS1u5ZFCgTvxGjoSks=
+github.com/cockroachdb/pebble v0.0.0-20230724234444-7ef7553fd9e1 h1:aBPW14kJEdbuzyryv+8u1wxY6k8bu7uNVEuifOZxjRg=
+github.com/cockroachdb/pebble v0.0.0-20230724234444-7ef7553fd9e1/go.mod h1:FN5O47SBEz5+kO9fG8UTR64g2WS1u5ZFCgTvxGjoSks=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
 github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
diff --git a/pkg/cmd/dev/io/os/os.go b/pkg/cmd/dev/io/os/os.go
@@ -195,6 +195,35 @@ func (o *OS) Setenv(key, value string) error {
 	return err
 }
 
+// IsSymlink wraps around os.Lstat to determine if filename is a symbolic link
+// or not.
+func (o *OS) IsSymlink(filename string) (bool, error) {
+	command := fmt.Sprintf("stat %s", filename)
+	if !o.knobs.silent {
+		o.logger.Print(command)
+	}
+
+	isLinkStr, err := o.Next(command, func() (string, error) {
+		// Use os.Lstat here, since it does not attempt to resolve symlinks.
+		stat, err := os.Lstat(filename)
+		if err != nil {
+			return "", err
+		}
+
+		isLink := stat.Mode()&fs.ModeSymlink != 0
+
+		// o.Next only accepts string return values, so serialize a boolean
+		// and deserialize it outside of o.Next.
+		return strconv.FormatBool(isLink), nil
+	})
+
+	if err != nil {
+		return false, err
+	}
+
+	return strconv.ParseBool(isLinkStr)
+}
+
 // Readlink wraps around os.Readlink, which returns the destination of the named
 // symbolic link. If there is an error, it will be of type *PathError.
 func (o *OS) Readlink(filename string) (string, error) {
diff --git a/pkg/cmd/dev/testdata/datadriven/ui b/pkg/cmd/dev/testdata/datadriven/ui
@@ -146,7 +146,10 @@ rm -rf crdb-checkout/pkg/ui/workspaces/eslint-plugin-crdb/dist
 rm -rf crdb-checkout/pkg/ui/node_modules
 rm -rf crdb-checkout/pkg/ui/workspaces/db-console/node_modules
 rm -rf crdb-checkout/pkg/ui/workspaces/db-console/src/js/node_modules
+rm -rf crdb-checkout/pkg/ui/workspaces/db-console/ccl/src/js/node_modules
 rm -rf crdb-checkout/pkg/ui/workspaces/cluster-ui/node_modules
+rm -rf crdb-checkout/pkg/ui/workspaces/e2e-tests/node_modules
+rm -rf crdb-checkout/pkg/ui/workspaces/eslint-plugin-crdb/node_modules
 
 exec
 dev ui e2e
diff --git a/pkg/cmd/dev/ui.go b/pkg/cmd/dev/ui.go
@@ -162,8 +162,24 @@ func (d *dev) assertNoLinkedNpmDeps(targets []buildTarget) error {
 				continue
 			}
 
-			// Resolve the possible symlink.
 			depPath := filepath.Join(crlModulesPath, depName)
+
+			// Determine if depName is a symlink. It should be when dependencies
+			// are installed via pnpm, but there are cases involving stale
+			// node_modules/ trees that can result in it being a regular
+			// directory.
+			isSymlink, err := d.os.IsSymlink(depPath)
+			if err != nil {
+				return fmt.Errorf("could not determine if %s is a symlink: %w", depPath, err)
+			}
+
+			// Regular directories cannot (by definition) link outside the Bazel
+			// workspace, so are safe to ignore.
+			if !isSymlink {
+				continue
+			}
+
+			// Resolve the symlink.
 			resolved, err := d.os.Readlink(depPath)
 			if err != nil {
 				return fmt.Errorf("could not evaluate symlink %s: %w", depPath, err)
@@ -591,7 +607,10 @@ func makeUICleanCmd(d *dev) *cobra.Command {
 					filepath.Join(workspace, "pkg", "ui", "node_modules"),
 					filepath.Join(uiDirs.dbConsole, "node_modules"),
 					filepath.Join(uiDirs.dbConsole, "src", "js", "node_modules"),
+					filepath.Join(uiDirs.dbConsole, "ccl", "src", "js", "node_modules"),
 					filepath.Join(uiDirs.clusterUI, "node_modules"),
+					filepath.Join(uiDirs.e2eTests, "node_modules"),
+					filepath.Join(uiDirs.eslintPlugin, "node_modules"),
 				)
 			}
 
diff --git a/pkg/cmd/roachtest/tests/sqlsmith.go b/pkg/cmd/roachtest/tests/sqlsmith.go
@@ -194,7 +194,8 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version;
 			stmt := ""
 			err := func() error {
 				done := make(chan error, 1)
-				go func(context.Context) {
+				m := c.NewMonitor(ctx, c.Node(1))
+				m.Go(func(context.Context) error {
 					// Generate can potentially panic in bad cases, so
 					// to avoid Go routines from dying we are going
 					// catch that here, and only pass the error into
@@ -210,7 +211,7 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version;
 					if stmt == "" {
 						// If an empty statement is generated, then ignore it.
 						done <- errors.Newf("Empty statement returned by generate")
-						return
+						return nil
 					}
 
 					// TODO(yuzefovich): investigate why using the context with
@@ -225,7 +226,9 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version;
 						}
 					}
 					done <- err
-				}(ctx)
+					return nil
+				})
+				defer m.Wait()
 				select {
 				case <-time.After(timeout * 2):
 					// SQLSmith generates queries that either perform full table scans of
diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go
@@ -1099,10 +1099,12 @@ func (s *Smither) makeCreateFunc() (cf *tree.CreateFunction, ok bool) {
 		IsUDF:      true,
 	}
 
-	functions[class][rtyp.Oid()] = append(functions[class][rtyp.Oid()], function{
+	functions.Lock()
+	functions.fns[class][rtyp.Oid()] = append(functions.fns[class][rtyp.Oid()], function{
 		def:      tree.NewFunctionDefinition(name.String(), &tree.FunctionProperties{}, nil /* def */),
 		overload: ov,
 	})
+	functions.Unlock()
 	return stmt, true
 }
 
diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go
@@ -413,7 +413,9 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
 	if class == tree.WindowClass && s.d6() != 1 {
 		class = tree.NormalClass
 	}
-	fns := functions[class][typ.Oid()]
+	functions.Lock()
+	fns := functions.fns[class][typ.Oid()]
+	functions.Unlock()
 	if len(fns) == 0 {
 		return nil, false
 	}
diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treebin"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/lib/pq/oid"
 )
 
@@ -483,7 +484,14 @@ type function struct {
 	overload *tree.Overload
 }
 
-var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
+type functionsMu struct {
+	syncutil.Mutex
+	// User-defined functions are added into the map after the initialization,
+	// so we need to protect the map with the mutex.
+	fns map[tree.FunctionClass]map[oid.Oid][]function
+}
+
+var functions = func() *functionsMu {
 	m := map[tree.FunctionClass]map[oid.Oid][]function{}
 	for _, def := range tree.FunDefs {
 		switch def.Name {
@@ -505,10 +513,6 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
 
 		skip := false
 		for _, substr := range []string{
-			// crdb_internal.complete_stream_ingestion_job is a stateful
-			// function that requires a running stream ingestion job. Invoking
-			// this against random parameters is likely to fail and so we skip
-			// it.
 			"stream_ingestion",
 			"crdb_internal.force_",
 			"crdb_internal.unsafe_",
@@ -558,5 +562,5 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
 			})
 		}
 	}
-	return m
+	return &functionsMu{fns: m}
 }()
diff --git a/pkg/ui/package.json b/pkg/ui/package.json
@@ -4,6 +4,9 @@
   "private": true,
   "repository": "github.com/cockroachdb/cockroach",
   "license": "MIT",
+  "engines": {
+    "pnpm": ">=8.6.10"
+  },
   "pnpm": {
     "neverBuiltDependencies": [
       "contextify"
diff --git a/pkg/ui/pnpm-lock.yaml b/pkg/ui/pnpm-lock.yaml
