Merge cockroachdb#61279 cockroachdb#61344

61279: kvserver: plug a tracing span leak r=irfansharif a=irfansharif Fixes cockroachdb#60677, removing a stop-gap introduced in cockroachdb#59992. We were previously leaking "async consensus" spans, which was possible when a given proposal was never flushed out of the replica's proposal buffer. On server shut down, this buffered proposal was never finished, and thus the embedded span never closed. We now add a closer to clean up after ourselves. Release justification: bug fixes and low-risk updates to new functionality. Release note: None --- +cc @angelapwen / @knz / @erikgrinaker for pod-visibility. 61344: sql: add estimated row count to all operators in EXPLAIN r=RaduBerinde a=RaduBerinde This change adds the estimated row count to all operators in the vanilla EXPLAIN, as long as the tables involved had statistics. Fixes cockroachdb#61310. Release justification: bug fixes and low-risk updates to new functionality Release note (sql change): EXPLAIN now shows estimated row counts for all operators even without VERBOSE (except when we don't have statistics for the tables). Co-authored-by: irfan sharif <[email protected]> Co-authored-by: Radu Berinde <[email protected]>
nvanbenschoten · Mar 3, 2021 · 50641ee · 50641ee
3 parents a19dc18 + 3aea858 + c3c7d70
commit 50641ee
Show file tree

Hide file tree

Showing 21 changed files with 159 additions and 17 deletions.
diff --git a/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit b/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit
@@ -656,6 +656,7 @@ vectorized: true
 │           │     spans: FULL SCAN
 │           │
 │           └── • scan buffer
+│                 estimated row count: 1
 │                 label: buffer 1
 │
 └── • constraint-check
@@ -673,6 +674,7 @@ vectorized: true
             │     spans: FULL SCAN
             │
             └── • scan buffer
+                  estimated row count: 1
                   label: buffer 1
 
 statement ok

diff --git a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
@@ -687,6 +687,7 @@ vectorized: true
 │           │ pred: column15 != crdb_region
 │           │
 │           └── • cross join
+│               │ estimated row count: 3
 │               │
 │               ├── • values
 │               │     size: 1 column, 3 rows
@@ -705,6 +706,7 @@ vectorized: true
 │           │ pred: (column1 != pk) OR (column15 != crdb_region)
 │           │
 │           └── • cross join
+│               │ estimated row count: 3
 │               │
 │               ├── • values
 │               │     size: 1 column, 3 rows
@@ -723,6 +725,7 @@ vectorized: true
             │ pred: (column1 != pk) OR (column15 != crdb_region)
             │
             └── • cross join
+                │ estimated row count: 3
                 │
                 ├── • values
                 │     size: 1 column, 3 rows
@@ -831,6 +834,7 @@ vectorized: true
 │               │ locking strength: for update
 │               │
 │               └── • render
+│                   │ estimated row count: 2
 │                   │
 │                   └── • values
 │                         size: 5 columns, 2 rows
@@ -1156,6 +1160,7 @@ vectorized: true
             │ pred: (column1 != pk) OR (column10 != crdb_region_col)
             │
             └── • cross join
+                │ estimated row count: 3
                 │
                 ├── • values
                 │     size: 1 column, 3 rows

