Merge #63935

63935: changefeedccl: remove changefeed.poll_request_nanos r=stevendanna a=stevendanna This removes the poll_request_nanos metric. This metric was not being updated and as such would always report zero. This also removes a defunct entry from the chart catalog. Release note (api change): The changefeed.poll_request_nanos metric is no longer reported by the node status API, the crdb_internal.metrics table, or the prometheus endpoint. Co-authored-by: Steven Danna <[email protected]>
cockroachdb · Apr 21, 2021 · feb357a · feb357a
2 parents cc94126 + 68c4795
commit feb357a
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 34 deletions.
diff --git a/pkg/ccl/changefeedccl/kvfeed/metrics.go b/pkg/ccl/changefeedccl/kvfeed/metrics.go
@@ -14,8 +14,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 )
 
-const pollRequestNanosHistMaxLatency = time.Hour
-
 var (
 	metaChangefeedBufferEntriesIn = metric.Metadata{
 		Name:        "changefeed.buffer_entries.in",
@@ -29,45 +27,22 @@ var (
 		Measurement: "Entries",
 		Unit:        metric.Unit_COUNT,
 	}
-	metaChangefeedPollRequestNanos = metric.Metadata{
-		Name:        "changefeed.poll_request_nanos",
-		Help:        "Time spent fetching changes",
-		Measurement: "Nanoseconds",
-		Unit:        metric.Unit_NANOSECONDS,
-	}
 )
 
 // Metrics is a metric.Struct for kvfeed metrics.
 //
 // TODO(ajwerner): Make these metrics more reasonable given the removal of the
 // poller and polling in general.
 type Metrics struct {
-	BufferEntriesIn      *metric.Counter
-	BufferEntriesOut     *metric.Counter
-	PollRequestNanosHist *metric.Histogram
+	BufferEntriesIn  *metric.Counter
+	BufferEntriesOut *metric.Counter
 }
 
 // MakeMetrics constructs a Metrics struct with the provided histogram window.
 func MakeMetrics(histogramWindow time.Duration) Metrics {
 	return Metrics{
 		BufferEntriesIn:  metric.NewCounter(metaChangefeedBufferEntriesIn),
 		BufferEntriesOut: metric.NewCounter(metaChangefeedBufferEntriesOut),
-		// Metrics for changefeed performance debugging: - PollRequestNanos and
-		// PollRequestNanosHist, things are first
-		//   fetched with some limited concurrency. We're interested in both the
-		//   total amount of time fetching as well as outliers, so we need both
-		//   the counter and the histogram.
-		// - N/A. Each change is put into a buffer. Right now nothing measures
-		//   this since the buffer doesn't actually buffer and so it just tracks
-		//   the poll sleep time.
-		// - ProcessingNanos. Everything from the buffer until the SQL row is
-		//   about to be emitted. This includes TableMetadataNanos, which is
-		//   dependent on network calls, so also tracked in case it's ever the
-		//   cause of a ProcessingNanos blowup.
-		// - EmitNanos and FlushNanos. All of our interactions with the sink.
-		PollRequestNanosHist: metric.NewHistogram(
-			metaChangefeedPollRequestNanos, histogramWindow,
-			pollRequestNanosHistMaxLatency.Nanoseconds(), 1),
 	}
 }
 

diff --git a/pkg/ts/catalog/chart_catalog.go b/pkg/ts/catalog/chart_catalog.go
@@ -1086,12 +1086,6 @@ var charts = []sectionDescription{
 					"changefeed.min_high_water",
 				},
 			},
-			{
-				Title: "Poll Request Time",
-				Metrics: []string{
-					"changefeed.poll_request_nanos",
-				},
-			},
 			{
 				Title: "Currently Running",
 				Metrics: []string{

diff --git a/pkg/ui/src/util/query.spec.ts b/pkg/ui/src/util/query.spec.ts
@@ -15,7 +15,7 @@ import { Location } from "history";
 const location: Location = {
   pathname: "/debug/chart",
   search:
-    "?charts=%5B%7B%22metrics%22%3A%5B%7B%22downsampler%22%3A1%2C%22aggregator%22%3A2%2C%22derivative%22%3A0%2C%22perNode%22%3Afalse%2C%22source%22%3A%22%22%2C%22metric%22%3A%22cr.node.build.timestamp%22%7D%2C%7B%22downsampler%22%3A1%2C%22aggregator%22%3A2%2C%22derivative%22%3A0%2C%22perNode%22%3Afalse%2C%22source%22%3A%22%22%2C%22metric%22%3A%22cr.node.changefeed.poll_request_nanos-p50%22%7D%5D%2C%22axisUnits%22%3A0%7D%5D&start=1581478532&end=1581500132",
+    "?charts=%5B%7B%22metrics%22%3A%5B%7B%22downsampler%22%3A1%2C%22aggregator%22%3A2%2C%22derivative%22%3A0%2C%22perNode%22%3Afalse%2C%22source%22%3A%22%22%2C%22metric%22%3A%22cr.node.build.timestamp%22%7D%2C%7B%22downsampler%22%3A1%2C%22aggregator%22%3A2%2C%22derivative%22%3A0%2C%22perNode%22%3Afalse%2C%22source%22%3A%22%22%2C%22metric%22%3A%22cr.node.changefeed.some_metric-p50%22%7D%5D%2C%22axisUnits%22%3A0%7D%5D&start=1581478532&end=1581500132",
   hash: "",
   state: null,
   key: null,