Merge #91461 #91681 #91860 #91951 #92073

91461: allocator: refactor StorePool usage to interface r=AlexTalks a=AlexTalks

This change refactors the usage of `StorePool` in the allocator to a new interface, `AllocatorStorePool`, in order to be able to utilize a store pool with overriden liveness to properly evaluate decommission pre-flight checks.

Part of #91570.

Release note: None

91681: sql/opt: Support oid type in foldStringToRegclassCast r=rafiss a=e-mbrown

Informs: #91022

This commit adds allows queries like
`select * from pg_class where oid ='my_table'::regclass::oid` to use index on `pg_class(oid)`.

Release note: None

91860: ui: rename "active" execution types and functions to "recent" r=amyyq2 a=amyyq2

This change renames the active execution types and functions to "recent" types and functions. This is in preparation for the addition of recent executions to the current active executions table.

The file names will be changed in a later PR.

Release note: None

91951: kvserver,logstore: move log appends to logstore r=tbg a=pavelkalinnikov

This change moves a few components related to storing the newly appended Raft entries from `kvserver` to the `logstore` package.

Part of #91979
Release note: None

92073: ci: use GCS instead of S3 to download binaries r=rickystewart a=rail

Previously, CI used S3 to download the binaries. Now that we are moving the primary location to GCS and will stop uploading to s3 at some point, it's time to start using GCS for this operation.

Part of RE-342
Release note: None

Co-authored-by: Alex Sarkesian <[email protected]>
Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: Amy Qian <[email protected]>
Co-authored-by: amyyq2 <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>

build/teamcity/internal/release/process/make-and-publish-build-artifacts.sh

@@ -33,6 +33,7 @@ else
   # export the variable to avoid shell escaping
   export gcs_credentials="$GCS_CREDENTIALS_DEV"
 fi
+download_prefix="https://storage.googleapis.com/$gcs_bucket"
 
 cat << EOF
 
@@ -88,7 +89,7 @@ for platform_name in "${platform_names[@]}"; do
     --silent \
     --show-error \
     --output /dev/stdout \
-    --url "https://${bucket}.s3.amazonaws.com/cockroach-${build_name}.${linux_platform}-${tarball_arch}.tgz" \
+    --url "${download_prefix}/cockroach-${build_name}.${linux_platform}-${tarball_arch}.tgz" \
     | tar \
     --directory="build/deploy-${docker_arch}" \
     --extract \

build/teamcity/internal/release/process/publish-cockroach-release.sh

@@ -37,7 +37,6 @@ if [[ -z "${DRY_RUN}" ]] ; then
   gcr_repository="us-docker.pkg.dev/cockroach-cloud-images/cockroachdb/cockroach"
   # Used for docker login for gcloud
   gcr_hostname="us-docker.pkg.dev"
-  s3_download_hostname="${bucket}"
   git_repo_for_tag="cockroachdb/cockroach"
 else
   bucket="cockroach-builds-test"
@@ -48,7 +47,6 @@ else
   dockerhub_repository="docker.io/cockroachdb/cockroach-misc"
   gcr_repository="us.gcr.io/cockroach-release/cockroach-test"
   gcr_hostname="us.gcr.io"
-  s3_download_hostname="${bucket}.s3.amazonaws.com"
   git_repo_for_tag="cockroachlabs/release-staging"
   if [[ -z "$(echo ${build_name} | grep -E -o '^v[0-9]+\.[0-9]+\.[0-9]+$')" ]] ; then
     # Using `.` to match how we usually format the pre-release portion of the
@@ -62,6 +60,7 @@ else
   fi
 fi
 
+download_prefix="https://storage.googleapis.com/$gcs_bucket"
 tc_end_block "Variable Setup"
 
 
@@ -119,7 +118,7 @@ for platform_name in "${platform_names[@]}"; do
     --silent \
     --show-error \
     --output /dev/stdout \
-    --url "https://${s3_download_hostname}/cockroach-${build_name}.${linux_platform}-${tarball_arch}.tgz" \
+    --url "${download_prefix}/cockroach-${build_name}.${linux_platform}-${tarball_arch}.tgz" \
     | tar \
     --directory="build/deploy-${docker_arch}" \
     --extract \

