Merge cockroachdb#107388 cockroachdb#108415 cockroachdb#108535

107388: metrics: refactor histogram bucket generation and testing r=ericharmeling a=ericharmeling

This commit refactors histogram bucketing for legibility and composibility. It also introduces a data-driven test for histogram bucket generation.

This refactor should make it easier to add additional metric categories, distributions, and bucket types.

Part of cockroachdb#97144.

Release note: None

108415: roachtest: deflake npgsql test r=rafiss a=rafiss

These upstream tests are flaky, so we ignore them.

informs cockroachdb#108414
fixes cockroachdb#108044
fixes cockroachdb#108504
Release note: None

108535: roachtest: remove rangeTs variants of import-cancellation test r=stevendanna a=jbowens

Previously the import-cancellation roachtest was split into two variants: one with MVCC range tombstones enabled and one without. MVCC range tombstones are always enabled now, so the two variants were effectively identical. This commit consolidates the two tests into a single `import-cancellation` roachtest.

Informs cockroachdb#97869.
Epic: None
Release note: None

Co-authored-by: Eric Harmeling <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

Author: 4 people

pkg/ccl/changefeedccl/metrics.go

@@ -552,52 +552,52 @@ func newAggregateMetrics(histogramWindow time.Duration) *AggMetrics {
 		EmittedMessages:  b.Counter(metaChangefeedEmittedMessages),
 		FilteredMessages: b.Counter(metaChangefeedFilteredMessages),
 		MessageSize: b.Histogram(metric.HistogramOptions{
-			Metadata: metaMessageSize,
-			Duration: histogramWindow,
-			MaxVal:   10 << 20, /* 10MB max message size */
-			SigFigs:  1,
-			Buckets:  metric.DataSize16MBBuckets,
+			Metadata:     metaMessageSize,
+			Duration:     histogramWindow,
+			MaxVal:       10 << 20, /* 10MB max message size */
+			SigFigs:      1,
+			BucketConfig: metric.DataSize16MBBuckets,
 		}),
 		EmittedBytes:     b.Counter(metaChangefeedEmittedBytes),
 		FlushedBytes:     b.Counter(metaChangefeedFlushedBytes),
 		Flushes:          b.Counter(metaChangefeedFlushes),
 		SizeBasedFlushes: b.Counter(metaSizeBasedFlushes),
 		ParallelIOQueueNanos: b.Histogram(metric.HistogramOptions{
-			Metadata: metaChangefeedParallelIOQueueNanos,
-			Duration: histogramWindow,
-			MaxVal:   changefeedIOQueueMaxLatency.Nanoseconds(),
-			SigFigs:  2,
-			Buckets:  metric.BatchProcessLatencyBuckets,
+			Metadata:     metaChangefeedParallelIOQueueNanos,
+			Duration:     histogramWindow,
+			MaxVal:       changefeedIOQueueMaxLatency.Nanoseconds(),
+			SigFigs:      2,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
 		}),
 		SinkIOInflight: b.Gauge(metaChangefeedSinkIOInflight),
 
 		BatchHistNanos: b.Histogram(metric.HistogramOptions{
-			Metadata: metaChangefeedBatchHistNanos,
-			Duration: histogramWindow,
-			MaxVal:   changefeedBatchHistMaxLatency.Nanoseconds(),
-			SigFigs:  1,
-			Buckets:  metric.BatchProcessLatencyBuckets,
+			Metadata:     metaChangefeedBatchHistNanos,
+			Duration:     histogramWindow,
+			MaxVal:       changefeedBatchHistMaxLatency.Nanoseconds(),
+			SigFigs:      1,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
 		}),
 		FlushHistNanos: b.Histogram(metric.HistogramOptions{
-			Metadata: metaChangefeedFlushHistNanos,
-			Duration: histogramWindow,
-			MaxVal:   changefeedFlushHistMaxLatency.Nanoseconds(),
-			SigFigs:  2,
-			Buckets:  metric.BatchProcessLatencyBuckets,
+			Metadata:     metaChangefeedFlushHistNanos,
+			Duration:     histogramWindow,
+			MaxVal:       changefeedFlushHistMaxLatency.Nanoseconds(),
+			SigFigs:      2,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
 		}),
 		CommitLatency: b.Histogram(metric.HistogramOptions{
-			Metadata: metaCommitLatency,
-			Duration: histogramWindow,
-			MaxVal:   commitLatencyMaxValue.Nanoseconds(),
-			SigFigs:  1,
-			Buckets:  metric.BatchProcessLatencyBuckets,
+			Metadata:     metaCommitLatency,
+			Duration:     histogramWindow,
+			MaxVal:       commitLatencyMaxValue.Nanoseconds(),
+			SigFigs:      1,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
 		}),
 		AdmitLatency: b.Histogram(metric.HistogramOptions{
-			Metadata: metaAdmitLatency,
-			Duration: histogramWindow,
-			MaxVal:   admitLatencyMaxValue.Nanoseconds(),
-			SigFigs:  1,
-			Buckets:  metric.BatchProcessLatencyBuckets,
+			Metadata:     metaAdmitLatency,
+			Duration:     histogramWindow,
+			MaxVal:       admitLatencyMaxValue.Nanoseconds(),
+			SigFigs:      1,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
 		}),
 		BackfillCount:             b.Gauge(metaChangefeedBackfillCount),
 		BackfillPendingRanges:     b.Gauge(metaChangefeedBackfillPendingRanges),
@@ -712,27 +712,27 @@ func MakeMetrics(histogramWindow time.Duration) metric.Struct {
 		Failures:          metric.NewCounter(metaChangefeedFailures),
 		QueueTimeNanos:    metric.NewCounter(metaEventQueueTime),
 		CheckpointHistNanos: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaChangefeedCheckpointHistNanos,
-			Duration: histogramWindow,
-			MaxVal:   changefeedCheckpointHistMaxLatency.Nanoseconds(),
-			SigFigs:  2,
-			Buckets:  metric.IOLatencyBuckets,
+			Metadata:     metaChangefeedCheckpointHistNanos,
+			Duration:     histogramWindow,
+			MaxVal:       changefeedCheckpointHistMaxLatency.Nanoseconds(),
+			SigFigs:      2,
+			BucketConfig: metric.IOLatencyBuckets,
 		}),
 		FrontierUpdates: metric.NewCounter(metaChangefeedFrontierUpdates),
 		ThrottleMetrics: cdcutils.MakeMetrics(histogramWindow),
 		// Below two metrics were never implemented using the hdr histogram. Set ForceUsePrometheus
 		// to true.
 		ParallelConsumerFlushNanos: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaChangefeedEventConsumerFlushNanos,
