Merge #146301 #146415 #146455

146301: admission: ensure GetPebbleMetrics is not called after StoreGrantCoor… r=RaduBerinde a=sumeerbhola

…dinators.close

Fixes #140454, #144172

Epic: none

Release note: None

146415: roachtest: metamorphically enable buffered writes r=yuzefovich a=yuzefovich

This commit adjusts the roachtest runner to enable buffered writes in 50% cases (unless the test is marked as a benchmark). It also adds a new operation to change the corresponding cluster setting too.

Informs: #143860.
Epic: None

Release note: None

146455: ci: remove unused MacOS signing script r=jlinder,rickystewart a=rail

This script has been replaced by a new one that uses `rcodesign` on Linux.

Release note: none
Epic: none

Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>

Author: 4 people

build/teamcity/internal/cockroach/release/publish/sign_staged_macos_release.sh

@@ -108,6 +108,12 @@ func registerClusterSettings(r registry.Registry) {
 			Generator: timeBasedValues(timeutil.Now, []string{"true", "false"}, 12*time.Hour),
 			Owner:     registry.OwnerObservability,
 		},
+		{
+			// Periodically switch between two transaction protocol variants.
+			Name:      "kv.transaction.write_buffering.enabled",
+			Generator: timeBasedValues(timeutil.Now, []string{"true", "false"}, 6*time.Hour),
+			Owner:     registry.OwnerKV,
+		},
 	}
 	sanitizeOpName := func(name string) string {
 		return strings.ReplaceAll(name, ".", "_")

pkg/cmd/roachtest/operations/cluster_settings.go

@@ -936,15 +936,26 @@ func (r *testRunner) runWorker(
 				// Apply metamorphic settings not explicitly defined by the test.
 				// These settings should only be applied to non-benchmark tests.
 				if !testSpec.Benchmark {
-					// 50% chance of enabling the rangefeed buffered sender. Disabled by
-					// default. Disabled for mixed-version tests since this cluster setting
-					// is only supported in >= v25.2.
-					useBufferedSender := prng.Intn(2) == 0
-					if !t.spec.Suites.Contains(registry.MixedVersion) && useBufferedSender {
-						c.clusterSettings["kv.rangefeed.buffered_sender.enabled"] = "true"
+					// 50% chance of enabling the rangefeed buffered sender.
+					// 50% change of enabling buffered writes.
+					//
+					// Disabled by default. Disabled for mixed-version tests
+					// since these cluster settings are not supported in all
+					// versions.
+					for _, tc := range []struct {
+						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
+						if !t.spec.Suites.Contains(registry.MixedVersion) && enable {
+							c.clusterSettings[tc.setting] = "true"
+							c.status(fmt.Sprintf("metamorphically setting %q to 'true'", tc.setting))
+							t.AddParam(tc.label, fmt.Sprint(enable))
+						}
 					}
-					c.status(fmt.Sprintf("metamorphically using buffered sender: %t", useBufferedSender))
-					t.AddParam("metamorphicBufferedSender", fmt.Sprint(useBufferedSender))
 				}
 
 				c.goCoverDir = t.GoCoverArtifactsDir()

pkg/cmd/roachtest/test_runner.go

@@ -276,6 +276,14 @@ func (sgc *StoreGrantCoordinators) TryGetSnapshotQueueForStore(storeID roachpb.S
 func (sgc *StoreGrantCoordinators) close() {
 	// closeCh can be nil in tests that never called SetPebbleMetricsProvider.
 	if sgc.closeCh != nil {
+		// Ensure that the goroutine has observed the close and will no longer
+		// call GetPebbleMetrics, since the engines will be closed soon after this
+		// method returns, and calling GetPebbleMetrics on closed engines is not
+		// permitted.
+		sgc.closeCh <- struct{}{}
+		// Close the channel, so that if close gets called twice due to a bug,
+		// sending on the closed channel will panic instead of the send being
+		// blocked forever.
 		close(sgc.closeCh)
 	}