Merge #83138 #84037 #84092

83138: publish-artifacts: Add GCS support r=rickystewart,jlinder a=rail

Previously, the only supported storage provider was S3.

This PR adds support for GCS.
* Make the bucket name be provider specific.
* The GCS bucket is optional for now.
* Move logic to separate files.
* No direct s3 imports from publish-artifacts and publish-provisional-artifacts.
* Create an extra abstraction interface to hide the s3-specific structs and calls.
* The "latest" URLs in publish-provisional-artifacts use S3's website redirect instead of copying the whole binary. This unifies it with publish-artifacts.

Release note: None

84037: ui: add mvcc info to Database page r=maryliag a=maryliag

This commit adds the mvcc info to:
- Column on the tables list inside Database page
- Table Details page

Fixes #82617

<img width="1429" alt="Screen Shot 2022-07-11 at 5 20 04 PM" src="https://user-images.githubusercontent.com/1017486/178360499-b8c53f64-726f-4a37-adf9-a6047fb6d7bf.png">

<img width="551" alt="Screen Shot 2022-07-11 at 5 20 53 PM" src="https://user-images.githubusercontent.com/1017486/178360553-155b78bf-9c08-49e3-ad72-97822aced286.png">


<img width="269" alt="Screen Shot 2022-07-08 at 5 35 49 PM" src="https://user-images.githubusercontent.com/1017486/178073512-6566e288-643a-4f64-96fb-7ccb843c24ea.png">



Release note (ui change): Adds mvcc info to tables list inside
the Database page and on the Tables details page.

84092: opt: fix nil pointer exception in SplitGroupByScanIntoUnionScans rule r=mgartner a=mgartner

#### opt: remove unnecessary queueLength variable in splitScanIntoUnionScansOrSelects

Release note: None

#### opt: fix nil pointer exception in SplitGroupByScanIntoUnionScans rule

This commit fixes a nil pointer exception in the
`SplitGroupByScanIntoUnionScans` rule that occurs when the first
splittable span in a constraint has more individual keys than the
`maxScanCount` of 256.

Fixes #83973

Release note (bug fix): A bug has been fixed that could cause internal
errors in rare cases when running queries with `GROUP BY` clauses.


Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

build/bazelutil/check.sh

@@ -55,7 +55,6 @@ pkg/acceptance/BUILD.bazel
 pkg/cmd/cockroach-oss/BUILD.bazel
 pkg/cmd/github-post/BUILD.bazel
 pkg/cmd/prereqs/BUILD.bazel
-pkg/cmd/publish-artifacts/BUILD.bazel
 pkg/cmd/roachtest/BUILD.bazel
 pkg/cmd/teamcity-trigger/BUILD.bazel
 "

pkg/cmd/publish-artifacts/BUILD.bazel

@@ -7,9 +7,6 @@ go_library(
     visibility = ["//visibility:private"],
     deps = [
         "//pkg/release",
-        "@com_github_aws_aws_sdk_go//aws",
-        "@com_github_aws_aws_sdk_go//aws/session",
-        "@com_github_aws_aws_sdk_go//service/s3",
         "@com_github_kr_pretty//:pretty",
     ],
 )
@@ -29,8 +26,6 @@ go_test(
         "//pkg/release",
         "//pkg/testutils",
         "@com_github_alessio_shellescape//:shellescape",
-        "@com_github_aws_aws_sdk_go//service/s3",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",
     ],
 )

pkg/cmd/publish-artifacts/main.go

@@ -18,9 +18,6 @@ import (
 	"os/exec"
 	"path/filepath"
 
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/session"
-	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/cockroachdb/cockroach/pkg/release"
 	"github.com/kr/pretty"
 )
@@ -31,24 +28,9 @@ const (
 	teamcityBuildBranchKey = "TC_BUILD_BRANCH"
 )
 
