Merge #148476 #151110 #151163 #151218

148476: kvserver: update store pool for leasequeue.process r=sumeerbhola,tbg a=wenyihu6

Previously, the lease queue didn’t update the local store pool correctly because
`AllocationTransferLeaseOp.ApplyImpact` was unimplemented. This wasn’t an issue
for the replicate queue, as it updates the local store pool directly through
`rq.TransferLease` by calling `UpdateLocalStoresAfterLeaseTransfer`. This patch
addresses the issue by calling `UpdateLocalStoresAfterLeaseTransfer` after
`replica.AdminTransferLease` inside the lease queue.

This fixes a bug where the allocator may make rebalancing decisions based on
stale store pool data, failing to account for recent lease transfers not yet
reflected in gossip. This state should eventually be corrected when real store
capacity is measured and gossiped, and store pools are refreshed.

This bug was found when reading the code.

Epic: none
Release note (bug fixes): Fixed a bug present since v24.1 where the allocator
could make rebalancing decisions based on stale data, failing to account for
recent local lease transfers not yet reflected in store capacity or gossip.

151110: k8s: fix python3 compatibility issue for multi-region setup r=NishanthNalluri a=shreyaskm623

Previously , the setup.py file was used for the kubernetes cluster setup across the multiple regions , which worked only for the python2 versions , this fix will make it compatible for the python3 versions as well

Epic: none
Fixes: https://cockroachlabs.atlassian.net/browse/CRDB-48938
Release note : None

151163: roachtest: bump min version for db-console/mixed-version-endpoints roachtest r=alyshanjahani-crl a=alyshanjahani-crl

Previously the mixed version roachtest would fail whenever test plans included versions of crdb < 24.3. This was because a handful of /api/v2 endpoints were added in 24.3 (and not backported), and so they would return 404s in the test on versions prior.

This commit bumps the min version to 24.3.0.

Fixes: #150601, #148875, #149866
Release note: None

151218: integration: fix flaky TestInsightsIntegrationForContention r=kyle-a-wong a=kyle-a-wong

This test has been seen to flake recently, due to the query on crdb_internal.node_transactions not returning any rows. To fix, it is now run within a testutils.SucceedSoon and will retry until it exists.

Fixes: #151125, #150555, #150139
Release note: None

Co-authored-by: wenyihu6 <[email protected]>
Co-authored-by: Shreyas KM <[email protected]>
Co-authored-by: Alyshan Jahani <[email protected]>
Co-authored-by: Kyle Wong <[email protected]>

Author: 5 people

cloud/kubernetes/multiregion/setup.py

@@ -6,7 +6,6 @@
 # included in the /LICENSE file.
 
 
-import distutils.spawn
 import json
 import os
 from subprocess import check_call,check_output
@@ -116,12 +115,12 @@
 for zone, context in contexts.items():
     external_ip = ''
     while True:
-        external_ip = check_output(['kubectl', 'get', 'svc', 'kube-dns-lb', '--namespace', 'kube-system', '--context', context, '--template', '{{range .status.loadBalancer.ingress}}{{.ip}}{{end}}'])
+        external_ip = check_output(['kubectl', 'get', 'svc', 'kube-dns-lb', '--namespace', 'kube-system', '--context', context, '--template', '{{range .status.loadBalancer.ingress}}{{.ip}}{{end}}']).decode('utf-8')
         if external_ip:
             break
-        print  'Waiting for DNS load balancer IP in %s...' % (zone)
+        print('Waiting for DNS load balancer IP in %s...' % (zone))
         sleep(10)
-    print 'DNS endpoint for zone %s: %s' % (zone, external_ip)
+    print('DNS endpoint for zone %s: %s' % (zone, external_ip))
     dns_ips[zone] = external_ip
 
 # Update each cluster's DNS configuration with an appropriate configmap. Note
@@ -175,7 +174,7 @@
     check_call(['kubectl', 'apply', '-f', yaml_file, '--namespace', zone, '--context', context])
 
 # Finally, initialize the cluster.
