Merge #66811 #66886 #66921 #66957

66811: clusterversion: make some room for a v21.1PLUS "release" r=RaduBerinde a=RaduBerinde

The summer serverless offering requires an "in-between" release which
is v21.1.x plus multitenant-specific functionality. We will call this
the v21.1PLUS release. It has not yet been decided if v21.1PLUS
changes will become part of the 21.1.x branch or whether it will live
on its own branch.

This commit shifts cluster versions around to make some space for
migrations specific to this release:
 - we shift existing v21.2 versions (Major=21, Minor=1, Internal=2
   to Internal=102 through 112. Consequently, any existing cluster
   running `master` will rerun all the 21.2 migrations (which is ok
   since they are required to be idempotent).

 - we start a new chunk of v21.1PLUS versions at Major=21, Minor=1,
   Internal=14. We start at a value higher than the current master to
   avoid any confusion on clusters running master.

Any new v21.1PLUS feature that needs a migration will be developed on
master and will add a v21.2 version, e.g.
```
   {
     Key:     DeleteDeprecatedNamespaceTableDescriptorMigration,
     Version: roachpb.Version{Major: 21, Minor: 1, Internal: 112},
   },
+  {
+    Key:     SomeServerlessSpecificFeature,
+    Version: roachpb.Version{Major: 21, Minor: 1, Internal: 114},
+  },
```

When that change is backported to v21.1PLUS, this will become a
v21.2PLUS version:
```
   {
     Key: Start21_1PLUS,
     Version: roachpb.Version{Major: 21, Minor: 1, Internal: 14},
   },
+  {
+    Key:     SomeServerlessSpecificFeature,
+    Version: roachpb.Version{Major: 21, Minor: 1, Internal: 16},
+  },
```

Release note: None

66886: admission: optimizations to admission.WorkQueue r=sumeerbhola a=sumeerbhola

These small optimizations reduce memory allocations and
the length of the critical sections.

A 10s mutex profile of kv50 with cpu overload had
~1.2s (of a total of 2s) in the admission package.
This was correlated with what showed up in the
corresponding cpu profile, which directed these
changes.

Release note: None

66921: build: add script for build config to cross-compile to macos r=rail a=rickystewart

Also generalize some scripts that were unnecessarily repetitive.

Closes #66210

Release note: None

66957: Add dev-inf as codeowners for /build/ r=rail,rickystewart a=jlinder

Before: no owners were set for /build/

Why: dev-inf owns /build/

Now: dev-inf is set as code owners for /build/

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: James H. Linder <[email protected]>

.github/CODEOWNERS

@@ -19,6 +19,8 @@
 
 /.github/                    @cockroachdb/dev-inf
 
+/build/                      @cockroachdb/dev-inf
+
 /docs/RFCS/                  @cockroachdb/rfc-prs
 
 /Makefile                    @cockroachdb/dev-inf

build/bazelutil/bazelbuild.sh

@@ -2,8 +2,14 @@
 
 set -xeuo pipefail
 
+if [ -z "$1" ]
+then
+    echo 'Usage: bazelbuild.sh CONFIG'
+    exit 1
+fi
+
 bazel build //pkg/cmd/bazci --config=ci
 $(bazel info bazel-bin)/pkg/cmd/bazci/bazci_/bazci --compilation_mode opt \
 		       --config ci \
-		       --config crosslinux \
+		       --config "$1" \
 		       build //pkg/cmd/cockroach-short

build/teamcity-bazel-macos-build.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+source "$(dirname "${0}")/teamcity-support.sh"  # For $root
+source "$(dirname "${0}")/teamcity-bazel-support.sh"  # For run_bazel
+
+tc_prepare
+
+tc_start_block "Run Bazel build"
+run_bazel build/bazelutil/bazelbuild.sh crossmacos
+tc_end_block "Run Bazel build"

build/teamcity-bazel-windows-build.sh

@@ -8,5 +8,5 @@ source "$(dirname "${0}")/teamcity-bazel-support.sh"  # For run_bazel
 tc_prepare
 
 tc_start_block "Run Bazel build"
-run_bazel build/bazelutil/bazelbuildwindows.sh
+run_bazel build/bazelutil/bazelbuild.sh crosswindows
 tc_end_block "Run Bazel build"

build/teamcity-bazel.sh

@@ -8,5 +8,5 @@ source "$(dirname "${0}")/teamcity-bazel-support.sh"  # For run_bazel
 tc_prepare
 
 tc_start_block "Run Bazel build"
-run_bazel build/bazelutil/bazelbuild.sh
+run_bazel build/bazelutil/bazelbuild.sh crosslinux
 tc_end_block "Run Bazel build"

docs/generated/settings/settings-for-tenants.txt

@@ -111,4 +111,4 @@ trace.datadog.project	string	CockroachDB	the project under which traces will be
 trace.debug.enable	boolean	false	if set, traces for recent requests can be seen at https://<ui>/debug/requests
 trace.lightstep.token	string		if set, traces go to Lightstep using this token
 trace.zipkin.collector	string		if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.
-version	version	21.1-12	set the active cluster version in the format '<major>.<minor>'
+version	version	21.1-112	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -113,6 +113,6 @@
 <tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen at https://<ui>/debug/requests</td></tr>
 <tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.</td></tr>
-<tr><td><code>version</code></td><td>version</td><td><code>21.1-12</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
+<tr><td><code>version</code></td><td>version</td><td><code>21.1-112</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -282,6 +282,10 @@ const (
 	// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
 	V21_1
 
+	// v21.1PLUS release. This is a special v21.1.x release with extra changes,
+	// used internally for the 2021 serverless offering.
+	Start21_1PLUS
+
 	// v21.2 versions.
 	//
 	// Start21_2 demarcates work towards CockroachDB v21.2.
@@ -321,7 +325,7 @@ const (
 // Such clusters would need to be wiped. As a result, do not bump the major or
 // minor version until we are absolutely sure that no new migrations will need
 // to be added (i.e., when cutting the final release candidate).
-var versionsSingleton = keyedVersions([]keyedVersion{
+var versionsSingleton = keyedVersions{
 
 	// v20.2 versions.
 	{
@@ -476,34 +480,46 @@ var versionsSingleton = keyedVersions([]keyedVersion{
 		Version: roachpb.Version{Major: 21, Minor: 1},
 	},
 
+	// v21.1PLUS version. This is a special v21.1.x release with extra changes,
+	// used internally for the 2021 Serverless offering.
+	//
+	// Any v21.1PLUS change that needs a migration will have a v21.2 version on
+	// master but a v21.1PLUS version on the v21.1PLUS branch.
+	{
+		Key: Start21_1PLUS,
+		// The Internal version starts out at 14 for historic reasons: at the time
+		// this was added, v21.2 versions were already defined up to 12.
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 14},
+	},
+
 	// v21.2 versions.
 	{
 		Key:     Start21_2,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 2},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 102},
 	},
 	{
 		Key:     JoinTokensTable,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 4},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 104},
 	},
 	{
 		Key:     AcquisitionTypeInLeaseHistory,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 6},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 106},
 	},
 	{
 		Key:     SerializeViewUDTs,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 8},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 108},
 	},
 	{
 		Key:     ExpressionIndexes,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 10},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 110},
 	},
 	{
 		Key:     DeleteDeprecatedNamespaceTableDescriptorMigration,
-		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 12},
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 112},
 	},
 
 	// Step (2): Add new versions here.
