Merge #56842

56842: sql: move ComponentStats to execinfrapb r=RaduBerinde a=RaduBerinde This proto belongs in execinfrapb. For example, we should be able to store ProcessorID/StreamIDs in it if necessary. The differences in the proto file are due to proto2/proto3. The DistSQL version doesn't need bumping, the messages are different (because of the package change) and unknown messages will just be ignored. Release note: None Co-authored-by: Radu Berinde <[email protected]>
cockroachdb · Nov 18, 2020 · d8aad35 · d8aad35
2 parents 8a14e28 + f26024d
commit d8aad35
Show file tree

Hide file tree

Showing 36 changed files with 368 additions and 429 deletions.
diff --git a/pkg/sql/colexec/BUILD.bazel b/pkg/sql/colexec/BUILD.bazel
@@ -72,7 +72,6 @@ go_library(
         "//pkg/sql/colmem",
         "//pkg/sql/execinfra",
         "//pkg/sql/execinfrapb",
-        "//pkg/sql/execstats/execstatspb",
         "//pkg/sql/parser",
         "//pkg/sql/rowenc",
         "//pkg/sql/sem/tree",

diff --git a/pkg/sql/colexec/materializer.go b/pkg/sql/colexec/materializer.go
@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/colexecbase/colexecerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
-	"github.com/cockroachdb/cockroach/pkg/sql/execstats/execstatspb"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/errors"
@@ -170,7 +169,7 @@ func NewMaterializer(
 	output execinfra.RowReceiver,
 	metadataSourcesQueue []execinfrapb.MetadataSource,
 	toClose []colexecbase.Closer,
-	execStatsForTrace func() *execstatspb.ComponentStats,
+	execStatsForTrace func() *execinfrapb.ComponentStats,
 	cancelFlow func() context.CancelFunc,
 ) (*Materializer, error) {
 	vecIdxsToConvert := make([]int, len(typs))

diff --git a/pkg/sql/colexec/stats.go b/pkg/sql/colexec/stats.go
@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/colexecbase/colexecerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
-	"github.com/cockroachdb/cockroach/pkg/sql/execstats/execstatspb"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
@@ -166,10 +165,10 @@ type vectorizedStatsCollectorImpl struct {
 }
 
 // finish returns the collected stats.
-func (vsc *vectorizedStatsCollectorImpl) finish() *execstatspb.ComponentStats {
+func (vsc *vectorizedStatsCollectorImpl) finish() *execinfrapb.ComponentStats {
 	numBatches, numTuples, time := vsc.batchInfoCollector.finish()
 
-	s := &execstatspb.ComponentStats{ComponentID: vsc.operatorID}
+	s := &execinfrapb.ComponentStats{ComponentID: vsc.operatorID}
 
 	for _, memMon := range vsc.memMonitors {
 		s.Exec.MaxAllocatedMem.Add(memMon.MaximumBytes())
@@ -250,10 +249,10 @@ type networkVectorizedStatsCollectorImpl struct {
 }
 
 // finish returns the collected stats.
-func (nvsc *networkVectorizedStatsCollectorImpl) finish() *execstatspb.ComponentStats {
+func (nvsc *networkVectorizedStatsCollectorImpl) finish() *execinfrapb.ComponentStats {
 	numBatches, numTuples, time := nvsc.batchInfoCollector.finish()
 
-	s := &execstatspb.ComponentStats{ComponentID: nvsc.operatorID}
+	s := &execinfrapb.ComponentStats{ComponentID: nvsc.operatorID}
 
 	s.NetRx.Latency = nvsc.latency
 	s.NetRx.WaitTime = time
@@ -283,7 +282,7 @@ func createStatsSpan(
 	opName string,
 	flowID string,
 	idTagKey string,
-	stats *execstatspb.ComponentStats,
+	stats *execinfrapb.ComponentStats,
 ) {
 	// We're creating a new span for every component setting the appropriate
 	// tag so that it is displayed correctly on the flow diagram.

diff --git a/pkg/sql/colflow/BUILD.bazel b/pkg/sql/colflow/BUILD.bazel
@@ -20,7 +20,6 @@ go_library(
         "//pkg/sql/colmem",
         "//pkg/sql/execinfra",
         "//pkg/sql/execinfrapb",
-        "//pkg/sql/execstats/execstatspb",
         "//pkg/sql/flowinfra",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",

diff --git a/pkg/sql/colflow/vectorized_flow.go b/pkg/sql/colflow/vectorized_flow.go
@@ -35,7 +35,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/colmem"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
-	"github.com/cockroachdb/cockroach/pkg/sql/execstats/execstatspb"
 	"github.com/cockroachdb/cockroach/pkg/sql/flowinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
@@ -993,12 +992,12 @@ func (s *vectorizedFlowCreator) setupOutput(
 		s.leaves = append(s.leaves, outbox)
 	case execinfrapb.StreamEndpointSpec_SYNC_RESPONSE:
 		// Make the materializer, which will write to the given receiver.
-		var outputStatsToTrace func() *execstatspb.ComponentStats
+		var outputStatsToTrace func() *execinfrapb.ComponentStats
 		if s.recordingStats {
 			// Make a copy given that vectorizedStatsCollectorsQueue is reset and
 			// appended to.
 			vscq := append([]colexec.VectorizedStatsCollector(nil), s.vectorizedStatsCollectorsQueue...)
-			outputStatsToTrace = func() *execstatspb.ComponentStats {
+			outputStatsToTrace = func() *execinfrapb.ComponentStats {
 				// TODO(radu): this is a sketchy way to use this infrastructure. We
 				// aren't actually returning any stats, but we are creating and closing
 				// child spans with stats.

diff --git a/pkg/sql/execinfra/BUILD.bazel b/pkg/sql/execinfra/BUILD.bazel
@@ -41,7 +41,6 @@ go_library(
         "//pkg/sql/catalog/descs",
         "//pkg/sql/catalog/hydratedtables",
         "//pkg/sql/execinfrapb",
-        "//pkg/sql/execstats/execstatspb",
         "//pkg/sql/rowenc",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sqlliveness",

diff --git a/pkg/sql/execinfra/processorsbase.go b/pkg/sql/execinfra/processorsbase.go
@@ -16,7 +16,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
-	"github.com/cockroachdb/cockroach/pkg/sql/execstats/execstatspb"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
@@ -342,9 +341,9 @@ func (h *ProcOutputHelper) consumerClosed() {
 }
 
 // Stats returns output statistics.
-func (h *ProcOutputHelper) Stats() execstatspb.OutputStats {
-	return execstatspb.OutputStats{
-		NumTuples: execstatspb.MakeIntValue(h.rowIdx),
+func (h *ProcOutputHelper) Stats() execinfrapb.OutputStats {
+	return execinfrapb.OutputStats{
+		NumTuples: execinfrapb.MakeIntValue(h.rowIdx),
 	}
 }
 
@@ -497,7 +496,7 @@ type ProcessorBase struct {
 	// MakeDeterministic might get called on the returned stats.
 	//
 	// Can return nil.
-	ExecStatsForTrace func() *execstatspb.ComponentStats
+	ExecStatsForTrace func() *execinfrapb.ComponentStats
 
 	// trailingMetaCallback, if set, will be called by moveToTrailingMeta(). The
 	// callback is expected to close all inputs, do other cleanup on the processor

diff --git a/pkg/sql/execinfrapb/BUILD.bazel b/pkg/sql/execinfrapb/BUILD.bazel
@@ -1,14 +1,40 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
+go_library(
+    name = "execstatspb",
+    srcs = [
+        "component_stats.go",
+        "component_stats.pb.go",
+        "int_value.go",
+    ],
+    importpath = "github.com/cockroachdb/cockroach/pkg/sql/execstats/execstatspb",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//vendor/github.com/dustin/go-humanize",
+        "//vendor/github.com/gogo/protobuf/proto",
+        "//vendor/github.com/gogo/protobuf/types",
+    ],
+)
+
+go_test(
+    name = "execstatspb_test",
+    srcs = ["int_value_test.go"],
+    embed = [":execstatspb"],
+    deps = ["//vendor/github.com/stretchr/testify/require"],
+)
+
 go_library(
     name = "execinfrapb",
     srcs = [
         "api.go",
         "api.pb.go",
+        "component_stats.go",
+        "component_stats.pb.go",
         "data.go",
         "data.pb.go",
         "expr.go",
         "flow_diagram.go",
+        "int_value.go",
         "processors.go",
         "processors.pb.go",
         "processors_base.pb.go",
@@ -71,6 +97,7 @@ go_test(
     srcs = [
         "expr_test.go",
         "flow_diagram_test.go",
+        "int_value_test.go",
     ],
     embed = [":execinfrapb"],
     deps = [
@@ -80,5 +107,6 @@ go_test(
         "//pkg/sql/sem/tree",
         "//pkg/sql/types",
         "//pkg/util/leaktest",
+        "//vendor/github.com/stretchr/testify/require",
     ],
 )
diff --git a/.../execstats/execstatspb/component_stats.go → pkg/sql/execinfrapb/component_stats.go b/.../execstats/execstatspb/component_stats.go → pkg/sql/execinfrapb/component_stats.go
@@ -8,12 +8,12 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package execstatspb
+package execinfrapb
 
 import (
-	fmt "fmt"
+	"fmt"
 	"strings"
-	time "time"
+	"time"
 
 	"github.com/dustin/go-humanize"
 )