-print 'Sleeping 30 seconds before attempting to initialize cluster to give time for volumes to be created and pods started.'
+print('Sleeping 30 seconds before attempting to initialize cluster to give time for volumes to be created and pods started.')
 sleep(30)
 for zone, context in contexts.items():
     check_call(['kubectl', 'create', '-f', 'cluster-init-secure.yaml', '--namespace', zone, '--context', context])

pkg/cmd/roachtest/tests/db_console_endpoints.go

@@ -110,10 +110,8 @@ func registerDBConsoleEndpointsMixedVersion(r registry.Registry) {
 func runDBConsoleMixedVersion(ctx context.Context, t test.Test, c cluster.Cluster) {
 	mvt := mixedversion.NewTest(ctx, t, t.L(), c,
 		c.CRDBNodes(),
-		// We test only upgrades from 23.2 in this test because it uses
-		// the `workload init` command, which is only supported
-		// reliably multi-tenant mode starting from that version.
-		mixedversion.MinimumSupportedVersion("v23.2.0"),
+		// In 24.3 new endpoints were added to /api/v2 server.
+		mixedversion.MinimumSupportedVersion("v24.3.0"),
 	)
 
 	mvt.InMixedVersion("test db console endpoints", func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error {

pkg/kv/kvserver/lease_queue.go

@@ -139,7 +139,15 @@ func (lq *leaseQueue) process(
 		if err := repl.AdminTransferLease(ctx, transferOp.Target.StoreID, false /* bypassSafetyChecks */); err != nil {
 			return false, errors.Wrapf(err, "%s: unable to transfer lease to s%d", repl, transferOp.Target)
 		}
-		change.Op.ApplyImpact(lq.storePool)
+
+		// TODO(wenyihu6): Initially, change.Op.ApplyImpact was used here. This was
+		// a problem since AllocationTransferLeaseOp.ApplyImpact was left
+		// unimplemented. We should either implement
+		// AllocationTransferLeaseOp.ApplyImpact correctly or remove the use of
+		// ApplyImpact entirely. The replicate queue does not have this issue since
+		// it uses rq.TransferLease, which updates the local store pool directly.
+		lq.storePool.UpdateLocalStoresAfterLeaseTransfer(
+			transferOp.Source.StoreID, transferOp.Target.StoreID, transferOp.Usage)
 	}
 
 	return true, nil

pkg/sql/sqlstats/insights/integration/BUILD.bazel

@@ -26,6 +26,7 @@ go_test(
         "//pkg/util/log",
         "//pkg/util/timeutil",
         "//pkg/util/uuid",
+        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",
     ],
 )

pkg/sql/sqlstats/insights/integration/insights_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -803,7 +804,6 @@ func TestInsightsIntegrationForContention(t *testing.T) {
 	// Chan to wait for the txn to complete to avoid checking for insights before the txn is committed.
 	txnDoneChan := make(chan struct{})
 
-	observerConn := sqlutils.MakeSQLRunner(tc.ApplicationLayer(0).SQLConn(t))
 	txConn := sqlutils.MakeSQLRunner(tc.ApplicationLayer(0).SQLConn(t))
 	tx := txConn.Begin(t)
 
@@ -828,10 +828,20 @@ func TestInsightsIntegrationForContention(t *testing.T) {
 	require.NoError(t, errTxn)
 
 	var waitingTxnID uuid.UUID
-	observerConn.QueryRow(t,
-		`SELECT id
+	testutils.SucceedsSoon(t, func() error {
+		r, err := tc.ApplicationLayer(0).SQLConn(t).Query(`SELECT id
          FROM crdb_internal.node_transactions
-         WHERE application_name = 'waiting_txn'`).Scan(&waitingTxnID)
+         WHERE application_name = 'waiting_txn'`)
+		require.NoError(t, err)
+		defer r.Close()
+
+		if !r.Next() {
+			return errors.New("waiting_txn not found")
+		}
+		err = r.Scan(&waitingTxnID)
+		require.NoError(t, err)
+		return nil
+	})
 
 	require.NoError(t, tx.Commit())