-			Duration: histogramWindow,
-			Buckets:  metric.IOLatencyBuckets,
-			Mode:     metric.HistogramModePrometheus,
+			Metadata:     metaChangefeedEventConsumerFlushNanos,
+			Duration:     histogramWindow,
+			BucketConfig: metric.IOLatencyBuckets,
+			Mode:         metric.HistogramModePrometheus,
 		}),
 		ParallelConsumerConsumeNanos: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaChangefeedEventConsumerConsumeNanos,
-			Duration: histogramWindow,
-			Buckets:  metric.IOLatencyBuckets,
-			Mode:     metric.HistogramModePrometheus,
+			Metadata:     metaChangefeedEventConsumerConsumeNanos,
+			Duration:     histogramWindow,
+			BucketConfig: metric.IOLatencyBuckets,
+			Mode:         metric.HistogramModePrometheus,
 		}),
 		ParallelConsumerInFlightEvents: metric.NewGauge(metaChangefeedEventConsumerInFlightEvents),
 	}

pkg/ccl/sqlproxyccl/connector_test.go

@@ -381,10 +381,10 @@ func TestConnector_dialTenantCluster(t *testing.T) {
 		c := &connector{
 			TenantID: roachpb.MustMakeTenantID(42),
 			DialTenantLatency: metric.NewHistogram(metric.HistogramOptions{
-				Mode:     metric.HistogramModePrometheus,
-				Metadata: metaDialTenantLatency,
-				Duration: time.Millisecond,
-				Buckets:  metric.IOLatencyBuckets,
+				Mode:         metric.HistogramModePrometheus,
+				Metadata:     metaDialTenantLatency,
+				Duration:     time.Millisecond,
+				BucketConfig: metric.IOLatencyBuckets,
 			}),
 			DialTenantRetries: metric.NewCounter(metaDialTenantRetries),
 		}
@@ -466,10 +466,10 @@ func TestConnector_dialTenantCluster(t *testing.T) {
 
 		c := &connector{
 			DialTenantLatency: metric.NewHistogram(metric.HistogramOptions{
-				Mode:     metric.HistogramModePreferHdrLatency,
-				Metadata: metaDialTenantLatency,
-				Duration: time.Millisecond,
-				Buckets:  metric.IOLatencyBuckets,
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     metaDialTenantLatency,
+				Duration:     time.Millisecond,
+				BucketConfig: metric.IOLatencyBuckets,
 			}),
 			DialTenantRetries: metric.NewCounter(metaDialTenantRetries),
 		}
@@ -500,10 +500,10 @@ func TestConnector_dialTenantCluster(t *testing.T) {
 		c := &connector{
 			TenantID: roachpb.MustMakeTenantID(42),
 			DialTenantLatency: metric.NewHistogram(metric.HistogramOptions{
-				Mode:     metric.HistogramModePreferHdrLatency,
-				Metadata: metaDialTenantLatency,
-				Duration: time.Millisecond,
-				Buckets:  metric.IOLatencyBuckets,
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     metaDialTenantLatency,
+				Duration:     time.Millisecond,
+				BucketConfig: metric.IOLatencyBuckets,
 			}),
 			DialTenantRetries: metric.NewCounter(metaDialTenantRetries),
 		}

pkg/ccl/sqlproxyccl/metrics.go

@@ -234,19 +234,19 @@ func makeProxyMetrics() metrics {
 		RefusedConnCount:       metric.NewCounter(metaRefusedConnCount),
 		SuccessfulConnCount:    metric.NewCounter(metaSuccessfulConnCount),
 		ConnectionLatency: metric.NewHistogram(metric.HistogramOptions{
-			Mode:     metric.HistogramModePreferHdrLatency,
-			Metadata: metaConnMigrationAttemptedCount,
-			Duration: base.DefaultHistogramWindowInterval(),
-			Buckets:  metric.IOLatencyBuckets,
+			Mode:         metric.HistogramModePreferHdrLatency,
+			Metadata:     metaConnMigrationAttemptedCount,
+			Duration:     base.DefaultHistogramWindowInterval(),
+			BucketConfig: metric.IOLatencyBuckets,
 		}),
 		AuthFailedCount:        metric.NewCounter(metaAuthFailedCount),
 		ExpiredClientConnCount: metric.NewCounter(metaExpiredClientConnCount),
 		// Connector metrics.
 		DialTenantLatency: metric.NewHistogram(metric.HistogramOptions{
-			Mode:     metric.HistogramModePreferHdrLatency,
-			Metadata: metaDialTenantLatency,
-			Duration: base.DefaultHistogramWindowInterval(),
-			Buckets:  metric.IOLatencyBuckets},
+			Mode:         metric.HistogramModePreferHdrLatency,
+			Metadata:     metaDialTenantLatency,
+			Duration:     base.DefaultHistogramWindowInterval(),
+			BucketConfig: metric.IOLatencyBuckets},
 		),
 		DialTenantRetries: metric.NewCounter(metaDialTenantRetries),
 		// Connection migration metrics.
@@ -255,17 +255,17 @@ func makeProxyMetrics() metrics {
 		ConnMigrationErrorRecoverableCount: metric.NewCounter(metaConnMigrationErrorRecoverableCount),
 		ConnMigrationAttemptedCount:        metric.NewCounter(metaConnMigrationAttemptedCount),
 		ConnMigrationAttemptedLatency: metric.NewHistogram(metric.HistogramOptions{
-			Mode:     metric.HistogramModePreferHdrLatency,
-			Metadata: metaConnMigrationAttemptedLatency,
-			Duration: base.DefaultHistogramWindowInterval(),
-			Buckets:  metric.IOLatencyBuckets,
+			Mode:         metric.HistogramModePreferHdrLatency,
+			Metadata:     metaConnMigrationAttemptedLatency,
+			Duration:     base.DefaultHistogramWindowInterval(),
+			BucketConfig: metric.IOLatencyBuckets,
 		}),
 		ConnMigrationTransferResponseMessageSize: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaConnMigrationTransferResponseMessageSize,
-			Duration: base.DefaultHistogramWindowInterval(),
-			Buckets:  metric.DataSize16MBBuckets,
-			MaxVal:   maxExpectedTransferResponseMessageSize,
-			SigFigs:  1,
+			Metadata:     metaConnMigrationTransferResponseMessageSize,
+			Duration:     base.DefaultHistogramWindowInterval(),
+			BucketConfig: metric.DataSize16MBBuckets,
+			MaxVal:       maxExpectedTransferResponseMessageSize,
+			SigFigs:      1,
 		}),
 		QueryCancelReceivedPGWire: metric.NewCounter(metaQueryCancelReceivedPGWire),
 		QueryCancelReceivedHTTP:   metric.NewCounter(metaQueryCancelReceivedHTTP),

pkg/ccl/streamingccl/streamingest/metrics.go

@@ -171,25 +171,25 @@ func MakeMetrics(histogramWindow time.Duration) metric.Struct {
 		JobProgressUpdates:   metric.NewCounter(metaJobProgressUpdates),
 		ReplanCount:          metric.NewCounter(metaDistSQLReplanCount),
 		FlushHistNanos: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaReplicationFlushHistNanos,
-			Duration: histogramWindow,
-			Buckets:  metric.BatchProcessLatencyBuckets,
-			MaxVal:   streamingFlushHistMaxLatency.Nanoseconds(),
-			SigFigs:  1,
+			Metadata:     metaReplicationFlushHistNanos,
+			Duration:     histogramWindow,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
+			MaxVal:       streamingFlushHistMaxLatency.Nanoseconds(),
+			SigFigs:      1,
 		}),
 		CommitLatency: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaReplicationCommitLatency,
-			Duration: histogramWindow,
-			Buckets:  metric.BatchProcessLatencyBuckets,
-			MaxVal:   streamingCommitLatencyMaxValue.Nanoseconds(),
-			SigFigs:  1,
+			Metadata:     metaReplicationCommitLatency,
+			Duration:     histogramWindow,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
+			MaxVal:       streamingCommitLatencyMaxValue.Nanoseconds(),
+			SigFigs:      1,
 		}),
 		AdmitLatency: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaReplicationAdmitLatency,