-type s3putter interface {
-	PutObject(*s3.PutObjectInput) (*s3.PutObjectOutput, error)
-}
-
-// Overridden in testing.
-var testableS3 = func() (s3putter, error) {
-	sess, err := session.NewSession(&aws.Config{
-		Region: aws.String("us-east-1"),
-	})
-	if err != nil {
-		return nil, err
-	}
-	return s3.New(sess), nil
-}
-
-var destBucket = flag.String("bucket", "", "override default bucket")
-
 func main() {
+	var destBucket = flag.String("bucket", "cockroach", "override default bucket")
+	var gcsBucket = flag.String("gcs-bucket", "", "override default bucket")
 	flag.Parse()
 
 	if _, ok := os.LookupEnv(awsAccessKeyIDKey); !ok {
@@ -81,119 +63,109 @@ func main() {
 	}
 	versionStr := string(bytes.TrimSpace(out))
 
-	svc, err := testableS3()
+	var providers []release.ObjectPutGetter
+	s3, err := release.NewS3("us-east-1", *destBucket)
 	if err != nil {
 		log.Fatalf("Creating AWS S3 session: %s", err)
 	}
-
-	var bucketName string
-	if len(*destBucket) > 0 {
-		bucketName = *destBucket
-	} else {
-		bucketName = "cockroach"
+	providers = append(providers, s3)
+
+	if *gcsBucket != "" {
+		if _, ok := os.LookupEnv("GOOGLE_APPLICATION_CREDENTIALS"); !ok {
+			log.Fatal("GOOGLE_APPLICATION_CREDENTIALS environment variable is not set")
+		}
+		gcs, err := release.NewGCS(*gcsBucket)
+		if err != nil {
+			log.Fatalf("Creating GCS session: %s", err)
+		}
+		providers = append(providers, gcs)
 	}
-	log.Printf("Using S3 bucket: %s", bucketName)
 
-	run(svc, runFlags{
-		pkgDir:     pkg,
-		branch:     branch,
-		sha:        versionStr,
-		bucketName: bucketName,
+	run(providers, runFlags{
+		pkgDir: pkg,
+		branch: branch,
+		sha:    versionStr,
 	}, release.ExecFn{})
 }
 
 type runFlags struct {
-	branch, sha string
-	pkgDir      string
-	bucketName  string
+	branch string
+	sha    string
+	pkgDir string
 }
 
-func run(svc s3putter, flags runFlags, execFn release.ExecFn) {
+func run(providers []release.ObjectPutGetter, flags runFlags, execFn release.ExecFn) {
 	for _, platform := range []release.Platform{release.PlatformLinux, release.PlatformLinuxArm, release.PlatformMacOS, release.PlatformWindows} {
 		var o opts
 		o.Platform = platform
-		o.ReleaseVersionStrs = []string{flags.sha}
+		o.ReleaseVersions = []string{flags.sha}
 		o.PkgDir = flags.pkgDir
 		o.Branch = flags.branch
 		o.VersionStr = flags.sha
-		o.BucketName = flags.bucketName
 		o.AbsolutePath = filepath.Join(flags.pkgDir, "cockroach"+release.SuffixFromPlatform(platform))
 		o.CockroachSQLAbsolutePath = filepath.Join(flags.pkgDir, "cockroach-sql"+release.SuffixFromPlatform(platform))
 
 		log.Printf("building %s", pretty.Sprint(o))
-
-		buildOneCockroach(svc, o, execFn)
+		buildOneCockroach(providers, o, execFn)
 	}
 	// We build workload only for Linux.
 	var o opts
 	o.Platform = release.PlatformLinux
 	o.PkgDir = flags.pkgDir
-	o.BucketName = flags.bucketName
 	o.Branch = flags.branch
 	o.VersionStr = flags.sha
-	buildOneWorkload(svc, o, execFn)
+	buildAndPublishWorkload(providers, o, execFn)
 }
 
-func buildOneCockroach(svc s3putter, o opts, execFn release.ExecFn) {
+func buildOneCockroach(providers []release.ObjectPutGetter, o opts, execFn release.ExecFn) {
 	log.Printf("building cockroach %s", pretty.Sprint(o))
-	defer func() {
-		log.Printf("done building cockroach: %s", pretty.Sprint(o))
-	}()
-
 	if err := release.MakeRelease(o.Platform, release.BuildOptions{ExecFn: execFn}, o.PkgDir); err != nil {
 		log.Fatal(err)
 	}
-
-	putNonRelease(svc, o, release.MakeCRDBLibraryNonReleaseFiles(o.PkgDir, o.Platform, o.VersionStr)...)
+	for _, provider := range providers {
+		release.PutNonRelease(
+			provider,
+			release.PutNonReleaseOptions{
+				Branch: o.Branch,
+				Files: append(
+					[]release.NonReleaseFile{
+						release.MakeCRDBBinaryNonReleaseFile(o.AbsolutePath, o.VersionStr),
+						release.MakeCRDBBinaryNonReleaseFile(o.CockroachSQLAbsolutePath, o.VersionStr),
+					},
+					release.MakeCRDBLibraryNonReleaseFiles(o.PkgDir, o.Platform, o.VersionStr)...,
+				),
+			},
+		)
+	}
+	log.Printf("done building cockroach: %s", pretty.Sprint(o))
 }
 
-func buildOneWorkload(svc s3putter, o opts, execFn release.ExecFn) {
-	defer func() {
-		log.Printf("done building workload: %s", pretty.Sprint(o))
-	}()
-
+func buildAndPublishWorkload(providers []release.ObjectPutGetter, o opts, execFn release.ExecFn) {
+	log.Printf("building workload %s", pretty.Sprint(o))
 	if err := release.MakeWorkload(release.BuildOptions{ExecFn: execFn}, o.PkgDir); err != nil {
 		log.Fatal(err)
 	}
 	o.AbsolutePath = filepath.Join(o.PkgDir, "bin", "workload")
-	release.PutNonRelease(
-		svc,
-		release.PutNonReleaseOptions{
-			Branch:     o.Branch,
-			BucketName: o.BucketName,
-			Files: []release.NonReleaseFile{
-				release.MakeCRDBBinaryNonReleaseFile(o.AbsolutePath, o.VersionStr),
+	for _, provider := range providers {
+		release.PutNonRelease(
+			provider,
+			release.PutNonReleaseOptions{
+				Branch: o.Branch,
+				Files: []release.NonReleaseFile{
+					release.MakeCRDBBinaryNonReleaseFile(o.AbsolutePath, o.VersionStr),
+				},
 			},
-		},
-	)
+		)
+	}
+	log.Printf("done building workload: %s", pretty.Sprint(o))
 }
 
 type opts struct {
-	VersionStr         string
-	Branch             string
-	ReleaseVersionStrs []string
-
-	Platform release.Platform
-
-	BucketName               string
+	VersionStr               string
+	Branch                   string
+	ReleaseVersions          []string
+	Platform                 release.Platform
 	AbsolutePath             string
 	CockroachSQLAbsolutePath string
 	PkgDir                   string
 }
-
-func putNonRelease(svc s3putter, o opts, additionalNonReleaseFiles ...release.NonReleaseFile) {
-	release.PutNonRelease(
-		svc,
-		release.PutNonReleaseOptions{
-			Branch:     o.Branch,
-			BucketName: o.BucketName,
-			Files: append(
-				[]release.NonReleaseFile{
-					release.MakeCRDBBinaryNonReleaseFile(o.AbsolutePath, o.VersionStr),
-					release.MakeCRDBBinaryNonReleaseFile(o.CockroachSQLAbsolutePath, o.VersionStr),
-				},
-				additionalNonReleaseFiles...,
-			),
-		},
-	)
-}

pkg/cmd/publish-artifacts/main_test.go

@@ -21,28 +21,38 @@ import (
 	"unicode/utf8"
 
 	"github.com/alessio/shellescape"
-	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/cockroachdb/cockroach/pkg/release"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
-	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
-type mockS3 struct {
+type mockStorage struct {
 	puts []string
 }
 
-var _ s3putter = (*mockS3)(nil)
+var _ release.ObjectPutGetter = (*mockStorage)(nil)
 
-func (s *mockS3) PutObject(i *s3.PutObjectInput) (*s3.PutObjectOutput, error) {
-	url := fmt.Sprintf(`s3://%s/%s`, *i.Bucket, *i.Key)
+func (s *mockStorage) Bucket() string {
+	return "cockroach"
+}
+
+func (s mockStorage) URL(key string) string {
+	return "storage://bucket/" + key
+}
+
+func (s *mockStorage) GetObject(*release.GetObjectInput) (*release.GetObjectOutput, error) {
+	return &release.GetObjectOutput{}, nil
+}
+
+func (s *mockStorage) PutObject(i *release.PutObjectInput) error {
+	url := fmt.Sprintf(`s3://%s/%s`, s.Bucket(), *i.Key)
 	if i.CacheControl != nil {
 		url += `/` + *i.CacheControl
 	}
 	if i.Body != nil {
 		bytes, err := ioutil.ReadAll(i.Body)
 		if err != nil {
-			return nil, err
+			return err
 		}
 		if strings.HasSuffix(*i.Key, release.ChecksumSuffix) {
 			// Unfortunately the archive tarball checksum changes every time,
@@ -57,7 +67,7 @@ func (s *mockS3) PutObject(i *s3.PutObjectInput) (*s3.PutObjectOutput, error) {
 	} else if i.WebsiteRedirectLocation != nil {
 		s.puts = append(s.puts, fmt.Sprintf("%s REDIRECT %s", url, *i.WebsiteRedirectLocation))
 	}
-	return &s3.PutObjectOutput{}, nil
+	return nil
 }
 
 type mockExecRunner struct {
@@ -70,7 +80,7 @@ func (r *mockExecRunner) run(c *exec.Cmd) ([]byte, error) {
 		panic("r.fakeBazelBin not set")
 	}
 	if c.Dir == `` {
-		return nil, errors.Errorf(`Dir must be specified`)
+		return nil, fmt.Errorf("`Dir` must be specified")
 	}
 	cmd := fmt.Sprintf("env=%s args=%s", c.Env, shellescape.QuoteCommand(c.Args))
 	r.cmds = append(r.cmds, cmd)
@@ -138,9 +148,8 @@ func TestPublish(t *testing.T) {
 		{
 			name: `release`,
 			flags: runFlags{
-				branch:     "master",
-				sha:        "1234567890abcdef",
-				bucketName: "cockroach",
+				branch: "master",
+				sha:    "1234567890abcdef",
 			},
 			expectedCmds: []string{
 				"env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
@@ -251,17 +260,19 @@ func TestPublish(t *testing.T) {
 			dir, cleanup := testutils.TempDir(t)
 			defer cleanup()
 
-			var s3 mockS3
-			var exec mockExecRunner
+			var s3 mockStorage
+			var gcs mockStorage
+			var runner mockExecRunner
 			fakeBazelBin, cleanup := testutils.TempDir(t)
 			defer cleanup()
-			exec.fakeBazelBin = fakeBazelBin
+			runner.fakeBazelBin = fakeBazelBin
 			flags := test.flags
 			flags.pkgDir = dir
-			execFn := release.ExecFn{MockExecFn: exec.run}
-			run(&s3, flags, execFn)
-			require.Equal(t, test.expectedCmds, exec.cmds)
+			execFn := release.ExecFn{MockExecFn: runner.run}
+			run([]release.ObjectPutGetter{&s3, &gcs}, flags, execFn)
+			require.Equal(t, test.expectedCmds, runner.cmds)
 			require.Equal(t, test.expectedPuts, s3.puts)
+			require.Equal(t, test.expectedPuts, gcs.puts)
 		})
 	}
 }

pkg/cmd/publish-provisional-artifacts/BUILD.bazel

@@ -8,9 +8,6 @@ go_library(
     deps = [
         "//pkg/release",
         "//pkg/util/version",
-        "@com_github_aws_aws_sdk_go//aws",
-        "@com_github_aws_aws_sdk_go//aws/session",
-        "@com_github_aws_aws_sdk_go//service/s3",
         "@com_github_kr_pretty//:pretty",
     ],
 )
@@ -30,8 +27,6 @@ go_test(
         "//pkg/release",
         "//pkg/testutils",
         "@com_github_alessio_shellescape//:shellescape",
-        "@com_github_aws_aws_sdk_go//service/s3",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",
     ],
 )