Merge #125610 #125694 #125847

125610: rangefeed: remove non-muxrangefeed code r=nvanbenschoten a=wenyihu6 In 22.2, mux rangefeeds were introduced to reduce the number of rpc streams from one per replica to one per client for all rangefeeds on a node. In 24.1, to simplify the implementation of #110432, we removed any cluster setting for enabling non-mux rangefeeds, making them always enabled. But we retained the non-mux rangefeed code there for test coverage in mixed-version clusters. Given that we are going to 24.2, we are now more comfortable with removing the tests. This patch completely removes client and server code for non-mux rangefeed. Resolves: #125666 Release note: none 125694: compose: TestComposeCompare improvements r=rafiss a=rafiss This PR accomplishes: - properly link libgeos - preserve logs on failure - run tests as current user fixes #125286 fixes #125027 fixes #125392 fixes #124379 Release note: None 125847: acceptance/compose: add docker health check r=rafiss a=rail Previously, docker compose based GSS tests sometimes failed, because the cockroach container took some time to bring the server up. This doesn't happen with `--insecure` tests, such as `flyway`. This PR adds a health check in order to ensure the cockroach container is healthy before running any tests. Fixes: #125003 Fixes: #125089 Fixes: #125413 Release note: None Co-authored-by: Wenyi Hu <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Rail Aliiev <[email protected]>
cockroachdb · Jun 18, 2024 · 8b17f49 · 8b17f49
4 parents a4e20b7 + 294e45d + 297c0c2 + a0f8d9f
commit 8b17f49
Show file tree

Hide file tree

Showing 24 changed files with 77 additions and 714 deletions.
diff --git a/build/bazelutil/check.sh b/build/bazelutil/check.sh
@@ -21,7 +21,7 @@ pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangeca
 pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -destination=mocks_generated_test.go --package=rangefeed . DB
 pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *keyLocks concurrency
 pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch
-pkg/kv/kvpb/api.go://go:generate mockgen -package=kvpbmock -destination=kvpbmock/mocks_generated.go . InternalClient,Internal_RangeFeedClient,Internal_MuxRangeFeedClient
+pkg/kv/kvpb/api.go://go:generate mockgen -package=kvpbmock -destination=kvpbmock/mocks_generated.go . InternalClient,Internal_MuxRangeFeedClient
 pkg/kv/kvpb/batch.go://go:generate go run gen/main.go --filename batch_generated.go *.pb.go
 pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated_test.go . Cert
 pkg/spanconfig/spanconfigstore/span_store.go://go:generate ../../util/interval/generic/gen.sh *entry spanconfigstore

diff --git a/pkg/acceptance/compose/gss/docker-compose-python.yml b/pkg/acceptance/compose/gss/docker-compose-python.yml
@@ -17,11 +17,17 @@ services:
       - ${CERTS_DIR:-../../.localcluster.certs}:/certs
       - keytab:/keytab
       - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach
+    healthcheck:
+      test: "bash -c 'echo -n > /dev/tcp/cockroach/26257'"
+      interval: 0.5s
+      timeout: 10s
+      retries: 25
   python:
     image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-python:20221214-141947
     user: "${UID}:${GID}"
     depends_on:
-      - cockroach
+      cockroach:
+        condition: service_healthy
     environment:
       - PGHOST=cockroach
       - PGPORT=26257

diff --git a/pkg/acceptance/compose/gss/docker-compose.yml b/pkg/acceptance/compose/gss/docker-compose.yml
@@ -1,3 +1,4 @@
+version: '3'
 services:
   kdc:
     image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-kdc:20221214-131000
@@ -17,11 +18,17 @@ services:
       - ${CERTS_DIR:-../../.localcluster.certs}:/certs
       - keytab:/keytab
       - ${COCKROACH_BINARY:-../../../../cockroach-linux-2.6.32-gnu-amd64}:/cockroach/cockroach
+    healthcheck:
+      test: "bash -c 'echo -n > /dev/tcp/cockroach/26257'"
+      interval: 0.5s
+      timeout: 10s
+      retries: 25
   psql:
     image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-psql:20230907-113902
     user: "${UID}:${GID}"
     depends_on:
-      - cockroach
+      cockroach:
+        condition: service_healthy
     environment:
       - PGHOST=cockroach
       - PGPORT=26257

diff --git a/pkg/compose/BUILD.bazel b/pkg/compose/BUILD.bazel
@@ -14,6 +14,7 @@ go_test(
     ],
     deps = [
         "//pkg/build/bazel",
+        "//pkg/testutils/datapathutils",
         "//pkg/testutils/skip",
         "//pkg/util/envutil",
     ],

