upgrades: remove sampling stmt diagnostics migration from 22.2

We only support upgrading to 23.2 or later from at least 22.2, so we no
longer need this non-permanent migration to add "sampling probability"
column to the statement diagnostics table.

Release note: None

Author: yuzefovich

pkg/cli/clisqlclient/statement_diag.go

@@ -13,7 +13,6 @@ package clisqlclient
 import (
 	"context"
 	"database/sql/driver"
-	"fmt"
 	"io"
 	"os"
 	"strconv"
@@ -127,39 +126,9 @@ func StmtDiagListOutstandingRequests(
 	return result, nil
 }
 
-// TODO(irfansharif): Remove this in 23.1.
-func isAtLeast22dot2ClusterVersion(ctx context.Context, conn Conn) (bool, error) {
-	// Check whether the upgrade to add the sampling_probability column to the
-	// statement_diagnostics_requests system table has already been run.
-	row, err := conn.QueryRow(ctx, `
- SELECT
-   count(*)
- FROM
-   [SHOW COLUMNS FROM system.statement_diagnostics_requests]
- WHERE
-   column_name = 'sampling_probability';`)
-	if err != nil {
-		return false, err
-	}
-	c, ok := row[0].(int64)
-	if !ok {
-		return false, nil
-	}
-	return c == 1, nil
-}
-
 func stmtDiagListOutstandingRequestsInternal(
 	ctx context.Context, conn Conn,
 ) ([]StmtDiagActivationRequest, error) {
-	var extraColumns string
-	atLeast22dot2, err := isAtLeast22dot2ClusterVersion(ctx, conn)
-	if err != nil {
-		return nil, err
-	}
-	if atLeast22dot2 {
-		extraColumns = ", sampling_probability"
-	}
-
 	// Converting an INTERVAL to a number of milliseconds within that interval
 	// is a pain - we extract the number of seconds and multiply it by 1000,
 	// then we extract the number of milliseconds and add that up to the
@@ -169,10 +138,10 @@ func stmtDiagListOutstandingRequestsInternal(
                         EXTRACT(millisecond FROM min_execution_latency)::INT8 -
                         EXTRACT(second FROM min_execution_latency)::INT8 * 1000`
 	rows, err := conn.Query(ctx,
-		fmt.Sprintf("SELECT id, statement_fingerprint, requested_at, "+getMilliseconds+`, expires_at%s
+		"SELECT id, statement_fingerprint, requested_at, "+getMilliseconds+`, expires_at, sampling_probability
 			FROM system.statement_diagnostics_requests
 			WHERE NOT completed
-			ORDER BY requested_at DESC`, extraColumns),
+			ORDER BY requested_at DESC`,
 	)
 	if err != nil {
 		return nil, err
@@ -195,10 +164,8 @@ func stmtDiagListOutstandingRequestsInternal(
 		if e, ok := vals[4].(time.Time); ok {
 			expiresAt = e
 		}
-		if atLeast22dot2 {
-			if sp, ok := vals[5].(float64); ok {
-				samplingProbability = sp
-			}
+		if sp, ok := vals[5].(float64); ok {
+			samplingProbability = sp
 		}
 		info := StmtDiagActivationRequest{
 			ID:                  vals[0].(int64),

pkg/clusterversion/cockroach_versions.go

@@ -201,10 +201,6 @@ const (
 	TODODelete_V22_2RemoveGrantPrivilege
 	// TODODelete_V22_2MVCCRangeTombstones enables the use of MVCC range tombstones.
 	TODODelete_V22_2MVCCRangeTombstones
-	// TODODelete_V22_2SampledStmtDiagReqs enables installing statement diagnostic requests that
-	// probabilistically collects stmt bundles, controlled by the user provided
-	// sampling rate.
-	TODODelete_V22_2SampledStmtDiagReqs
 	// TODODelete_V22_2SystemPrivilegesTable adds system.privileges table.
 	TODODelete_V22_2SystemPrivilegesTable
 	// TODODelete_V22_2EnablePredicateProjectionChangefeed indicates that changefeeds support
@@ -650,10 +646,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2MVCCRangeTombstones,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 16},
 	},
-	{
-		Key:     TODODelete_V22_2SampledStmtDiagReqs,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 20},
-	},
 	{
 		Key:     TODODelete_V22_2SystemPrivilegesTable,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 24},

pkg/server/statement_diagnostics_requests.go

@@ -12,10 +12,8 @@ package server
 
 import (
 	"context"
-	"fmt"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/server/authserver"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -137,25 +135,20 @@ func (s *statusServer) StatementDiagnosticsRequests(
 
 	var err error
 
-	// TODO(irfansharif): Remove this version gating in 23.1.
-	var extraColumns string
-	if s.st.Version.IsActive(ctx, clusterversion.TODODelete_V22_2SampledStmtDiagReqs) {
-		extraColumns = `,
-			sampling_probability`
-	}
 	// TODO(davidh): Add pagination to this request.
 	it, err := s.internalExecutor.QueryIteratorEx(ctx, "stmt-diag-get-all", nil, /* txn */
 		sessiondata.RootUserSessionDataOverride,
-		fmt.Sprintf(`SELECT
+		`SELECT
 			id,
 			statement_fingerprint,
 			completed,
 			statement_diagnostics_id,
 			requested_at,
 			min_execution_latency,
-			expires_at%s
+			expires_at,
+			sampling_probability
 		FROM
-			system.statement_diagnostics_requests`, extraColumns))
+			system.statement_diagnostics_requests`)
 	if err != nil {
 		return nil, err
 	}
@@ -179,10 +172,8 @@ func (s *statusServer) StatementDiagnosticsRequests(
 		if requestedAt, ok := row[4].(*tree.DTimestampTZ); ok {
 			req.RequestedAt = requestedAt.Time
 		}
-		if extraColumns != "" {
-			if samplingProbability, ok := row[7].(*tree.DFloat); ok {
-				req.SamplingProbability = float64(*samplingProbability)
-			}
+		if samplingProbability, ok := row[7].(*tree.DFloat); ok {
+			req.SamplingProbability = float64(*samplingProbability)
 		}
 
 		if minExecutionLatency, ok := row[5].(*tree.DInterval); ok {

pkg/sql/stmtdiagnostics/BUILD.bazel

@@ -6,7 +6,6 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/stmtdiagnostics",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/clusterversion",
         "//pkg/multitenant",
         "//pkg/settings",
         "//pkg/settings/cluster",

pkg/sql/stmtdiagnostics/statement_diagnostics.go

@@ -16,7 +16,6 @@ import (
 	"math/rand"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -292,12 +291,6 @@ func (r *Registry) insertRequestInternal(
 	minExecutionLatency time.Duration,
 	expiresAfter time.Duration,
 ) (RequestID, error) {
-	isSamplingProbabilitySupported := r.st.Version.IsActive(ctx, clusterversion.TODODelete_V22_2SampledStmtDiagReqs)
-	if !isSamplingProbabilitySupported && samplingProbability != 0 {
-		return 0, errors.New(
-			"sampling probability only supported after 22.2 version migrations have completed",
-		)
-	}
 	if samplingProbability != 0 {
 		if samplingProbability < 0 || samplingProbability > 1 {
 			return 0, errors.Newf(
@@ -641,23 +634,18 @@ func (r *Registry) InsertStatementDiagnostics(
 // updates r.mu.requests accordingly.
 func (r *Registry) pollRequests(ctx context.Context) error {
 	var rows []tree.Datums
-	isSamplingProbabilitySupported := r.st.Version.IsActive(ctx, clusterversion.TODODelete_V22_2SampledStmtDiagReqs)
 
 	// Loop until we run the query without straddling an epoch increment.
 	for {
 		r.mu.Lock()
 		epoch := r.mu.epoch
 		r.mu.Unlock()
 
-		var extraColumns string
-		if isSamplingProbabilitySupported {
-			extraColumns = ", sampling_probability"
-		}
 		it, err := r.db.Executor().QueryIteratorEx(ctx, "stmt-diag-poll", nil, /* txn */
 			sessiondata.RootUserSessionDataOverride,
-			fmt.Sprintf(`SELECT id, statement_fingerprint, min_execution_latency, expires_at%s
+			`SELECT id, statement_fingerprint, min_execution_latency, expires_at, sampling_probability
 				FROM system.statement_diagnostics_requests
-				WHERE completed = false AND (expires_at IS NULL OR expires_at > now())`, extraColumns),
+				WHERE completed = false AND (expires_at IS NULL OR expires_at > now())`,
 		)
 		if err != nil {
 			return err
@@ -698,14 +686,12 @@ func (r *Registry) pollRequests(ctx context.Context) error {
 		if e, ok := row[3].(*tree.DTimestampTZ); ok {
 			expiresAt = e.Time
 		}
-		if isSamplingProbabilitySupported {
-			if prob, ok := row[4].(*tree.DFloat); ok {
-				samplingProbability = float64(*prob)
-				if samplingProbability < 0 || samplingProbability > 1 {
-					log.Warningf(ctx, "malformed sampling probability for request %d: %f (expected in range [0, 1]), resetting to 1.0",
-						id, samplingProbability)
-					samplingProbability = 1.0
-				}
+		if prob, ok := row[4].(*tree.DFloat); ok {
+			samplingProbability = float64(*prob)
+			if samplingProbability < 0 || samplingProbability > 1 {
+				log.Warningf(ctx, "malformed sampling probability for request %d: %f (expected in range [0, 1]), resetting to 1.0",
+					id, samplingProbability)
+				samplingProbability = 1.0
 			}
 		}
 		ids.Add(int(id))

pkg/upgrade/upgrades/BUILD.bazel

@@ -22,7 +22,6 @@ go_library(
         "key_visualizer_migration.go",
         "permanent_upgrades.go",
         "role_members_ids_migration.go",
-        "sampled_stmt_diagnostics_requests.go",
         "schema_changes.go",
         "schemachanger_elements.go",
         "sql_stats_ttl.go",
@@ -115,7 +114,6 @@ go_test(
         "key_visualizer_migration_test.go",
         "main_test.go",
         "role_members_ids_migration_test.go",
-        "sampled_stmt_diagnostics_requests_test.go",
         "schema_changes_external_test.go",
         "schema_changes_helpers_test.go",
         "schemachanger_elements_test.go",