build/teamcity/internal/release/process/roachtest-release-qualification.sh

@@ -24,10 +24,8 @@ fi
 artifacts=$PWD/artifacts/$(date +"%%Y%%m%%d")-${TC_BUILD_ID}
 mkdir -p "$artifacts"
 
-bucket="${BUCKET-cockroach-builds}"
-
 release_version=$(echo $TC_BUILD_BRANCH | sed -e 's/provisional_[[:digit:]]*_//')
-curl -f -s -S -o- "https://${bucket}.s3.amazonaws.com/cockroach-${release_version}.linux-amd64.tgz" | tar ixfz - --strip-components 1
+curl -f -s -S -o- "https://storage.googleapis.com/cockroach-builds-artifacts-prod/cockroach-${release_version}.linux-amd64.tgz" | tar ixfz - --strip-components 1
 chmod +x cockroach
 
 run_bazel <<'EOF'

pkg/cmd/release/sentry.go

@@ -21,7 +21,7 @@ func generatePanic(tag string) error {
 	// the call usually exits non-zero on panic, but we don't need to fail in that case, thus "|| true"
 	// TODO: do not hardcode the URL
 	script := fmt.Sprintf(`
-curl https://cockroach-builds.s3.amazonaws.com/cockroach-%s.linux-amd64.tgz | tar -xz
+curl https://binaries.cockroachdb.com/cockroach-%s.linux-amd64.tgz | tar -xz
 ./cockroach-%s.linux-amd64/cockroach demo --insecure -e "select crdb_internal.force_panic('testing')" || true
 `, tag, tag)
 	cmd := exec.Command("bash", "-c", script)

pkg/kv/kvserver/allocation_op.go

@@ -27,7 +27,7 @@ type AllocationOp interface {
 	trackPlanningMetrics()
 	// applyImpact updates the given storepool to reflect the result of
 	// applying this operation.
-	applyImpact(storepool *storepool.StorePool)
+	applyImpact(storepool storepool.AllocatorStorePool)
 	// lhBeingRemoved returns true when the leaseholder is will be removed if
 	// this operation succeeds, otherwise false.
 	lhBeingRemoved() bool
@@ -49,7 +49,7 @@ func (o AllocationTransferLeaseOp) lhBeingRemoved() bool {
 	return true
 }
 
-func (o AllocationTransferLeaseOp) applyImpact(storepool *storepool.StorePool) {
+func (o AllocationTransferLeaseOp) applyImpact(storepool storepool.AllocatorStorePool) {
 	// TODO(kvoli): Currently the local storepool is updated directly in the
 	// lease transfer call, rather than in this function. Move the storepool
 	// tracking from rq.TransferLease to this function once #89771 is merged.
@@ -89,7 +89,7 @@ func (o AllocationChangeReplicasOp) lhBeingRemoved() bool {
 
 // applyEstimatedImpact updates the given storepool to reflect the result
 // of applying this operation.
-func (o AllocationChangeReplicasOp) applyImpact(storepool *storepool.StorePool) {
+func (o AllocationChangeReplicasOp) applyImpact(storepool storepool.AllocatorStorePool) {
 	for _, chg := range o.chgs {
 		storepool.UpdateLocalStoreAfterRebalance(chg.Target.StoreID, o.usage, chg.ChangeType)
 	}
@@ -109,16 +109,16 @@ type AllocationFinalizeAtomicReplicationOp struct{}
 
 // TODO(kvoli): This always returns false, however it is possible that the LH
 // may have been removed here.
-func (o AllocationFinalizeAtomicReplicationOp) lhBeingRemoved() bool                       { return false }
-func (o AllocationFinalizeAtomicReplicationOp) applyImpact(storepool *storepool.StorePool) {}
-func (o AllocationFinalizeAtomicReplicationOp) trackPlanningMetrics()                      {}
+func (o AllocationFinalizeAtomicReplicationOp) lhBeingRemoved() bool                               { return false }
+func (o AllocationFinalizeAtomicReplicationOp) applyImpact(storepool storepool.AllocatorStorePool) {}
+func (o AllocationFinalizeAtomicReplicationOp) trackPlanningMetrics()                              {}
 
 // AllocationNoop represents no operation.
 type AllocationNoop struct{}
 
-func (o AllocationNoop) lhBeingRemoved() bool                       { return false }
-func (o AllocationNoop) applyImpact(storepool *storepool.StorePool) {}
-func (o AllocationNoop) trackPlanningMetrics()                      {}
+func (o AllocationNoop) lhBeingRemoved() bool                               { return false }
+func (o AllocationNoop) applyImpact(storepool storepool.AllocatorStorePool) {}
+func (o AllocationNoop) trackPlanningMetrics()                              {}
 
 // effectBuilder is a utility struct to track a list of effects, which may be
 // used to construct a single effect function that in turn calls all tracked

pkg/kv/kvserver/allocator/allocatorimpl/allocator.go

@@ -477,7 +477,8 @@ type AllocatorMetrics struct {
 // Allocator tries to spread replicas as evenly as possible across the stores
 // in the cluster.
 type Allocator struct {
-	StorePool     *storepool.StorePool
+	st            *cluster.Settings
+	StorePool     storepool.AllocatorStorePool
 	nodeLatencyFn func(addr string) (time.Duration, bool)
 	// TODO(aayush): Let's replace this with a *rand.Rand that has a rand.Source
 	// wrapped inside a mutex, to avoid misuse.
@@ -509,20 +510,23 @@ func makeAllocatorMetrics() AllocatorMetrics {
 
 // MakeAllocator creates a new allocator using the specified StorePool.
 func MakeAllocator(
-	storePool *storepool.StorePool,
+	st *cluster.Settings,
+	storePool storepool.AllocatorStorePool,
 	nodeLatencyFn func(addr string) (time.Duration, bool),
 	knobs *allocator.TestingKnobs,
 ) Allocator {
 	var randSource rand.Source
 	// There are number of test cases that make a test store but don't add
 	// gossip or a store pool. So we can't rely on the existence of the
 	// store pool in those cases.
-	if storePool != nil && storePool.Deterministic {
+	if storePool != nil && storePool.IsDeterministic() {
 		randSource = rand.NewSource(777)
 	} else {
+
 		randSource = rand.NewSource(rand.Int63())
 	}
 	allocator := Allocator{
+		st:            st,
 		StorePool:     storePool,
 		nodeLatencyFn: nodeLatencyFn,
 		randGen:       makeAllocatorRand(randSource),
@@ -931,7 +935,7 @@ func (a *Allocator) allocateTarget(
 	// as possible, and therefore any store that is good enough will be
 	// considered.
 	var selector CandidateSelector
-	if replicaStatus == Alive || recoveryStoreSelector.Get(&a.StorePool.St.SV) == "best" {
+	if replicaStatus == Alive || recoveryStoreSelector.Get(&a.st.SV) == "best" {
 		selector = a.NewBestCandidateSelector()
 	} else {
 		selector = a.NewGoodCandidateSelector()
@@ -1515,8 +1519,8 @@ func (a Allocator) RebalanceNonVoter(
 func (a *Allocator) ScorerOptions(ctx context.Context) *RangeCountScorerOptions {
 	return &RangeCountScorerOptions{
 		StoreHealthOptions:      a.StoreHealthOptions(ctx),
-		deterministic:           a.StorePool.Deterministic,
-		rangeRebalanceThreshold: RangeRebalanceThreshold.Get(&a.StorePool.St.SV),
+		deterministic:           a.StorePool.IsDeterministic(),
+		rangeRebalanceThreshold: RangeRebalanceThreshold.Get(&a.st.SV),
 	}
 }
 
@@ -1525,7 +1529,7 @@ func (a *Allocator) ScorerOptionsForScatter(ctx context.Context) *ScatterScorerO
 	return &ScatterScorerOptions{
 		RangeCountScorerOptions: RangeCountScorerOptions{
 			StoreHealthOptions:      a.StoreHealthOptions(ctx),
-			deterministic:           a.StorePool.Deterministic,
+			deterministic:           a.StorePool.IsDeterministic(),
 			rangeRebalanceThreshold: 0,
 		},
 		// We set jitter to be equal to the padding around replica-count rebalancing
@@ -1534,7 +1538,7 @@ func (a *Allocator) ScorerOptionsForScatter(ctx context.Context) *ScatterScorerO
 		// made by the replicateQueue during normal course of operations. In other
 		// words, we don't want stores that are too far away from the mean to be
 		// affected by the jitter.
-		jitter: RangeRebalanceThreshold.Get(&a.StorePool.St.SV),
+		jitter: RangeRebalanceThreshold.Get(&a.st.SV),
 	}
 }
 
@@ -1691,10 +1695,10 @@ func (a *Allocator) leaseholderShouldMoveDueToPreferences(
 // storeHealthLogOnly. By default storeHealthBlockRebalanceTo is the action taken. When
 // there is a mixed version cluster, storeHealthNoAction is set instead.
 func (a *Allocator) StoreHealthOptions(_ context.Context) StoreHealthOptions {
-	enforcementLevel := StoreHealthEnforcement(l0SublevelsThresholdEnforce.Get(&a.StorePool.St.SV))
+	enforcementLevel := StoreHealthEnforcement(l0SublevelsThresholdEnforce.Get(&a.st.SV))
 	return StoreHealthOptions{
 		EnforcementLevel:    enforcementLevel,
-		L0SublevelThreshold: l0SublevelsThreshold.Get(&a.StorePool.St.SV),
+		L0SublevelThreshold: l0SublevelsThreshold.Get(&a.st.SV),
 	}
 }
 
@@ -1862,8 +1866,8 @@ func (a *Allocator) TransferLeaseTarget(
 			storeDescMap,
 			&QPSScorerOptions{
 				StoreHealthOptions:    a.StoreHealthOptions(ctx),
-				QPSRebalanceThreshold: allocator.QPSRebalanceThreshold.Get(&a.StorePool.St.SV),
-				MinRequiredQPSDiff:    allocator.MinQPSDifferenceForTransfers.Get(&a.StorePool.St.SV),
+				QPSRebalanceThreshold: allocator.QPSRebalanceThreshold.Get(&a.st.SV),
+				MinRequiredQPSDiff:    allocator.MinQPSDifferenceForTransfers.Get(&a.st.SV),
 			},
 		)
 
@@ -2066,7 +2070,7 @@ func (a Allocator) shouldTransferLeaseForAccessLocality(
 	// stats and locality information to base our decision on.
 	if statSummary == nil ||
 		statSummary.LocalityCounts == nil ||
-		!EnableLoadBasedLeaseRebalancing.Get(&a.StorePool.St.SV) {
+		!EnableLoadBasedLeaseRebalancing.Get(&a.st.SV) {
 		return decideWithoutStats, roachpb.ReplicaDescriptor{}
 	}
 	replicaLocalities := a.StorePool.GetLocalitiesByNode(existing)
@@ -2128,7 +2132,7 @@ func (a Allocator) shouldTransferLeaseForAccessLocality(
 		if !ok {
 			continue
 		}
-		addr, err := a.StorePool.Gossip.GetNodeIDAddress(repl.NodeID)
+		addr, err := a.StorePool.GossipNodeIDAddress(repl.NodeID)
 		if err != nil {
 			log.KvDistribution.Errorf(ctx, "missing address for n%d: %+v", repl.NodeID, err)
 			continue
@@ -2140,7 +2144,7 @@ func (a Allocator) shouldTransferLeaseForAccessLocality(
 
 		remoteWeight := math.Max(minReplicaWeight, replicaWeights[repl.NodeID])
 		replScore, rebalanceAdjustment := loadBasedLeaseRebalanceScore(
-			ctx, a.StorePool.St, remoteWeight, remoteLatency, storeDesc, sourceWeight, source, candidateLeasesMean)
+			ctx, a.st, remoteWeight, remoteLatency, storeDesc, sourceWeight, source, candidateLeasesMean)
 		if replScore > bestReplScore {
 			bestReplScore = replScore
 			bestRepl = repl