-			Duration: histogramWindow,
-			Buckets:  metric.BatchProcessLatencyBuckets,
-			MaxVal:   streamingAdmitLatencyMaxValue.Nanoseconds(),
-			SigFigs:  1,
+			Metadata:     metaReplicationAdmitLatency,
+			Duration:     histogramWindow,
+			BucketConfig: metric.BatchProcessLatencyBuckets,
+			MaxVal:       streamingAdmitLatencyMaxValue.Nanoseconds(),
+			SigFigs:      1,
 		}),
 		RunningCount:                metric.NewGauge(metaStreamsRunning),
 		EarliestDataCheckpointSpan:  metric.NewGauge(metaEarliestDataCheckpointSpan),

pkg/cmd/roachtest/tests/import_cancellation.go

@@ -29,27 +29,23 @@ import (
 )
 
 func registerImportCancellation(r registry.Registry) {
-	for _, rangeTombstones := range []bool{true, false} {
-		r.Add(registry.TestSpec{
-			Name:      fmt.Sprintf(`import-cancellation/rangeTs=%t`, rangeTombstones),
-			Owner:     registry.OwnerDisasterRecovery,
-			Benchmark: true,
-			Timeout:   4 * time.Hour,
-			Cluster:   r.MakeClusterSpec(6, spec.CPU(32)),
-			Leases:    registry.MetamorphicLeases,
-			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
-				if c.Spec().Cloud != spec.GCE {
-					t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968")
-				}
-				runImportCancellation(ctx, t, c, rangeTombstones)
-			},
-		})
-	}
+	r.Add(registry.TestSpec{
+		Name:      `import-cancellation`,
+		Owner:     registry.OwnerDisasterRecovery,
+		Benchmark: true,
+		Timeout:   4 * time.Hour,
+		Cluster:   r.MakeClusterSpec(6, spec.CPU(32)),
+		Leases:    registry.MetamorphicLeases,
+		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
+			if c.Spec().Cloud != spec.GCE {
+				t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968")
+			}
+			runImportCancellation(ctx, t, c)
+		},
+	})
 }
 