-})
+}
 
 // TODO(irfansharif): clusterversion.binary{,MinimumSupported}Version
 // feels out of place. A "cluster version" and a "binary version" are two

pkg/util/admission/work_queue.go

@@ -15,6 +15,7 @@ import (
 	"context"
 	"math"
 	"sort"
+	"sync"
 	"sync/atomic"
 	"time"
 
@@ -188,9 +189,6 @@ func makeWorkQueue(
 	return q
 }
 
-// TODO(sumeer): reduce allocations by using a pool for tenantInfo, waitingWork,
-// waitingWork.ch.
-
 // Admit is called when requesting admission for some work. If err!=nil, the
 // request was not admitted, potentially due to the deadline being exceeded.
 // The enabled return value is relevant when err=nil, and represents whether
@@ -212,7 +210,7 @@ func (q *WorkQueue) Admit(ctx context.Context, info WorkInfo) (enabled bool, err
 	q.mu.Lock()
 	tenant, ok := q.mu.tenants[tenantID]
 	if !ok {
-		tenant = makeTenantInfo(tenantID)
+		tenant = newTenantInfo(tenantID)
 		q.mu.tenants[tenantID] = tenant
 	}
 	if info.BypassAdmission && roachpb.IsSystemTenantID(tenantID) && q.workKind == KVWork {
@@ -279,26 +277,32 @@ func (q *WorkQueue) Admit(ctx context.Context, info WorkInfo) (enabled bool, err
 		}
 	}
 	// Push onto heap(s).
-	work := makeWaitingWork(info.Priority, info.CreateTime)
+	work := newWaitingWork(info.Priority, info.CreateTime)
 	heap.Push(&tenant.waitingWorkHeap, work)
 	if len(tenant.waitingWorkHeap) == 1 {
 		heap.Push(&q.mu.tenantHeap, tenant)
 	}
 	// Else already in tenantHeap.
-	q.metrics.WaitQueueLength.Inc(1)
 
 	// Release all locks and start waiting.
 	q.mu.Unlock()
 	q.admitMu.Unlock()
+
+	q.metrics.WaitQueueLength.Inc(1)
+	defer releaseWaitingWork(work)
 	select {
 	case <-doneCh:
 		q.mu.Lock()
 		if work.heapIndex == -1 {
 			// No longer in heap. Raced with token/slot grant.
-			chainID := <-work.ch
 			tenant.used--
 			q.mu.Unlock()
 			q.granter.returnGrant()
+			// The channel is sent to after releasing mu, so we don't need to hold
+			// mu when receiving from it. Additionally, we've already called
+			// returnGrant so we're not holding back future grant chains if this one
+			// chain gets terminated.
+			chainID := <-work.ch
 			q.granter.continueGrantChain(chainID)
 		} else {
 			tenant.waitingWorkHeap.remove(work)
@@ -366,21 +370,25 @@ func (q *WorkQueue) hasWaitingRequests() bool {
 }
 
 func (q *WorkQueue) granted(grantChainID grantChainID) bool {
+	// Reduce critical section by getting time before mutex acquisition.
+	now := timeutil.Now()
 	q.mu.Lock()
-	defer q.mu.Unlock()
 	if len(q.mu.tenantHeap) == 0 {
+		q.mu.Unlock()
 		return false
 	}
 	tenant := q.mu.tenantHeap[0]
 	item := heap.Pop(&tenant.waitingWorkHeap).(*waitingWork)
-	item.grantTime = timeutil.Now()
-	item.ch <- grantChainID
+	item.grantTime = now
 	tenant.used++
 	if len(tenant.waitingWorkHeap) > 0 {
 		q.mu.tenantHeap.fix(tenant)
 	} else {
 		q.mu.tenantHeap.remove(tenant)
 	}
+	q.mu.Unlock()
+	// Reduce critical section by sending on channel after releasing mutex.
+	item.ch <- grantChainID
 	return true
 }
 
@@ -393,6 +401,7 @@ func (q *WorkQueue) gcTenantsAndResetTokens() {
 	for id, info := range q.mu.tenants {
 		if info.used == 0 && len(info.waitingWorkHeap) == 0 {
 			delete(q.mu.tenants, id)
+			releaseTenantInfo(info)
 		}
 		if q.usesTokens {
 			info.used = 0
@@ -456,8 +465,36 @@ type tenantHeap []*tenantInfo
 
 var _ heap.Interface = (*tenantHeap)(nil)
 
-func makeTenantInfo(id uint64) *tenantInfo {
-	return &tenantInfo{id: id, heapIndex: -1}
+var tenantInfoPool = sync.Pool{
+	New: func() interface{} {
+		return &tenantInfo{}
+	},
+}
+
+func newTenantInfo(id uint64) *tenantInfo {
+	ti := tenantInfoPool.Get().(*tenantInfo)
+	*ti = tenantInfo{
+		id:              id,
+		waitingWorkHeap: ti.waitingWorkHeap,
+		heapIndex:       -1,
+	}
+	return ti
+}
+
+func releaseTenantInfo(ti *tenantInfo) {
+	if len(ti.waitingWorkHeap) != 0 {
+		panic("tenantInfo has non-empty heap")
+	}
+	// NB: waitingWorkHeap.Pop nils the slice elements when removing, so we are
+	// not inadvertently holding any references.
+	if cap(ti.waitingWorkHeap) > 100 {
+		ti.waitingWorkHeap = nil
+	}
+	wwh := ti.waitingWorkHeap
+	*ti = tenantInfo{
+		waitingWorkHeap: wwh,
+	}
+	tenantInfoPool.Put(ti)
 }
 
 func (th *tenantHeap) fix(item *tenantInfo) {
@@ -519,13 +556,39 @@ type waitingWorkHeap []*waitingWork
 
 var _ heap.Interface = (*waitingWorkHeap)(nil)
 
-func makeWaitingWork(priority WorkPriority, createTime int64) *waitingWork {
-	return &waitingWork{
+var waitingWorkPool = sync.Pool{
+	New: func() interface{} {
+		return &waitingWork{}
+	},
+}
+
+func newWaitingWork(priority WorkPriority, createTime int64) *waitingWork {
+	ww := waitingWorkPool.Get().(*waitingWork)
+	ch := ww.ch
+	if ch == nil {
+		ch = make(chan grantChainID, 1)
+	}
+	*ww = waitingWork{
 		priority:   priority,
 		createTime: createTime,
-		ch:         make(chan grantChainID, 1),
+		ch:         ch,
 		heapIndex:  -1,
 	}
+	return ww
+}
+
+// releaseWaitingWork must be called with an empty waitingWork.ch.
+func releaseWaitingWork(ww *waitingWork) {
+	ch := ww.ch
+	select {
+	case <-ch:
+		panic("channel must be empty and not closed")
+	default:
+	}
+	*ww = waitingWork{
+		ch: ch,
+	}
+	waitingWorkPool.Put(ww)
 }
 
 func (wwh *waitingWorkHeap) remove(item *waitingWork) {

pkg/util/admission/work_queue_test.go

@@ -248,3 +248,5 @@ func scanTenantID(t *testing.T, d *datadriven.TestData) roachpb.TenantID {
 // - Test metrics
 // - Test race between grant and cancellation
 // - Test WorkQueue for tokens
+// - Add microbenchmark with high concurrency and procs for full admission
+//   system