Merge #62611 #62764 #62778 #62815 #62823

62611: sql: lease acquisition of OFFLINE descs may starve bulk operations r=ajwerner a=fqazi

Fixes: #61798

Previously, offline descriptors would never have their leases
cached and they would be released once the reference count hit zero.
This was inadequate because when attempting to online these tables
again the lease acquisition could be pushed back by other operations,
leading to starvation / live locks. To address this, this patch will
allow the leases of offline descriptors to be cached.

Release note (bug fix): Lease acquisitions of descriptor in a
offline state may starve out bulk operations (backup / restore)

62764: sql: gate global_reads zone attribute on cluster version and enterprise license r=nvanbenschoten a=nvanbenschoten

Fixes #61039.

This commit gates the use of the global_reads zone configuration
attribute on the cluster version and on the use of an enterprise
license.

The first half of this is necessary for correctness. The second half is
not, because follower reads won't be served from global read followers
without an enterprise license, but a check when the zone configs are set
improves UX.

62778: geo/geomfn: refactor logic for point in polygon optimization  r=otan a=andyyang890

Release note: None

62815: build: upgrade go to 1.15.10 r=otan a=rickystewart

Pick up a patch to a compiler bug (see #62521).

* [ ] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [ ] Adjust `GO_VERSION` in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
  and ask the Developer Infrastructure team to deploy new images.

Resolves #62809.

Release note (general change): Upgrade the CRDB binary to Go 1.15.10.

62823: schemaexpr: remove inapplicable TODO r=mgartner a=mgartner

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

WORKSPACE

@@ -98,7 +98,7 @@ load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_depe
 
 go_rules_dependencies()
 
-go_register_toolchains(go_version = "1.15.6")
+go_register_toolchains(go_version = "1.15.10")
 
 # Configure nodeJS.
 load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

build/README.md

@@ -80,7 +80,7 @@ Please copy this checklist (based on [Basic Process](#basic-process)) into the r
 back to this document and perform these steps:
 
 * [ ] Adjust the Pebble tests to run in new version.
-* [ ] Adjust version in Docker image ([source](./builder/Dockerfile#L199-L200)).
+* [ ] Adjust version in Docker image ([source](./builder/Dockerfile)).
 * [ ] Rebuild and push the Docker image (following [Basic Process](#basic-process))
 * [ ] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
 * [ ] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).

build/bootstrap/bootstrap-debian.sh

@@ -46,9 +46,9 @@ sudo tar -C /usr -zxf /tmp/cmake.tgz && rm /tmp/cmake.tgz
 
 # Install Go.
 trap 'rm -f /tmp/go.tgz' EXIT
-curl -fsSL https://dl.google.com/go/go1.15.6.linux-amd64.tar.gz > /tmp/go.tgz
+curl -fsSL https://dl.google.com/go/go1.15.10.linux-amd64.tar.gz > /tmp/go.tgz
 sha256sum -c - <<EOF
-3918e6cc85e7eaaa6f859f1bdbaac772e7a825b0eb423c63d3ae68b21f84b844 /tmp/go.tgz
+4aa1267517df32f2bf1cc3d55dfc27d0c6b2c2b0989449c96dd19273ccca051d /tmp/go.tgz
 EOF
 sudo tar -C /usr/local -zxf /tmp/go.tgz && rm /tmp/go.tgz
 

build/builder.sh

@@ -3,7 +3,7 @@
 set -euo pipefail
 
 image=cockroachdb/builder
-version=20210205-000935
+version=20210330-194147
 
 function init() {
   docker build --tag="${image}" "$(dirname "${0}")/builder"

build/builder/Dockerfile

@@ -235,8 +235,8 @@ RUN curl -fsSL https://github.com/Kitware/CMake/releases/download/v3.17.0/cmake-
 # releases of Go will no longer be run in CI once it is changed. Consider
 # bumping the minimum allowed version of Go in /build/go-version-chech.sh.
 RUN DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends golang \
- && curl -fsSL https://storage.googleapis.com/golang/go1.15.6.src.tar.gz -o golang.tar.gz \
- && echo '890bba73c5e2b19ffb1180e385ea225059eb008eb91b694875dd86ea48675817 golang.tar.gz' | sha256sum -c - \
+ && curl -fsSL https://storage.googleapis.com/golang/go1.15.10.src.tar.gz -o golang.tar.gz \
+ && echo 'c1dbca6e0910b41d61a95bf9878f6d6e93d15d884c226b91d9d4b1113c10dd65 golang.tar.gz' | sha256sum -c - \
  && tar -C /usr/local -xzf golang.tar.gz \
  && rm golang.tar.gz \
  && cd /usr/local/go/src \

build/packer/teamcity-agent.sh

@@ -20,8 +20,6 @@ apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 0EBFCD88
 cat > /etc/apt/sources.list.d/docker.list <<EOF
 deb https://download.docker.com/linux/ubuntu bionic stable
 EOF
-# Per https://github.com/golang/go/wiki/Ubuntu
-add-apt-repository ppa:longsleep/golang-backports
 # Git 2.7, which ships with Xenial, has a bug where submodule metadata sometimes
 # uses absolute paths instead of relative paths, which means the affected
 # submodules cannot be mounted in Docker containers. Use the latest version of
@@ -32,25 +30,26 @@ apt-get update --yes
 # Install the sudo version patched for CVE-2021-3156
 apt-get install --yes sudo
 
-# Install the necessary dependencies. Keep this list small!
-GO_VERSION=1.15
-
 apt-get install --yes \
+  curl \
   docker-ce \
   docker-compose \
   gnome-keyring \
   gnupg2 \
   git \
-  golang-${GO_VERSION}-go \
   openjdk-11-jre-headless \
   pass \
   unzip
 
-# golang-X.Y-go does not install system wide symlinks, it's only done by the
-# golang-go package which points to the latest version. Explicitly symlink the
-# pinned version to /usr/bin.
-for f in go gofm; do
-    ln -s /usr/lib/go-${GO_VERSION}/bin/$f /usr/bin
+curl -fsSL https://dl.google.com/go/go1.15.10.linux-amd64.tar.gz > /tmp/go.tgz
+sha256sum -c - <<EOF
+4aa1267517df32f2bf1cc3d55dfc27d0c6b2c2b0989449c96dd19273ccca051d /tmp/go.tgz
+EOF
+tar -C /usr/local -zxf /tmp/go.tgz && rm /tmp/go.tgz
+
+# Explicitly symlink the pinned version to /usr/bin.
+for f in `ls /usr/local/go/bin`; do
+    ln -s /usr/local/go/bin/$f /usr/bin
 done
 
 # Installing gnome-keyring prevents the error described in
@@ -107,6 +106,8 @@ do
   git clean -dxf
 
   git checkout "$branch"
+  # Stupid submodules.
+  rm -rf vendor; git checkout vendor; git submodule update --init --recursive
   COCKROACH_BUILDER_CCACHE=1 build/builder.sh make test testrace TESTTIMEOUT=45m TESTS=-
   # TODO(benesch): store the acceptanceversion somewhere more accessible.
   docker pull $(git grep cockroachdb/acceptance -- '*.go' | sed -E 's/.*"([^"]*).*"/\1/') || true

pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1

@@ -26,3 +26,21 @@ CREATE TABLE t()
 
 statement error cannot alter a table's LOCALITY if its database is not multi-region enabled
 ALTER TABLE t SET LOCALITY REGIONAL BY ROW
+
+statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING global_reads = true
+
+statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING global_reads = false
+
+statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING num_voters = 3
+
+statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING num_voters = 0
+
+statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING voter_constraints = '[+region=us-east-1]'
+
+statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING voter_constraints = ''

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -1126,3 +1126,32 @@ ALTER PARTITION p3 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
   gc.ttlseconds = 15411;
 ALTER PARTITION p4 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
   gc.ttlseconds = 15418
+
+# Test that the global_reads attribute can be set with an enterprise license.
+# This same test fails in pkg/sql/logictest/testdata/logic_test/zone_config.
+statement ok
+CREATE TABLE global (id INT PRIMARY KEY)
+
+statement ok
+ALTER TABLE global CONFIGURE ZONE USING global_reads = true
+
+# This should reflect in the metrics.
+query T
+SELECT feature_name FROM crdb_internal.feature_usage
+WHERE feature_name IN (
+  'sql.schema.zone_config.table.global_reads'
+) AND usage_count > 0 ORDER BY feature_name
+----
+sql.schema.zone_config.table.global_reads
+
+query TT
+SHOW ZONE CONFIGURATION FOR TABLE global
+----
+TABLE global  ALTER TABLE global CONFIGURE ZONE USING
+              range_min_bytes = 134217728,
+              range_max_bytes = 536870912,
+              gc.ttlseconds = 90000,
+              global_reads = true,
+              num_replicas = 7,
+              constraints = '[]',
+              lease_preferences = '[]'

pkg/ccl/multiregionccl/multiregion_test.go

@@ -117,3 +117,46 @@ func TestMultiRegionAfterEnterpriseDisabled(t *testing.T) {
 		}
 	})
 }
+
+func TestGlobalReadsAfterEnterpriseDisabled(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+	defer utilccl.TestingEnableEnterprise()()
+
+	_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
+		t, 1 /* numServers */, base.TestingKnobs{}, nil, /* baseDir */
+	)
+	defer cleanup()
+
+	for _, setupQuery := range []string{
+		`CREATE DATABASE test`,
+		`USE test`,
+		`CREATE TABLE t1 ()`,
+		`CREATE TABLE t2 ()`,
+	} {
+		_, err := sqlDB.Exec(setupQuery)
+		require.NoError(t, err)
+	}
+
+	// Can set global_reads with enterprise license enabled.
+	_, err := sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
+	require.NoError(t, err)
+
+	_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = true`)
+	require.NoError(t, err)
+
+	// Can unset global_reads with enterprise license enabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = false`)
+	require.NoError(t, err)
+
+	defer utilccl.TestingDisableEnterprise()()
+
+	// Cannot set global_reads with enterprise license disabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
+	require.Error(t, err)
+	require.Regexp(t, "use of global_reads requires an enterprise license", err)
+
+	// Can unset global_reads with enterprise license disabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = false`)
+	require.NoError(t, err)
+}

pkg/geo/geomfn/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "make_geometry.go",
         "node.go",
         "orientation.go",
+        "point_polygon_optimization.go",
         "remove_repeated_points.go",
         "reverse.go",
         "segmentize.go",

pkg/geo/geomfn/binary_predicates.go

@@ -36,7 +36,7 @@ func Covers(a geo.Geometry, b geo.Geometry) (bool, error) {
 	case PolygonAndPoint:
 		// Computing whether a polygon covers a point is equivalent
 		// to computing whether the point is covered by the polygon.
-		return PointKindRelatesToPolygonKind(pointKind, polygonKind, PointPolygonCoveredBy)
+		return PointKindCoveredByPolygonKind(pointKind, polygonKind)
 	}
 
 	return geos.Covers(a.EWKB(), b.EWKB())
@@ -58,7 +58,7 @@ func CoveredBy(a geo.Geometry, b geo.Geometry) (bool, error) {
 		// A polygon cannot be covered by a point.
 		return false, nil
 	case PointAndPolygon:
-		return PointKindRelatesToPolygonKind(pointKind, polygonKind, PointPolygonCoveredBy)
+		return PointKindCoveredByPolygonKind(pointKind, polygonKind)
 	}
 
 	return geos.CoveredBy(a.EWKB(), b.EWKB())
@@ -82,7 +82,7 @@ func Contains(a geo.Geometry, b geo.Geometry) (bool, error) {
 	case PolygonAndPoint:
 		// Computing whether a polygon contains a point is equivalent
 		// to computing whether the point is contained within the polygon.
-		return PointKindRelatesToPolygonKind(pointKind, polygonKind, PointPolygonWithin)
+		return PointKindWithinPolygonKind(pointKind, polygonKind)
 	}
 
 	return geos.Contains(a.EWKB(), b.EWKB())
@@ -148,7 +148,7 @@ func Intersects(a geo.Geometry, b geo.Geometry) (bool, error) {
 	pointPolygonPair, pointKind, polygonKind := PointKindAndPolygonKind(a, b)
 	switch pointPolygonPair {
 	case PointAndPolygon, PolygonAndPoint:
-		return PointKindRelatesToPolygonKind(pointKind, polygonKind, PointPolygonIntersects)
+		return PointKindIntersectsPolygonKind(pointKind, polygonKind)
 	}
 
 	return geos.Intersects(a.EWKB(), b.EWKB())
@@ -335,7 +335,7 @@ func Within(a geo.Geometry, b geo.Geometry) (bool, error) {
 		// A polygon cannot be contained within a point.
 		return false, nil
 	case PointAndPolygon:
-		return PointKindRelatesToPolygonKind(pointKind, polygonKind, PointPolygonWithin)
+		return PointKindWithinPolygonKind(pointKind, polygonKind)
 	}
 
 	return geos.Within(a.EWKB(), b.EWKB())

pkg/geo/geomfn/distance.go

@@ -635,7 +635,7 @@ func findPointSideOfLinearRing(point geodist.Point, linearRing geodist.LinearRin
 	// See also: https://en.wikipedia.org/wiki/Nonzero-rule
 	windingNumber := 0
 	p := point.GeomPoint
-	for edgeIdx := 0; edgeIdx < linearRing.NumEdges(); edgeIdx++ {
+	for edgeIdx, numEdges := 0, linearRing.NumEdges(); edgeIdx < numEdges; edgeIdx++ {
 		e := linearRing.Edge(edgeIdx)
 		eV0 := e.V0.GeomPoint
 		eV1 := e.V1.GeomPoint
@@ -836,125 +836,6 @@ func verifyDensifyFrac(f float64) error {
 	return nil
 }
 
-// PointPolygonRelationType defines a relationship type between
-// a (multi)point and a (multi)polygon.
-type PointPolygonRelationType int
-
-const (
-	// PointPolygonIntersects is the relationship where a (multi)point
-	// intersects a (multi)polygon.
-	PointPolygonIntersects PointPolygonRelationType = iota + 1
-	// PointPolygonCoveredBy is the relationship where a (multi)point
-	// is covered by a (multi)polygon.
-	PointPolygonCoveredBy
-	// PointPolygonWithin is the relationship where a (multi)point
-	// is contained within a (multi)polygon.
-	PointPolygonWithin
-)
-
-// PointKindRelatesToPolygonKind returns whether a (multi)point and
-// a (multi)polygon have the given relationship.
-func PointKindRelatesToPolygonKind(
-	pointKind geo.Geometry, polygonKind geo.Geometry, relationType PointPolygonRelationType,
-) (bool, error) {
-	pointKindBaseT, err := pointKind.AsGeomT()
-	if err != nil {
-		return false, err
-	}
-	polygonKindBaseT, err := polygonKind.AsGeomT()
-	if err != nil {
-		return false, err
-	}
-	pointKindIterator := geo.NewGeomTIterator(pointKindBaseT, geo.EmptyBehaviorOmit)
-	polygonKindIterator := geo.NewGeomTIterator(polygonKindBaseT, geo.EmptyBehaviorOmit)
-
-	// TODO(ayang): Think about how to refactor these nested for loops
-	// Check whether each point intersects with at least one polygon.
-	// - For Intersects, at least one point must intersect with at least one polygon.
-	// - For CoveredBy, every point must intersect with at least one polygon.
-	// - For Within, every point must intersect with at least one polygon
-	//   and at least one point must be inside at least one polygon.
-	intersectsOnce := false
-	insideOnce := false
-pointOuterLoop:
-	for {
-		point, hasPoint, err := pointKindIterator.Next()
-		if err != nil {
-			return false, err
-		}
-		if !hasPoint {
-			break
-		}
-		// Reset the polygon iterator on each iteration of the point iterator.
-		polygonKindIterator.Reset()
-		curIntersects := false
-		for {
-			polygon, hasPolygon, err := polygonKindIterator.Next()
-			if err != nil {
-				return false, err
-			}
-			if !hasPolygon {
-				break
-			}
-			pointSide, err := findPointSideOfPolygon(point, polygon)
-			if err != nil {
-				return false, err
-			}
-			switch pointSide {
-			case insideLinearRing:
-				insideOnce = true
-				switch relationType {
-				case PointPolygonWithin:
-					continue pointOuterLoop
-				}
-				fallthrough
-			case onLinearRing:
-				intersectsOnce = true
-				curIntersects = true
-				switch relationType {
-				case PointPolygonIntersects:
-					// A single intersection is sufficient.
-					return true, nil
-				case PointPolygonCoveredBy:
-					// If the current point intersects, check the next point.
-					continue pointOuterLoop
-				case PointPolygonWithin:
-					// We can only skip to the next point if we have already seen a point
-					// that is inside the (multi)polygon.
-					if insideOnce {
-						continue pointOuterLoop
-					}
-				default:
-					return false, errors.Newf("unknown PointPolygonRelationType")
-				}
-			case outsideLinearRing:
-			default:
-				return false, errors.Newf("findPointSideOfPolygon returned unknown linearRingSide %d", pointSide)
-			}
-		}
-		// Case where a point in the (multi)point does not intersect
-		// a polygon in the (multi)polygon.
-		switch relationType {
-		case PointPolygonCoveredBy:
-			// Each point in a (multi)point must intersect a polygon in the
-			// (multi)point to be covered by it.
-			return false, nil
-		case PointPolygonWithin:
-			if !curIntersects {
-				return false, nil
-			}
-		}
-	}
-	switch relationType {
-	case PointPolygonCoveredBy:
-		return intersectsOnce, nil
-	case PointPolygonWithin:
-		return insideOnce, nil
-	default:
-		return false, nil
-	}
-}
-
 // findPointSideOfPolygon returns whether a point intersects with a polygon.
 func findPointSideOfPolygon(point geom.T, polygon geom.T) (linearRingSide, error) {
 	// Convert point from a geom.T to a *geodist.Point.