Merge #155772 #155789 #155797 #155865

155772: sql/inspect: skip REFCURSOR stored columns in consistency check r=spilchen a=spilchen

REFCURSOR values cannot be compared for equality, which breaks the `INSPECT` command when an index stores a column of this type. During a consistency check, INSPECT attempts to compare stored column payloads between the primary and secondary indexes. This fails for REFCURSOR values because the column type cannot be used in an equality comparison.

This change skips REFCURSOR stored columns during the primary/secondary comparison.

Informs: #155676
Epic: CRDB-55075

Release note (bug fix): INSPECT can now be run on tables with indexes that store REFCURSOR-typed columns.

155789: publish-artifacts: change GCS latest key prefix for Workload binary r=rail a=williamchoe3

Informs #154274 

In gcs `gs://cockroach-edge-artifacts-prod`, change `latest` Workload binary key prefix from `cockroach` to `workload`
i.e.
```
gs://cockroach-edge-artifacts-prod/cockroach/workload.release-24.1
to
gs://cockroach-edge-artifacts-prod/workload/workload.release-24.1
```
This will allow `latest` workload binary to not be deleted after 30 days so tests can download and use workload binaries from unsupported versions (that no longer have nightlies running against them)

Notes:
* Will need to backfill currently unsupported versions 
* ".../workload/workload...." while descriptive also sounds redundant, open to suggestions if folks don't like that prefix
* The non latest copy of the obj i.e. `gs://cockroach-edge-artifacts-prod/cockroach/workload.ee80eab77ca6434f4be7c33c0f6d29dd0b9d07b1` is still prefixed with `cockroach` so these objects will get deleted after 30 days with the current lifecycle policy

Edited `PutNonRelease`'s docstring to remove "redirect" terminology, looks to be from a previous s3 implementation which is not a feature of GCS.
* Note: I don't feel that strongly about the docstring, it just confused me a bit when reading the code and just wanted it to match what the function is actually doing 

Also noticed there's no unit test coverage for non master branches, added 1 release branch test with simple coverage

Also noticed there's a slight difference between mockGCSProvider.PutObject & release.PutObject
```
# obj latest key names on gs://cockroach-edge-artifacts-prod
gs://cockroach-edge-artifacts-prod/cockroach/workload.release-23.2

# obj latest key names in unit tests
gs://edge-binaries-bucket/workload/workload.release-25.4/no-cache
```
Actual `release.PutObject` does not modify the key name based on the `PutObjectInput.CacheControl` field, but in `mockGCSProvider.PutObject` the `"/no-cache"` gets appended
I changed the behavior in the mock to match the actual function, and modified the unit tests
FWIW `PutObjectInput.CacheControl` is only set to `False` currently
* It looks like this might be a hold over from the S3 implementation? I didn't read that diff but let me know if I should revert this change

This PR is a dependency for these roachtest changes: #155397
Also discussed other options like using a new bucket or using the release bucket here: https://cockroachlabs.slack.com/archives/C03SG8QKYRJ/p1760977289072049?thread_ts=1760976858.868889&cid=C03SG8QKYRJ 
Decided on this approach for simplicity 

For verification, I'm relying on the unit tests. Just assumed this wouldn't work on my local, but if there's a simple way to verify this E2E let me know and i'll do it 

Will squash and write a proper commit msg after review, leaving commit separate incase i need to revert anything

155797: *: replace some usages of global rand r=yuzefovich a=yuzefovich

**sem/builtins: use RNG from eval context for 'st_generatepoints'**

***: replace some usages of global rand**

When we use `rand.*` methods like `math/rand.Intn`, under the hood we hit
the global rand source, which is protected by a mutex, so we could be subject
to contention on that lock. This commit updates some of such usages in
favor of object-specific rand source to avoid that. Note that allocating
a separate rand source is not free (the allocation itself is about 5KiB
in size), so we modify spots where the lifecycle matches that of the
server or when the affected code is heavy already / not on the hot path
(also when it's clear that the access is from within a single
goroutine).

Epic: None
Release note: None

155865: roachtest: remove metamorphicBufferedSender r=wenyihu6 a=wenyihu6

This setting is now enabled by default, so there is no need to enable this
metamorphically.

Epic: none 
Release note: none 

Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: William Choe <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: wenyihu6 <[email protected]>

Author: 5 people

pkg/cmd/publish-artifacts/main_test.go