diff --git a/pkg/compose/compare/compare/BUILD.bazel b/pkg/compose/compare/compare/BUILD.bazel
@@ -10,6 +10,7 @@ go_test(
     visibility = ["//pkg/compose:__subpackages__"],
     deps = [
         "//pkg/cmd/cmpconn",
+        "//pkg/geo/geos",
         "//pkg/internal/sqlsmith",
         "//pkg/sql/randgen",
         "//pkg/testutils",

diff --git a/pkg/compose/compare/compare/compare_test.go b/pkg/compose/compare/compare/compare_test.go
@@ -15,11 +15,13 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/cmpconn"
+	"github.com/cockroachdb/cockroach/pkg/geo/geos"
 	"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
 	"github.com/cockroachdb/cockroach/pkg/sql/randgen"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -43,6 +45,16 @@ func TestCompare(t *testing.T) {
 	var license = envutil.EnvOrDefaultString("COCKROACH_DEV_LICENSE", "")
 	require.NotEmptyf(t, license, "COCKROACH_DEV_LICENSE must be set")
 
+	// Initialize GEOS libraries so that the test can use geospatial types.
+	workingDir, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = geos.EnsureInit(geos.EnsureInitErrorDisplayPrivate, path.Join(workingDir, "lib"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	uris := map[string]struct {
 		addr string
 		init []string

diff --git a/pkg/compose/compare/docker-compose.yml b/pkg/compose/compare/docker-compose.yml
@@ -3,18 +3,26 @@ services:
     # We use a docker image mirror to avoid pulling from 3rd party repos, which sometimes have reliability issues.
     # See https://cockroachlabs.atlassian.net/wiki/spaces/devinf/pages/3462594561/Docker+image+sync for the details.
     image: us-east1-docker.pkg.dev/crl-docker-sync/docker-mirror/docker.io/library/ubuntu:xenial-20170214
-    command: /cockroach/cockroach start-single-node --insecure --listen-addr cockroach1
+    user: ${UID}:${GID}
+    command: /cockroach/cockroach start-single-node --insecure --store=/cockroach/store --spatial-libs=/cockroach/lib --listen-addr cockroach1
     volumes:
       - "${COCKROACH_PATH}:/cockroach/cockroach"
       - "${LIBGEOS_DIR_PATH}:/cockroach/lib"
+      - "${COMPARE_DIR_PATH}/store1:/cockroach/store"
+      - /etc/passwd:/etc/passwd:ro
+      - /etc/group:/etc/group:ro
   cockroach2:
     # We use a docker image mirror to avoid pulling from 3rd party repos, which sometimes have reliability issues.
     # See https://cockroachlabs.atlassian.net/wiki/spaces/devinf/pages/3462594561/Docker+image+sync for the details.
     image: us-east1-docker.pkg.dev/crl-docker-sync/docker-mirror/docker.io/library/ubuntu:xenial-20170214
-    command: /cockroach/cockroach start-single-node --insecure --listen-addr cockroach2
+    user: ${UID}:${GID}
+    command: /cockroach/cockroach start-single-node --insecure --store=/cockroach/store --spatial-libs=/cockroach/lib --listen-addr cockroach2
     volumes:
       - "${COCKROACH_PATH}:/cockroach/cockroach"
       - "${LIBGEOS_DIR_PATH}:/cockroach/lib"
+      - "${COMPARE_DIR_PATH}/store2:/cockroach/store"
+      - /etc/passwd:/etc/passwd:ro
+      - /etc/group:/etc/group:ro
   test:
     # We use a docker image mirror to avoid pulling from 3rd party repos, which sometimes have reliability issues.
     # See https://cockroachlabs.atlassian.net/wiki/spaces/devinf/pages/3462594561/Docker+image+sync for the details.

diff --git a/pkg/compose/compose_test.go b/pkg/compose/compose_test.go
@@ -17,11 +17,13 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/build/bazel"
+	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 )
@@ -76,10 +78,24 @@ func TestComposeCompare(t *testing.T) {
 		// start up docker-compose, but the files themselves will be
 		// Bazel-built symlinks. We want to copy these files to a
 		// different temporary location.
-		composeBinsDir := t.TempDir()
-		compareDir = composeBinsDir
-		cockroachBin = filepath.Join(composeBinsDir, "cockroach")
-		libGeosDir = filepath.Join(composeBinsDir, "lib")
+		compareDir, err = os.MkdirTemp(datapathutils.DebuggableTempDir(), "TestComposeCompare")
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Cleanup(func() {
+			if t.Failed() {
+				return
+			}
+			_ = os.RemoveAll(compareDir)
+		})
+		if err = os.MkdirAll(filepath.Join(compareDir, "store1"), 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err = os.MkdirAll(filepath.Join(compareDir, "store2"), 0755); err != nil {
+			t.Fatal(err)
+		}
+		cockroachBin = filepath.Join(compareDir, "cockroach")
+		libGeosDir = filepath.Join(compareDir, "lib")
 		if err = os.MkdirAll(libGeosDir, 0755); err != nil {
 			t.Fatal(err)
 		}
@@ -93,7 +109,7 @@ func TestComposeCompare(t *testing.T) {
 				t.Fatal(err)
 			}
 		}
-		if err = copyBin(*flagCompare, filepath.Join(composeBinsDir, "compare.test")); err != nil {
+		if err = copyBin(*flagCompare, filepath.Join(compareDir, "compare.test")); err != nil {
 			t.Fatal(err)
 		}
 		if *flagArtifacts == "" {
@@ -126,7 +142,13 @@ func TestComposeCompare(t *testing.T) {
 		"--force-recreate",
 		"--exit-code-from", "test",
 	)
+	userInfo, err := user.Current()
+	if err != nil {
+		t.Fatal(err)
+	}
 	cmd.Env = []string{
+		fmt.Sprintf("UID=%s", userInfo.Uid),
+		fmt.Sprintf("GID=%s", userInfo.Gid),
 		fmt.Sprintf("EACH=%s", *flagEach),
 		fmt.Sprintf("TESTS=%s", *flagTests),
 		fmt.Sprintf("COCKROACH_PATH=%s", cockroachBin),
@@ -137,6 +159,7 @@ func TestComposeCompare(t *testing.T) {
 		fmt.Sprintf("PATH=%s", os.Getenv("PATH")),
 		"COCKROACH_RUN_COMPOSE_COMPARE=true",
 	}
+	t.Logf("running: %s", cmd)
 	out, err := cmd.CombinedOutput()
 	t.Log(string(out))
 	if err != nil {