diff --git a/pkg/kv/kvserver/replica_raft.go b/pkg/kv/kvserver/replica_raft.go
@@ -135,13 +135,6 @@ func (r *Replica) evalAndPropose(
 		// Fork the proposal's context span so that the proposal's context
 		// can outlive the original proposer's context.
 		proposal.ctx, proposal.sp = tracing.ForkSpan(ctx, "async consensus")
-		{
-			// This span sometimes leaks. Disable it for the time being.
-			//
-			// Tracked in: https://github.com/cockroachdb/cockroach/issues/60677
-			proposal.sp.Finish()
-			proposal.sp = nil
-		}
 
 		// Signal the proposal's response channel immediately.
 		reply := *proposal.Local.Reply

diff --git a/pkg/kv/kvserver/store_raft.go b/pkg/kv/kvserver/store_raft.go
@@ -617,6 +617,26 @@ func (s *Store) processRaft(ctx context.Context) {
 	s.stopper.AddCloser(stop.CloserFn(func() {
 		s.cfg.Transport.Stop(s.StoreID())
 	}))
+
+	// We'll want to cancel all in-flight proposals. Proposals embed tracing
+	// spans in them, and we don't want to be leaking any.
+	s.stopper.AddCloser(stop.CloserFn(func() {
+		s.VisitReplicas(func(r *Replica) (more bool) {
+			r.mu.proposalBuf.FlushLockedWithoutProposing(ctx)
+			r.mu.Lock()
+			for k, prop := range r.mu.proposals {
+				delete(r.mu.proposals, k)
+				prop.finishApplication(
+					context.Background(),
+					proposalResult{
+						Err: roachpb.NewError(roachpb.NewAmbiguousResultError("store is stopping")),
+					},
+				)
+			}
+			r.mu.Unlock()
+			return true
+		})
+	}))
 }
 
 func (s *Store) raftTickLoop(ctx context.Context) {

diff --git a/pkg/sql/logictest/testdata/logic_test/explain_analyze_plans b/pkg/sql/logictest/testdata/logic_test/explain_analyze_plans
@@ -208,6 +208,7 @@ distribution: full
 vectorized: true
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • norows
 ·
@@ -330,6 +331,7 @@ network usage: <hidden>
             └── • filter
                 │ cluster nodes: <hidden>
                 │ actual row count: 1
+                │ estimated row count: 1
                 │ filter: column2 IS NOT NULL
                 │
                 └── • scan buffer

diff --git a/pkg/sql/logictest/testdata/logic_test/vectorize_threshold b/pkg/sql/logictest/testdata/logic_test/vectorize_threshold
@@ -66,6 +66,7 @@ distribution: full
 vectorized: false
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • scan
       estimated row count: 100 (100% of the table)
@@ -84,6 +85,7 @@ distribution: full
 vectorized: true
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • scan
       estimated row count: 100 (100% of the table)
@@ -114,6 +116,7 @@ distribution: full
 vectorized: true
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • scan
       estimated row count: 100,000 (100% of the table)
@@ -132,6 +135,7 @@ distribution: full
 vectorized: false
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • scan
       estimated row count: 100,000 (100% of the table)
@@ -150,6 +154,7 @@ distribution: full
 vectorized: true
 ·
 • merge join
+│ estimated row count: 100
 │ equality: (a) = (a)
 │ left cols are key
 │ right cols are key

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg b/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg
@@ -666,8 +666,10 @@ distribution: full
 vectorized: true
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • filter
+    │ estimated row count: 1
     │ filter: column2 > 3
     │
     └── • values
@@ -682,6 +684,7 @@ distribution: full
 vectorized: true
 ·
 • hash join
+│ estimated row count: 2
 │ equality: (column1) = (column1)
 │
 ├── • values

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/distsql_union b/pkg/sql/opt/exec/execbuilder/testdata/distsql_union
@@ -237,6 +237,7 @@ distribution: full
 vectorized: true
 ·
 • union
+│ estimated row count: 4
 │
 ├── • values
 │     size: 1 column, 2 rows

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain
@@ -1310,6 +1310,7 @@ distribution: local
 vectorized: true
 ·
 • group (scalar)
+│ estimated row count: 1
 │
 └── • values
       size: 2 columns, 2 rows

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/fk b/pkg/sql/opt/exec/execbuilder/testdata/fk
@@ -112,6 +112,7 @@ vectorized: true
             │ equality cols are key
             │
             └── • filter
+                │ estimated row count: 1
                 │ filter: column2 IS NOT NULL
                 │
                 └── • scan buffer
@@ -156,6 +157,7 @@ vectorized: true
             │ equality cols are key
             │
             └── • filter
+                │ estimated row count: 1
                 │ filter: (column2 IS NOT NULL) AND (column3 IS NOT NULL)
                 │
                 └── • scan buffer
@@ -204,6 +206,7 @@ vectorized: true
 │           │ equality cols are key
 │           │
 │           └── • filter
+│               │ estimated row count: 1
 │               │ filter: column2 IS NOT NULL
 │               │
 │               └── • scan buffer
@@ -219,6 +222,7 @@ vectorized: true
             │ equality cols are key
             │
             └── • filter
+                │ estimated row count: 1
                 │ filter: (column3 IS NOT NULL) AND (column4 IS NOT NULL)
                 │
                 └── • scan buffer
@@ -1210,6 +1214,7 @@ vectorized: true
     └── • error if rows
         │
         └── • hash join (anti)
+            │ estimated row count: 2
             │ equality: (column2) = (p)
             │ right cols are key
             │
@@ -1237,6 +1242,7 @@ vectorized: true
 │       │ label: buffer 1
 │       │
 │       └── • render
+│           │ estimated row count: 1
 │           │
 │           └── • scan
 │                 estimated row count: 1 (100% of the table)
@@ -1249,10 +1255,12 @@ vectorized: true
     └── • error if rows
         │
         └── • hash join (anti)
+            │ estimated row count: 0
             │ equality: (p_new) = (p)
             │ right cols are key
             │
             ├── • filter
+            │   │ estimated row count: 1
             │   │ filter: p_new IS NOT NULL
             │   │
             │   └── • scan buffer
@@ -1279,6 +1287,7 @@ vectorized: true
 │       │ label: buffer 1
 │       │
 │       └── • render
+│           │ estimated row count: 1
 │           │
 │           └── • scan
 │                 estimated row count: 1 (100% of the table)
@@ -1291,10 +1300,12 @@ vectorized: true
     └── • error if rows
         │
         └── • hash join (semi)
+            │ estimated row count: 0
             │ equality: (p) = (p)
             │ left cols are key
             │
             ├── • except
+            │   │ estimated row count: 1
             │   │
             │   ├── • scan buffer
             │   │     label: buffer 1
@@ -1331,10 +1342,12 @@ vectorized: true
     └── • error if rows
         │
         └── • hash join (semi)
+            │ estimated row count: 0
             │ equality: (p) = (p)
             │ left cols are key
             │
             ├── • scan buffer
+            │     estimated row count: 1
             │     label: buffer 1
             │
             └── • scan
@@ -1374,6 +1387,7 @@ vectorized: true
 │       │ label: buffer 1
 │       │
 │       └── • render
+│           │ estimated row count: 1
 │           │
 │           └── • scan
 │                 estimated row count: 1 (100% of the table)
@@ -1386,11 +1400,13 @@ vectorized: true
     └── • error if rows
         │
         └── • lookup join (anti)
+            │ estimated row count: 0
             │ table: p@primary
             │ equality: (p_new) = (p)
             │ equality cols are key
             │
             └── • filter
+                │ estimated row count: 1
                 │ filter: p_new IS NOT NULL
                 │
                 └── • scan buffer
@@ -1412,6 +1428,7 @@ vectorized: true
 │       │ label: buffer 1
 │       │
 │       └── • render
+│           │ estimated row count: 1
 │           │
 │           └── • scan
 │                 estimated row count: 1 (100% of the table)
@@ -1424,10 +1441,12 @@ vectorized: true
     └── • error if rows
         │
         └── • lookup join (semi)
+            │ estimated row count: 0
             │ table: c@c_p_idx
             │ equality: (p) = (p)
             │
             └── • except
+                │ estimated row count: 1
                 │
                 ├── • scan buffer
                 │     label: buffer 1
@@ -1459,10 +1478,12 @@ vectorized: true
     └── • error if rows
         │
         └── • lookup join (semi)
+            │ estimated row count: 0
             │ table: c@c_p_idx
             │ equality: (p) = (p)
             │
             └── • scan buffer
+                  estimated row count: 1
                   label: buffer 1
 
 statement ok
@@ -1500,8 +1521,10 @@ vectorized: true
 │       │ label: buffer 1
 │       │
 │       └── • render
+│           │ estimated row count: 10
 │           │
 │           └── • project set
+│               │ estimated row count: 10
 │               │
 │               └── • emptyrow
 │
@@ -1510,6 +1533,7 @@ vectorized: true
     └── • error if rows
         │
         └── • hash join (anti)
+            │ estimated row count: 0
             │ equality: (?column?) = (p)
             │ right cols are key
             │