@@ -1104,7 +1104,6 @@ func (r *testRunner) runWorker(
 						setting string
 						label   string
 					}{
-						{setting: "kv.rangefeed.buffered_sender.enabled", label: "metamorphicBufferedSender"},
 						{setting: "kv.transaction.write_buffering.enabled", label: "metamorphicWriteBuffering"},
 					} {
 						enable := prng.Intn(2) == 0

pkg/cmd/roachtest/test_runner.go

@@ -34,6 +34,7 @@ go_library(
         "//pkg/util/log",
         "//pkg/util/metric",
         "//pkg/util/protoutil",
+        "//pkg/util/randutil",
         "//pkg/util/stop",
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",

pkg/gossip/BUILD.bazel

@@ -62,6 +62,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -1360,9 +1361,10 @@ func (g *Gossip) manage(rpcContext *rpc.Context) {
 		var cullTimer, stallTimer timeutil.Timer
 		defer cullTimer.Stop()
 		defer stallTimer.Stop()
+		rng, _ := randutil.NewPseudoRand()
 
-		cullTimer.Reset(jitteredInterval(g.cullInterval))
-		stallTimer.Reset(jitteredInterval(g.stallInterval))
+		cullTimer.Reset(jitteredInterval(g.cullInterval, rng))
+		stallTimer.Reset(jitteredInterval(g.stallInterval, rng))
 		for {
 			select {
 			case <-g.server.stopper.ShouldQuiesce():
@@ -1372,7 +1374,7 @@ func (g *Gossip) manage(rpcContext *rpc.Context) {
 			case <-g.tighten:
 				g.tightenNetwork(ctx, rpcContext)
 			case <-cullTimer.C:
-				cullTimer.Reset(jitteredInterval(g.cullInterval))
+				cullTimer.Reset(jitteredInterval(g.cullInterval, rng))
 				func() {
 					g.mu.Lock()
 					if !g.outgoing.hasSpace() {
@@ -1402,7 +1404,7 @@ func (g *Gossip) manage(rpcContext *rpc.Context) {
 					g.mu.Unlock()
 				}()
 			case <-stallTimer.C:
-				stallTimer.Reset(jitteredInterval(g.stallInterval))
+				stallTimer.Reset(jitteredInterval(g.stallInterval, rng))
 				func() {
 					g.mu.Lock()
 					defer g.mu.Unlock()
@@ -1415,8 +1417,8 @@ func (g *Gossip) manage(rpcContext *rpc.Context) {
 
 // jitteredInterval returns a randomly jittered (+/-25%) duration
 // from checkInterval.
-func jitteredInterval(interval time.Duration) time.Duration {
-	return time.Duration(float64(interval) * (0.75 + 0.5*rand.Float64()))
+func jitteredInterval(interval time.Duration, rng *rand.Rand) time.Duration {
+	return time.Duration(float64(interval) * (0.75 + 0.5*rng.Float64()))
 }
 
 // tightenNetwork "tightens" the network by starting a new gossip client to the

pkg/gossip/gossip.go

@@ -183,12 +183,18 @@ func readFile(path string, dst io.Writer) error {
 	return errors.CombineErrors(err, file.Close())
 }
 
-// PutNonRelease uploads non-release related files.
-// Files are uploaded to /cockroach/<FilePath> for each non release file.
-// A `latest` key is then put at cockroach/<RedirectPrefix>.<BranchName> that redirects
-// to the above file.
+// PutNonRelease uploads non-release related files
+//
+// Each file is uploaded to /<prefix>/<FilePath> with prefix cockroach
+// i.e. /cockroach/<FilePath>
+//
+// Then that file is uploaded again under a `latest` key
+// <prefix>/<RedirectPrefix>.<BranchName>
+// Workload "latest" objects are stored under workload/<RedirectPrefix>.<BranchName>
+// All other "latest" objects are stored under cockroach/<RedirectPrefix>.<BranchName>
 func PutNonRelease(svc ObjectPutGetter, o PutNonReleaseOptions) {
 	const nonReleasePrefix = "cockroach"
+	const workloadPrefix = "workload"
 	for _, f := range o.Files {
 		disposition := mime.FormatMediaType("attachment", map[string]string{
 			"filename": f.FileName,
@@ -214,7 +220,13 @@ func PutNonRelease(svc ObjectPutGetter, o PutNonReleaseOptions) {
 		if latestSuffix == "master" {
 			latestSuffix = "LATEST"
 		}
-		latestKey := fmt.Sprintf("%s/%s.%s", nonReleasePrefix, f.RedirectPathPrefix, latestSuffix)
+		var latestPrefix string
+		if strings.HasPrefix(f.RedirectPathPrefix, "workload") {
+			latestPrefix = workloadPrefix
+		} else {
+			latestPrefix = nonReleasePrefix
+		}
+		latestKey := fmt.Sprintf("%s/%s.%s", latestPrefix, f.RedirectPathPrefix, latestSuffix)
 		// NB: The leading slash is required to make redirects work
 		// correctly since we reuse this key as the redirect location.
 		target := "/" + versionKey

pkg/release/upload.go

@@ -36,6 +36,7 @@ go_library(
         "//pkg/util/log",
         "//pkg/util/log/logcrash",
         "//pkg/util/protoutil",
+        "//pkg/util/randutil",
         "//pkg/util/stop",
         "//pkg/util/syncutil",
         "//pkg/util/system",

pkg/server/diagnostics/BUILD.bazel

@@ -149,9 +149,9 @@ func addInfoToURL(
 }
 
 // randomly shift `d` to be up to `jitterSeconds` shorter or longer.
-func addJitter(d time.Duration) time.Duration {
+func addJitter(d time.Duration, rng *rand.Rand) time.Duration {
 	const jitterSeconds = 120
-	j := time.Duration(rand.Intn(jitterSeconds*2)-jitterSeconds) * time.Second
+	j := time.Duration(rng.Intn(jitterSeconds*2)-jitterSeconds) * time.Second
 	return d + j
 }
 

pkg/server/diagnostics/diagnostics.go

@@ -40,6 +40,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
@@ -185,6 +186,7 @@ func (r *Reporter) PeriodicallyReportDiagnostics(ctx context.Context, stopper *s
 		ctx, cancel = stopper.WithCancelOnQuiesce(ctx)
 		defer cancel()
 		defer logcrash.RecoverAndReportNonfatalPanic(ctx, &r.Settings.SV)
+		rng, _ := randutil.NewPseudoRand()
 		nextReport := r.StartTime
 
 		var timer timeutil.Timer
@@ -199,7 +201,7 @@ func (r *Reporter) PeriodicallyReportDiagnostics(ctx context.Context, stopper *s
 
 			nextReport = nextReport.Add(reportFrequency.Get(&r.Settings.SV))
 
-			timer.Reset(addJitter(nextReport.Sub(timeutil.Now())))
+			timer.Reset(addJitter(nextReport.Sub(timeutil.Now()), rng))
 			select {
 			case <-stopper.ShouldQuiesce():
 				return

pkg/server/diagnostics/reporter.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/httputil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
@@ -74,6 +75,7 @@ func (u *UpdateChecker) PeriodicallyCheckForUpdates(ctx context.Context, stopper
 		SpanOpt:  stop.SterileRootSpan,
 	}, func(ctx context.Context) {
 		defer logcrash.RecoverAndReportNonfatalPanic(ctx, &u.Settings.SV)
+		rng, _ := randutil.NewPseudoRand()
 		nextUpdateCheck := u.StartTime
 
 		var timer timeutil.Timer
@@ -84,7 +86,7 @@ func (u *UpdateChecker) PeriodicallyCheckForUpdates(ctx context.Context, stopper
 
 			nextUpdateCheck = u.maybeCheckForUpdates(ctx, now, nextUpdateCheck, runningTime)
 
-			timer.Reset(addJitter(nextUpdateCheck.Sub(timeutil.Now())))
+			timer.Reset(addJitter(nextUpdateCheck.Sub(timeutil.Now()), rng))
 			select {
 			case <-stopper.ShouldQuiesce():
 				return

pkg/server/diagnostics/update_checker.go

@@ -111,6 +111,9 @@ var LeaseRenewalCrossValidate = settings.RegisterBoolSetting(
 func (s storage) jitteredLeaseDuration() time.Duration {
 	leaseDuration := LeaseDuration.Get(&s.settings.SV)
 	jitterFraction := LeaseJitterFraction.Get(&s.settings.SV)
+	// TODO(yuzefovich): it would probably be worth replacing this usage of
+	// global rand with rng tied to the 'storage' object. It's not clear whether
+	// we need concurrency safety or not.
 	return time.Duration(float64(leaseDuration) * (1 - jitterFraction +
 		2*jitterFraction*rand.Float64()))
 }