-func runImportCancellation(
-	ctx context.Context, t test.Test, c cluster.Cluster, rangeTombstones bool,
-) {
+func runImportCancellation(ctx context.Context, t test.Test, c cluster.Cluster) {
 	c.Put(ctx, t.Cockroach(), "./cockroach")
 	c.Put(ctx, t.DeprecatedWorkload(), "./workload") // required for tpch
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings())
@@ -81,15 +77,6 @@ func runImportCancellation(
 	if _, err := conn.Exec(`SET CLUSTER SETTING kv.bulk_ingest.max_index_buffer_size = '2gb'`); err != nil {
 		t.Fatal(err)
 	}
-	// Enable MVCC Range tombstones, if required.
-	rtEnable := "f"
-	if rangeTombstones {
-		rtEnable = "t"
-	}
-	stmt := fmt.Sprintf(`SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = '%s'`, rtEnable)
-	if _, err := conn.Exec(stmt); err != nil {
-		t.Fatal(err)
-	}
 
 	seed := int64(1666467482296309000)
 	rng := randutil.NewTestRandWithSeed(seed)

pkg/cmd/roachtest/tests/npgsql_blocklist.go

@@ -745,8 +745,14 @@ var npgsqlBlocklist = blocklist{
 }
 
 var npgsqlIgnoreList = blocklist{
+	`Npgsql.Tests.CommandTests(Multiplexing).QueryNonQuery`:                                           "flaky",
+	`Npgsql.Tests.CommandTests(Multiplexing).SingleNonQuery`:                                          "flaky",
 	`Npgsql.Tests.CommandTests(Multiplexing).Statement_mapped_output_parameters(Default)`:             "flaky",
+	`Npgsql.Tests.CommandTests(NonMultiplexing).Cached_command_clears_parameters_placeholder_type`:    "flaky",
+	`Npgsql.Tests.CommandTests(NonMultiplexing).CloseConnection_with_exception`:                       "flaky",
+	`Npgsql.Tests.CommandTests(NonMultiplexing).Cursor_move_RecordsAffected `:                         "flaky",
 	`Npgsql.Tests.CommandTests(NonMultiplexing).Statement_mapped_output_parameters(SequentialAccess)`: "flaky",
+	`Npgsql.Tests.CommandTests(NonMultiplexing).Use_across_connection_change(Prepared)`:               "flaky",
 	`Npgsql.Tests.ConnectionTests(NonMultiplexing).PostgreSqlVersion_ServerVersion`:                   "flaky",
 	`Npgsql.Tests.ConnectionTests(NonMultiplexing).Connector_not_initialized_exception`:               "flaky",
 	`Npgsql.Tests.ConnectionTests(NonMultiplexing).Many_open_close_with_transaction`:                  "flaky",

pkg/kv/bulk/bulk_metrics.go

@@ -52,11 +52,11 @@ const log10int64times1000 = 19 * 1000
 func MakeBulkMetrics(histogramWindow time.Duration) Metrics {
 	return Metrics{
 		MaxBytesHist: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaMemMaxBytes,
-			Duration: histogramWindow,
-			MaxVal:   log10int64times1000,
-			SigFigs:  3,
-			Buckets:  metric.MemoryUsage64MBBuckets,
+			Metadata:     metaMemMaxBytes,
+			Duration:     histogramWindow,
+			MaxVal:       log10int64times1000,
+			SigFigs:      3,
+			BucketConfig: metric.MemoryUsage64MBBuckets,
 		}),
 		CurBytesCount: metric.NewGauge(metaMemCurBytes),
 	}

pkg/kv/kvclient/kvcoord/txn_metrics.go

@@ -285,20 +285,20 @@ func MakeTxnMetrics(histogramWindow time.Duration) TxnMetrics {
 		ClientRefreshAutoRetries:            metric.NewCounter(metaClientRefreshAutoRetries),
 		ServerRefreshSuccess:                metric.NewCounter(metaServerRefreshSuccess),
 		Durations: metric.NewHistogram(metric.HistogramOptions{
-			Mode:     metric.HistogramModePreferHdrLatency,
-			Metadata: metaDurationsHistograms,
-			Duration: histogramWindow,
-			Buckets:  metric.IOLatencyBuckets,
+			Mode:         metric.HistogramModePreferHdrLatency,
+			Metadata:     metaDurationsHistograms,
+			Duration:     histogramWindow,
+			BucketConfig: metric.IOLatencyBuckets,
 		}),
 		TxnsWithCondensedIntents:      metric.NewCounter(metaTxnsWithCondensedIntentSpans),
 		TxnsWithCondensedIntentsGauge: metric.NewGauge(metaTxnsWithCondensedIntentSpansGauge),
 		TxnsRejectedByLockSpanBudget:  metric.NewCounter(metaTxnsRejectedByLockSpanBudget),
 		Restarts: metric.NewHistogram(metric.HistogramOptions{
-			Metadata: metaRestartsHistogram,
-			Duration: histogramWindow,
-			MaxVal:   100,
-			SigFigs:  3,
-			Buckets:  metric.Count1KBuckets,
+			Metadata:     metaRestartsHistogram,
+			Duration:     histogramWindow,
+			MaxVal:       100,
+			SigFigs:      3,
+			BucketConfig: metric.Count1KBuckets,
 		}),
 		RestartsWriteTooOld:            telemetry.NewCounterWithMetric(metaRestartsWriteTooOld),
 		RestartsWriteTooOldMulti:       telemetry.NewCounterWithMetric(metaRestartsWriteTooOldMulti),