From 4eea1572d5fb83e413cb6ff6c840be2ccb690451 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Mon, 9 Jul 2018 08:27:56 -0700
Subject: [PATCH 01/12] Keep track of QueryStats in the QueryEngine instead of
 the TabletPlan so that it doesn't get LRU evicted as part of the
 TabletPlanCache.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go   | 131 +++++++++---------
 go/vt/vttablet/tabletserver/query_executor.go |   4 +-
 go/vt/vttablet/tabletserver/queryz.go         |   7 +-
 go/vt/vttablet/tabletserver/queryz_test.go    |   8 +-
 4 files changed, 79 insertions(+), 71 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index dfe4006a5a8..cd7d7157885 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -62,13 +62,6 @@ type TabletPlan struct {
 	Rules            *rules.Rules
 	LegacyAuthorized *tableacl.ACLResult
 	Authorized       []*tableacl.ACLResult
-
-	mu         sync.Mutex
-	QueryCount int64
-	Time       time.Duration
-	MysqlTime  time.Duration
-	RowCount   int64
-	ErrorCount int64
 }
 
 // Size allows TabletPlan to be in cache.LRUCache.
@@ -76,29 +69,6 @@ func (*TabletPlan) Size() int {
 	return 1
 }
 
-// AddStats updates the stats for the current TabletPlan.
-func (ep *TabletPlan) AddStats(queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	ep.mu.Lock()
-	ep.QueryCount += queryCount
-	ep.Time += duration
-	ep.MysqlTime += mysqlTime
-	ep.RowCount += rowCount
-	ep.ErrorCount += errorCount
-	ep.mu.Unlock()
-}
-
-// Stats returns the current stats of TabletPlan.
-func (ep *TabletPlan) Stats() (queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	ep.mu.Lock()
-	queryCount = ep.QueryCount
-	duration = ep.Time
-	mysqlTime = ep.MysqlTime
-	rowCount = ep.RowCount
-	errorCount = ep.ErrorCount
-	ep.mu.Unlock()
-	return
-}
-
 // buildAuthorized builds 'Authorized', which is the runtime part for 'Permissions'.
 func (ep *TabletPlan) buildAuthorized() {
 	ep.Authorized = make([]*tableacl.ACLResult, len(ep.Permissions))
@@ -126,6 +96,9 @@ type QueryEngine struct {
 	plans            *cache.LRUCache
 	queryRuleSources *rules.Map
 
+	queryStatsMu sync.RWMutex
+	queryStats   map[string]*QueryStats
+
 	// Pools
 	conns       *connpool.Pool
 	streamConns *connpool.Pool
@@ -182,6 +155,7 @@ func NewQueryEngine(checker connpool.MySQLChecker, se *schema.Engine, config tab
 		plans:              cache.NewLRUCache(int64(config.QueryPlanCacheSize)),
 		queryRuleSources:   rules.NewMap(),
 		queryPoolWaiterCap: sync2.NewAtomicInt64(int64(config.QueryPoolWaiterCap)),
+		queryStats:         make(map[string]*QueryStats),
 	}
 
 	qe.conns = connpool.New(
@@ -478,53 +452,75 @@ func (qe *QueryEngine) QueryPlanCacheCap() int {
 	return int(qe.plans.Capacity())
 }
 
-func (qe *QueryEngine) getQueryCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		queryCount, _, _, _, _ := plan.Stats()
-		return queryCount
+type QueryStats struct {
+	queryCount int64
+	time       time.Duration
+	mysqlTime  time.Duration
+	rowCount   int64
+	errorCount int64
+}
+
+func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	qe.queryStatsMu.Lock()
+	defer qe.queryStatsMu.Unlock()
+
+	qstats := &QueryStats{
+		queryCount: queryCount,
+		time:       duration,
+		mysqlTime:  mysqlTime,
+		rowCount:   rowCount,
+		errorCount: errorCount}
+
+	if stats, ok := qe.queryStats[planName+"."+tableName]; ok {
+		qstats = &QueryStats{
+			queryCount: stats.queryCount + queryCount,
+			time:       stats.time + duration,
+			mysqlTime:  stats.mysqlTime + mysqlTime,
+			rowCount:   stats.rowCount + rowCount,
+			errorCount: stats.errorCount + errorCount}
 	}
-	return qe.getQueryStats(f)
+
+	qe.queryStats[planName+"."+tableName] = qstats
 }
 
-func (qe *QueryEngine) getQueryTime() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, time, _, _, _ := plan.Stats()
-		return int64(time)
+func (qe *QueryEngine) Stats(planName, tableName string) *QueryStats {
+	qe.queryStatsMu.Lock()
+	defer qe.queryStatsMu.Unlock()
+	s, ok := qe.queryStats[planName+"."+tableName]
+	if !ok {
+		return nil
 	}
-	return qe.getQueryStats(f)
+	return s
 }
 
-func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, _, _, rowCount, _ := plan.Stats()
-		return rowCount
+func (qe *QueryEngine) getQueryCount() map[string]int64 {
+	qstats := make(map[string]int64)
+	for k, qs := range qe.queryStats {
+		qstats[k] = qs.queryCount
 	}
-	return qe.getQueryStats(f)
+	return qstats
 }
 
-func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, _, _, _, errorCount := plan.Stats()
-		return errorCount
+func (qe *QueryEngine) getQueryTime() map[string]int64 {
+	qstats := make(map[string]int64)
+	for k, qs := range qe.queryStats {
+		qstats[k] = int64(qs.time)
 	}
-	return qe.getQueryStats(f)
+	return qstats
 }
 
-type queryStatsFunc func(*TabletPlan) int64
+func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
+	qstats := make(map[string]int64)
+	for k, qs := range qe.queryStats {
+		qstats[k] = qs.rowCount
+	}
+	return qstats
+}
 
-func (qe *QueryEngine) getQueryStats(f queryStatsFunc) map[string]int64 {
-	keys := qe.plans.Keys()
+func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
 	qstats := make(map[string]int64)
-	for _, v := range keys {
-		if plan := qe.peekQuery(v); plan != nil {
-			table := plan.TableName()
-			if table.IsEmpty() {
-				table = sqlparser.NewTableIdent("Join")
-			}
-			planType := plan.PlanID.String()
-			data := f(plan)
-			qstats[table.String()+"."+planType] += data
-		}
+	for k, qs := range qe.queryStats {
+		qstats[k] = qs.errorCount
 	}
 	return qstats
 }
@@ -588,7 +584,14 @@ func (qe *QueryEngine) handleHTTPQueryStats(response http.ResponseWriter, reques
 			pqstats.Query = unicoded(sqlparser.TruncateForUI(v))
 			pqstats.Table = plan.TableName().String()
 			pqstats.Plan = plan.PlanID
-			pqstats.QueryCount, pqstats.Time, pqstats.MysqlTime, pqstats.RowCount, pqstats.ErrorCount = plan.Stats()
+			if stats := qe.Stats(pqstats.Plan.String(), pqstats.Table); stats != nil {
+				pqstats.QueryCount = stats.queryCount
+				pqstats.Time = stats.time
+				pqstats.MysqlTime = stats.mysqlTime
+				pqstats.RowCount = stats.rowCount
+				pqstats.ErrorCount = stats.errorCount
+			}
+
 			qstats = append(qstats, pqstats)
 		}
 	}
diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index 7b754c9bff6..34e650ecfae 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -80,10 +80,10 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
 		if reply == nil {
-			qre.plan.AddStats(1, duration, qre.logStats.MysqlResponseTime, 0, 1)
+			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
 			return
 		}
-		qre.plan.AddStats(1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
+		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows
 		tabletenv.ResultStats.Add(int64(len(reply.Rows)))
diff --git a/go/vt/vttablet/tabletserver/queryz.go b/go/vt/vttablet/tabletserver/queryz.go
index 21b5c482e25..fac9c193885 100644
--- a/go/vt/vttablet/tabletserver/queryz.go
+++ b/go/vt/vttablet/tabletserver/queryz.go
@@ -155,7 +155,12 @@ func queryzHandler(qe *QueryEngine, w http.ResponseWriter, r *http.Request) {
 			Plan:   plan.PlanID,
 			Reason: plan.Reason,
 		}
-		Value.Count, Value.tm, Value.mysqlTime, Value.Rows, Value.Errors = plan.Stats()
+		qs := qe.Stats(plan.PlanID.String(), plan.TableName().String())
+		Value.Count = qs.queryCount
+		Value.tm = qs.time
+		Value.mysqlTime = qs.mysqlTime
+		Value.Rows = qs.rowCount
+		Value.Errors = qs.errorCount
 		var timepq time.Duration
 		if Value.Count != 0 {
 			timepq = time.Duration(int64(Value.tm) / Value.Count)
diff --git a/go/vt/vttablet/tabletserver/queryz_test.go b/go/vt/vttablet/tabletserver/queryz_test.go
index 43cc7d384f3..b31d9f9656d 100644
--- a/go/vt/vttablet/tabletserver/queryz_test.go
+++ b/go/vt/vttablet/tabletserver/queryz_test.go
@@ -44,7 +44,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonTable,
 		},
 	}
-	plan1.AddStats(10, 2*time.Second, 1*time.Second, 2, 0)
+	qe.AddStats(plan1.PlanID.String(), "test_table", 10, 2*time.Second, 1*time.Second, 2, 0)
 	qe.plans.Set("select name from test_table", plan1)
 
 	plan2 := &TabletPlan{
@@ -54,7 +54,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	plan2.AddStats(1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan2.PlanID.String(), "test_table", 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
 	qe.plans.Set("insert into test_table values 1", plan2)
 
 	plan3 := &TabletPlan{
@@ -64,7 +64,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	plan3.AddStats(1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
+	qe.AddStats(plan3.PlanID.String(), "", 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
 	qe.plans.Set("show tables", plan3)
 	qe.plans.Set("", (*TabletPlan)(nil))
 
@@ -75,7 +75,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	plan4.AddStats(1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan4.PlanID.String(), "", 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
 	hugeInsert := "insert into test_table values 0"
 	for i := 1; i < 1000; i++ {
 		hugeInsert = hugeInsert + fmt.Sprintf(", %d", i)

From ef35d3822430142eb88e5ec9580e1cf58be01ad5 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Wed, 18 Jul 2018 17:52:21 -0700
Subject: [PATCH 02/12] golint

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index cd7d7157885..8829b2020d2 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -452,6 +452,7 @@ func (qe *QueryEngine) QueryPlanCacheCap() int {
 	return int(qe.plans.Capacity())
 }
 
+// QueryStats tracks query stats for export per planName/tableName
 type QueryStats struct {
 	queryCount int64
 	time       time.Duration
@@ -460,6 +461,7 @@ type QueryStats struct {
 	errorCount int64
 }
 
+// AddStats adds the given stats for the planName.tableName
 func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
 	qe.queryStatsMu.Lock()
 	defer qe.queryStatsMu.Unlock()
@@ -483,6 +485,7 @@ func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, du
 	qe.queryStats[planName+"."+tableName] = qstats
 }
 
+// Stats returns the stored QueryStats for the planName.tableName
 func (qe *QueryEngine) Stats(planName, tableName string) *QueryStats {
 	qe.queryStatsMu.Lock()
 	defer qe.queryStatsMu.Unlock()

From 1347656e03f787e652aee8d9b7c3027e3649406c Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Mon, 13 Aug 2018 17:49:20 -0700
Subject: [PATCH 03/12] Respond to @demmer's IRL feedback to use a different
 locking strategy for the QueryStats map that optimizes for the fact that
 there are many writers to the shared QueryStats map but few readers.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go | 39 ++++++++++++---------
 go/vt/vttablet/tabletserver/queryz.go       |  2 ++
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index 8829b2020d2..023165752bc 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -454,6 +454,7 @@ func (qe *QueryEngine) QueryPlanCacheCap() int {
 
 // QueryStats tracks query stats for export per planName/tableName
 type QueryStats struct {
+	mu         sync.Mutex
 	queryCount int64
 	time       time.Duration
 	mysqlTime  time.Duration
@@ -463,26 +464,30 @@ type QueryStats struct {
 
 // AddStats adds the given stats for the planName.tableName
 func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	qe.queryStatsMu.Lock()
-	defer qe.queryStatsMu.Unlock()
+	key := planName + "." + tableName
 
-	qstats := &QueryStats{
-		queryCount: queryCount,
-		time:       duration,
-		mysqlTime:  mysqlTime,
-		rowCount:   rowCount,
-		errorCount: errorCount}
+	qe.queryStatsMu.RLock()
+	stats, ok := qe.queryStats[key]
+	qe.queryStatsMu.RUnlock()
 
-	if stats, ok := qe.queryStats[planName+"."+tableName]; ok {
-		qstats = &QueryStats{
-			queryCount: stats.queryCount + queryCount,
-			time:       stats.time + duration,
-			mysqlTime:  stats.mysqlTime + mysqlTime,
-			rowCount:   stats.rowCount + rowCount,
-			errorCount: stats.errorCount + errorCount}
+	if !ok {
+		// Check again with the write lock held and
+		// create a new record only if none exists
+		qe.queryStatsMu.Lock()
+		if stats, ok = qe.queryStats[key]; !ok {
+			stats = &QueryStats{}
+			qe.queryStats[key] = stats
+		}
+		qe.queryStatsMu.Unlock()
 	}
 
-	qe.queryStats[planName+"."+tableName] = qstats
+	stats.mu.Lock()
+	stats.queryCount += queryCount
+	stats.time += duration
+	stats.mysqlTime += mysqlTime
+	stats.rowCount += rowCount
+	stats.errorCount += errorCount
+	stats.mu.Unlock()
 }
 
 // Stats returns the stored QueryStats for the planName.tableName
@@ -588,11 +593,13 @@ func (qe *QueryEngine) handleHTTPQueryStats(response http.ResponseWriter, reques
 			pqstats.Table = plan.TableName().String()
 			pqstats.Plan = plan.PlanID
 			if stats := qe.Stats(pqstats.Plan.String(), pqstats.Table); stats != nil {
+				stats.mu.Lock()
 				pqstats.QueryCount = stats.queryCount
 				pqstats.Time = stats.time
 				pqstats.MysqlTime = stats.mysqlTime
 				pqstats.RowCount = stats.rowCount
 				pqstats.ErrorCount = stats.errorCount
+				stats.mu.Unlock()
 			}
 
 			qstats = append(qstats, pqstats)
diff --git a/go/vt/vttablet/tabletserver/queryz.go b/go/vt/vttablet/tabletserver/queryz.go
index fac9c193885..0e0e0545987 100644
--- a/go/vt/vttablet/tabletserver/queryz.go
+++ b/go/vt/vttablet/tabletserver/queryz.go
@@ -156,11 +156,13 @@ func queryzHandler(qe *QueryEngine, w http.ResponseWriter, r *http.Request) {
 			Reason: plan.Reason,
 		}
 		qs := qe.Stats(plan.PlanID.String(), plan.TableName().String())
+		qs.mu.Lock()
 		Value.Count = qs.queryCount
 		Value.tm = qs.time
 		Value.mysqlTime = qs.mysqlTime
 		Value.Rows = qs.rowCount
 		Value.Errors = qs.errorCount
+		qs.mu.Unlock()
 		var timepq time.Duration
 		if Value.Count != 0 {
 			timepq = time.Duration(int64(Value.tm) / Value.Count)

From 202066de19faab734ab23dfca39ff9f455b6e19d Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Mon, 13 Aug 2018 18:03:32 -0700
Subject: [PATCH 04/12] Use the per-stat mutex on getters as well.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index 023165752bc..9a2dff5c9c2 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -504,7 +504,9 @@ func (qe *QueryEngine) Stats(planName, tableName string) *QueryStats {
 func (qe *QueryEngine) getQueryCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	for k, qs := range qe.queryStats {
+		qs.mu.Lock()
 		qstats[k] = qs.queryCount
+		qs.mu.Unlock()
 	}
 	return qstats
 }
@@ -512,7 +514,9 @@ func (qe *QueryEngine) getQueryCount() map[string]int64 {
 func (qe *QueryEngine) getQueryTime() map[string]int64 {
 	qstats := make(map[string]int64)
 	for k, qs := range qe.queryStats {
+		qs.mu.Lock()
 		qstats[k] = int64(qs.time)
+		qs.mu.Unlock()
 	}
 	return qstats
 }
@@ -520,7 +524,9 @@ func (qe *QueryEngine) getQueryTime() map[string]int64 {
 func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	for k, qs := range qe.queryStats {
+		qs.mu.Lock()
 		qstats[k] = qs.rowCount
+		qs.mu.Unlock()
 	}
 	return qstats
 }
@@ -528,7 +534,9 @@ func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	for k, qs := range qe.queryStats {
+		qs.mu.Lock()
 		qstats[k] = qs.errorCount
+		qs.mu.Unlock()
 	}
 	return qstats
 }

From ac0f65fd88de1ce1703d37e716d0454cd306db22 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Mon, 13 Aug 2018 19:34:01 -0700
Subject: [PATCH 05/12] Key QueryStats including the sql query string, to power
 `/debug/query_stats`

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go   | 12 ++++++------
 go/vt/vttablet/tabletserver/query_executor.go |  4 ++--
 go/vt/vttablet/tabletserver/queryz.go         |  2 +-
 go/vt/vttablet/tabletserver/queryz_test.go    |  8 ++++----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index 9a2dff5c9c2..272b8d09482 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -463,8 +463,8 @@ type QueryStats struct {
 }
 
 // AddStats adds the given stats for the planName.tableName
-func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	key := planName + "." + tableName
+func (qe *QueryEngine) AddStats(planName, tableName, query string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	key := planName + "." + tableName + "." + query
 
 	qe.queryStatsMu.RLock()
 	stats, ok := qe.queryStats[key]
@@ -490,11 +490,11 @@ func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, du
 	stats.mu.Unlock()
 }
 
-// Stats returns the stored QueryStats for the planName.tableName
-func (qe *QueryEngine) Stats(planName, tableName string) *QueryStats {
+// Stats returns the stored QueryStats for the planName.tableName.query
+func (qe *QueryEngine) Stats(planName, tableName, query string) *QueryStats {
 	qe.queryStatsMu.Lock()
 	defer qe.queryStatsMu.Unlock()
-	s, ok := qe.queryStats[planName+"."+tableName]
+	s, ok := qe.queryStats[planName+"."+tableName+"."+query]
 	if !ok {
 		return nil
 	}
@@ -600,7 +600,7 @@ func (qe *QueryEngine) handleHTTPQueryStats(response http.ResponseWriter, reques
 			pqstats.Query = unicoded(sqlparser.TruncateForUI(v))
 			pqstats.Table = plan.TableName().String()
 			pqstats.Plan = plan.PlanID
-			if stats := qe.Stats(pqstats.Plan.String(), pqstats.Table); stats != nil {
+			if stats := qe.Stats(pqstats.Plan.String(), pqstats.Table, pqstats.Query); stats != nil {
 				stats.mu.Lock()
 				pqstats.QueryCount = stats.queryCount
 				pqstats.Time = stats.time
diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index 34e650ecfae..acba0ae4947 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -80,10 +80,10 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
 		if reply == nil {
-			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
+			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), qre.query, 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
 			return
 		}
-		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
+		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), qre.query, 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows
 		tabletenv.ResultStats.Add(int64(len(reply.Rows)))
diff --git a/go/vt/vttablet/tabletserver/queryz.go b/go/vt/vttablet/tabletserver/queryz.go
index 0e0e0545987..368effe4a7e 100644
--- a/go/vt/vttablet/tabletserver/queryz.go
+++ b/go/vt/vttablet/tabletserver/queryz.go
@@ -155,7 +155,7 @@ func queryzHandler(qe *QueryEngine, w http.ResponseWriter, r *http.Request) {
 			Plan:   plan.PlanID,
 			Reason: plan.Reason,
 		}
-		qs := qe.Stats(plan.PlanID.String(), plan.TableName().String())
+		qs := qe.Stats(plan.PlanID.String(), plan.TableName().String(), v)
 		qs.mu.Lock()
 		Value.Count = qs.queryCount
 		Value.tm = qs.time
diff --git a/go/vt/vttablet/tabletserver/queryz_test.go b/go/vt/vttablet/tabletserver/queryz_test.go
index b31d9f9656d..891bfc934f0 100644
--- a/go/vt/vttablet/tabletserver/queryz_test.go
+++ b/go/vt/vttablet/tabletserver/queryz_test.go
@@ -44,7 +44,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonTable,
 		},
 	}
-	qe.AddStats(plan1.PlanID.String(), "test_table", 10, 2*time.Second, 1*time.Second, 2, 0)
+	qe.AddStats(plan1.PlanID.String(), "test_table", plan1.Query, 10, 2*time.Second, 1*time.Second, 2, 0)
 	qe.plans.Set("select name from test_table", plan1)
 
 	plan2 := &TabletPlan{
@@ -54,7 +54,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan2.PlanID.String(), "test_table", 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan2.PlanID.String(), "test_table", plan2.Query, 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
 	qe.plans.Set("insert into test_table values 1", plan2)
 
 	plan3 := &TabletPlan{
@@ -64,7 +64,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan3.PlanID.String(), "", 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
+	qe.AddStats(plan3.PlanID.String(), "", plan3.Query, 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
 	qe.plans.Set("show tables", plan3)
 	qe.plans.Set("", (*TabletPlan)(nil))
 
@@ -75,7 +75,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan4.PlanID.String(), "", 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan4.PlanID.String(), "", plan4.Query, 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
 	hugeInsert := "insert into test_table values 0"
 	for i := 1; i < 1000; i++ {
 		hugeInsert = hugeInsert + fmt.Sprintf(", %d", i)

From 8b5718086908b8f83d729a6a8384da252179c482 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Mon, 13 Aug 2018 20:31:45 -0700
Subject: [PATCH 06/12] Fix data race by acquiring read lock on the QueryStats
 datastructure.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index 272b8d09482..43542c49170 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -492,8 +492,8 @@ func (qe *QueryEngine) AddStats(planName, tableName, query string, queryCount in
 
 // Stats returns the stored QueryStats for the planName.tableName.query
 func (qe *QueryEngine) Stats(planName, tableName, query string) *QueryStats {
-	qe.queryStatsMu.Lock()
-	defer qe.queryStatsMu.Unlock()
+	qe.queryStatsMu.RLock()
+	defer qe.queryStatsMu.RUnlock()
 	s, ok := qe.queryStats[planName+"."+tableName+"."+query]
 	if !ok {
 		return nil
@@ -503,6 +503,8 @@ func (qe *QueryEngine) Stats(planName, tableName, query string) *QueryStats {
 
 func (qe *QueryEngine) getQueryCount() map[string]int64 {
 	qstats := make(map[string]int64)
+	qe.queryStatsMu.RLock()
+	defer qe.queryStatsMu.RUnlock()
 	for k, qs := range qe.queryStats {
 		qs.mu.Lock()
 		qstats[k] = qs.queryCount
@@ -513,6 +515,8 @@ func (qe *QueryEngine) getQueryCount() map[string]int64 {
 
 func (qe *QueryEngine) getQueryTime() map[string]int64 {
 	qstats := make(map[string]int64)
+	qe.queryStatsMu.RLock()
+	defer qe.queryStatsMu.RUnlock()
 	for k, qs := range qe.queryStats {
 		qs.mu.Lock()
 		qstats[k] = int64(qs.time)
@@ -523,6 +527,8 @@ func (qe *QueryEngine) getQueryTime() map[string]int64 {
 
 func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 	qstats := make(map[string]int64)
+	qe.queryStatsMu.RLock()
+	defer qe.queryStatsMu.RUnlock()
 	for k, qs := range qe.queryStats {
 		qs.mu.Lock()
 		qstats[k] = qs.rowCount
@@ -533,6 +539,8 @@ func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 
 func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
 	qstats := make(map[string]int64)
+	qe.queryStatsMu.RLock()
+	defer qe.queryStatsMu.RUnlock()
 	for k, qs := range qe.queryStats {
 		qs.mu.Lock()
 		qstats[k] = qs.errorCount

From 5f3aaae91c83d657de68760c5b97e6a0e5f46d4d Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Tue, 14 Aug 2018 16:05:48 -0700
Subject: [PATCH 07/12] Get the query string correctly.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/queryz_test.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/queryz_test.go b/go/vt/vttablet/tabletserver/queryz_test.go
index 891bfc934f0..f86bad22cbf 100644
--- a/go/vt/vttablet/tabletserver/queryz_test.go
+++ b/go/vt/vttablet/tabletserver/queryz_test.go
@@ -44,7 +44,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonTable,
 		},
 	}
-	qe.AddStats(plan1.PlanID.String(), "test_table", plan1.Query, 10, 2*time.Second, 1*time.Second, 2, 0)
+	qe.AddStats(plan1.PlanID.String(), "test_table", plan1.FullQuery.Query, 10, 2*time.Second, 1*time.Second, 2, 0)
 	qe.plans.Set("select name from test_table", plan1)
 
 	plan2 := &TabletPlan{
@@ -54,7 +54,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan2.PlanID.String(), "test_table", plan2.Query, 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan2.PlanID.String(), "test_table", plan2.FullQuery.Query, 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
 	qe.plans.Set("insert into test_table values 1", plan2)
 
 	plan3 := &TabletPlan{
@@ -64,7 +64,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan3.PlanID.String(), "", plan3.Query, 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
+	qe.AddStats(plan3.PlanID.String(), "", plan3.FullQuery.Query, 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
 	qe.plans.Set("show tables", plan3)
 	qe.plans.Set("", (*TabletPlan)(nil))
 
@@ -75,7 +75,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan4.PlanID.String(), "", plan4.Query, 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
+	qe.AddStats(plan4.PlanID.String(), "", plan4.FullQuery.Query, 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
 	hugeInsert := "insert into test_table values 0"
 	for i := 1; i < 1000; i++ {
 		hugeInsert = hugeInsert + fmt.Sprintf(", %d", i)

From f825425f42353c450e73d3b16b39486c969a455f Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Tue, 14 Aug 2018 17:24:57 -0700
Subject: [PATCH 08/12] Add back in the query stats information to the
 TabletPlan.

Drive the QueryCounts / QueryErrorCounts / QueryRowCounts prom/expvar
stats out of the QueryEngine tracked statistics keyed by Table/Plan
name.

Continue to use the existing query stats information in the TabletPlan
to drive queryz / query_stats.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go   | 122 +++++++++++++-----
 go/vt/vttablet/tabletserver/query_executor.go |   9 +-
 go/vt/vttablet/tabletserver/queryz.go         |   9 +-
 go/vt/vttablet/tabletserver/queryz_test.go    |   8 +-
 4 files changed, 104 insertions(+), 44 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index 43542c49170..b0a3dc8fedb 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -62,6 +62,13 @@ type TabletPlan struct {
 	Rules            *rules.Rules
 	LegacyAuthorized *tableacl.ACLResult
 	Authorized       []*tableacl.ACLResult
+
+	mu         sync.Mutex
+	QueryCount int64
+	Time       time.Duration
+	MysqlTime  time.Duration
+	RowCount   int64
+	ErrorCount int64
 }
 
 // Size allows TabletPlan to be in cache.LRUCache.
@@ -77,6 +84,29 @@ func (ep *TabletPlan) buildAuthorized() {
 	}
 }
 
+// AddStats updates the stats for the current TabletPlan.
+func (ep *TabletPlan) AddStats(queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	ep.mu.Lock()
+	ep.QueryCount += queryCount
+	ep.Time += duration
+	ep.MysqlTime += mysqlTime
+	ep.RowCount += rowCount
+	ep.ErrorCount += errorCount
+	ep.mu.Unlock()
+}
+
+// Stats returns the current stats of TabletPlan.
+func (ep *TabletPlan) Stats() (queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	ep.mu.Lock()
+	queryCount = ep.QueryCount
+	duration = ep.Time
+	mysqlTime = ep.MysqlTime
+	rowCount = ep.RowCount
+	errorCount = ep.ErrorCount
+	ep.mu.Unlock()
+	return
+}
+
 //_______________________________________________
 
 // QueryEngine implements the core functionality of tabletserver.
@@ -224,10 +254,10 @@ func NewQueryEngine(checker connpool.MySQLChecker, se *schema.Engine, config tab
 		stats.Publish("QueryCacheOldest", stats.StringFunc(func() string {
 			return fmt.Sprintf("%v", qe.plans.Oldest())
 		}))
-		_ = stats.NewCountersFuncWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}, qe.getQueryCount)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}, qe.getQueryTime)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}, qe.getQueryRowCount)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}, qe.getQueryErrorCount)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}, qe.getQueryCountByTablePlan)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}, qe.getQueryTimeByTablePlan)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}, qe.getQueryRowCountByTablePlan)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}, qe.getQueryErrorCountByTablePlan)
 
 		http.Handle("/debug/hotrows", qe.txSerializer)
 
@@ -452,6 +482,57 @@ func (qe *QueryEngine) QueryPlanCacheCap() int {
 	return int(qe.plans.Capacity())
 }
 
+func (qe *QueryEngine) getQueryCount() map[string]int64 {
+	f := func(plan *TabletPlan) int64 {
+		queryCount, _, _, _, _ := plan.Stats()
+		return queryCount
+	}
+	return qe.getQueryStats(f)
+}
+
+func (qe *QueryEngine) getQueryTime() map[string]int64 {
+	f := func(plan *TabletPlan) int64 {
+		_, time, _, _, _ := plan.Stats()
+		return int64(time)
+	}
+	return qe.getQueryStats(f)
+}
+
+func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
+	f := func(plan *TabletPlan) int64 {
+		_, _, _, rowCount, _ := plan.Stats()
+		return rowCount
+	}
+	return qe.getQueryStats(f)
+}
+
+func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
+	f := func(plan *TabletPlan) int64 {
+		_, _, _, _, errorCount := plan.Stats()
+		return errorCount
+	}
+	return qe.getQueryStats(f)
+}
+
+type queryStatsFunc func(*TabletPlan) int64
+
+func (qe *QueryEngine) getQueryStats(f queryStatsFunc) map[string]int64 {
+	keys := qe.plans.Keys()
+	qstats := make(map[string]int64)
+	for _, v := range keys {
+		if plan := qe.peekQuery(v); plan != nil {
+			table := plan.TableName()
+			if table.IsEmpty() {
+				table = sqlparser.NewTableIdent("Join")
+			}
+			planType := plan.PlanID.String()
+			data := f(plan)
+			qstats[table.String()+"."+planType] += data
+		}
+	}
+	return qstats
+}
+
 // QueryStats tracks query stats for export per planName/tableName
 type QueryStats struct {
 	mu         sync.Mutex
@@ -463,8 +544,8 @@ type QueryStats struct {
 }
 
 // AddStats adds the given stats for the planName.tableName
-func (qe *QueryEngine) AddStats(planName, tableName, query string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	key := planName + "." + tableName + "." + query
+func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	key := tableName + "." + planName
 
 	qe.queryStatsMu.RLock()
 	stats, ok := qe.queryStats[key]
@@ -490,18 +571,7 @@ func (qe *QueryEngine) AddStats(planName, tableName, query string, queryCount in
 	stats.mu.Unlock()
 }
 
-// Stats returns the stored QueryStats for the planName.tableName.query
-func (qe *QueryEngine) Stats(planName, tableName, query string) *QueryStats {
-	qe.queryStatsMu.RLock()
-	defer qe.queryStatsMu.RUnlock()
-	s, ok := qe.queryStats[planName+"."+tableName+"."+query]
-	if !ok {
-		return nil
-	}
-	return s
-}
-
-func (qe *QueryEngine) getQueryCount() map[string]int64 {
+func (qe *QueryEngine) getQueryCountByTablePlan() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -513,7 +583,7 @@ func (qe *QueryEngine) getQueryCount() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryTime() map[string]int64 {
+func (qe *QueryEngine) getQueryTimeByTablePlan() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -525,7 +595,7 @@ func (qe *QueryEngine) getQueryTime() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
+func (qe *QueryEngine) getQueryRowCountByTablePlan() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -537,7 +607,7 @@ func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
+func (qe *QueryEngine) getQueryErrorCountByTablePlan() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -608,15 +678,7 @@ func (qe *QueryEngine) handleHTTPQueryStats(response http.ResponseWriter, reques
 			pqstats.Query = unicoded(sqlparser.TruncateForUI(v))
 			pqstats.Table = plan.TableName().String()
 			pqstats.Plan = plan.PlanID
-			if stats := qe.Stats(pqstats.Plan.String(), pqstats.Table, pqstats.Query); stats != nil {
-				stats.mu.Lock()
-				pqstats.QueryCount = stats.queryCount
-				pqstats.Time = stats.time
-				pqstats.MysqlTime = stats.mysqlTime
-				pqstats.RowCount = stats.rowCount
-				pqstats.ErrorCount = stats.errorCount
-				stats.mu.Unlock()
-			}
+			pqstats.QueryCount, pqstats.Time, pqstats.MysqlTime, pqstats.RowCount, pqstats.ErrorCount = plan.Stats()
 
 			qstats = append(qstats, pqstats)
 		}
diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index acba0ae4947..dfa0b2fbcd8 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -69,6 +69,11 @@ var sequenceFields = []*querypb.Field{
 	},
 }
 
+func (qre *QueryExecutor) addStats(planName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
+	qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), queryCount, duration, mysqlTime, rowCount, errorCount)
+	qre.plan.AddStats(queryCount, duration, mysqlTime, rowCount, errorCount)
+}
+
 // Execute performs a non-streaming query execution.
 func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 	qre.logStats.TransactionID = qre.transactionID
@@ -80,10 +85,10 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
 		if reply == nil {
-			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), qre.query, 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
+			qre.addStats(planName, 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
 			return
 		}
-		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), qre.query, 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
+		qre.addStats(planName, 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows
 		tabletenv.ResultStats.Add(int64(len(reply.Rows)))
diff --git a/go/vt/vttablet/tabletserver/queryz.go b/go/vt/vttablet/tabletserver/queryz.go
index 368effe4a7e..21b5c482e25 100644
--- a/go/vt/vttablet/tabletserver/queryz.go
+++ b/go/vt/vttablet/tabletserver/queryz.go
@@ -155,14 +155,7 @@ func queryzHandler(qe *QueryEngine, w http.ResponseWriter, r *http.Request) {
 			Plan:   plan.PlanID,
 			Reason: plan.Reason,
 		}
-		qs := qe.Stats(plan.PlanID.String(), plan.TableName().String(), v)
-		qs.mu.Lock()
-		Value.Count = qs.queryCount
-		Value.tm = qs.time
-		Value.mysqlTime = qs.mysqlTime
-		Value.Rows = qs.rowCount
-		Value.Errors = qs.errorCount
-		qs.mu.Unlock()
+		Value.Count, Value.tm, Value.mysqlTime, Value.Rows, Value.Errors = plan.Stats()
 		var timepq time.Duration
 		if Value.Count != 0 {
 			timepq = time.Duration(int64(Value.tm) / Value.Count)
diff --git a/go/vt/vttablet/tabletserver/queryz_test.go b/go/vt/vttablet/tabletserver/queryz_test.go
index f86bad22cbf..43cc7d384f3 100644
--- a/go/vt/vttablet/tabletserver/queryz_test.go
+++ b/go/vt/vttablet/tabletserver/queryz_test.go
@@ -44,7 +44,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonTable,
 		},
 	}
-	qe.AddStats(plan1.PlanID.String(), "test_table", plan1.FullQuery.Query, 10, 2*time.Second, 1*time.Second, 2, 0)
+	plan1.AddStats(10, 2*time.Second, 1*time.Second, 2, 0)
 	qe.plans.Set("select name from test_table", plan1)
 
 	plan2 := &TabletPlan{
@@ -54,7 +54,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan2.PlanID.String(), "test_table", plan2.FullQuery.Query, 1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
+	plan2.AddStats(1, 2*time.Millisecond, 1*time.Millisecond, 1, 0)
 	qe.plans.Set("insert into test_table values 1", plan2)
 
 	plan3 := &TabletPlan{
@@ -64,7 +64,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan3.PlanID.String(), "", plan3.FullQuery.Query, 1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
+	plan3.AddStats(1, 75*time.Millisecond, 50*time.Millisecond, 1, 0)
 	qe.plans.Set("show tables", plan3)
 	qe.plans.Set("", (*TabletPlan)(nil))
 
@@ -75,7 +75,7 @@ func TestQueryzHandler(t *testing.T) {
 			Reason: planbuilder.ReasonDefault,
 		},
 	}
-	qe.AddStats(plan4.PlanID.String(), "", plan4.FullQuery.Query, 1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
+	plan4.AddStats(1, 1*time.Millisecond, 1*time.Millisecond, 1, 0)
 	hugeInsert := "insert into test_table values 0"
 	for i := 1; i < 1000; i++ {
 		hugeInsert = hugeInsert + fmt.Sprintf(", %d", i)

From 7d7716d653b2be20c7edcef462235823c6860e07 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Tue, 28 Aug 2018 14:05:52 -0700
Subject: [PATCH 09/12] Put back the buildAuthorized  method where it used to
 be Get rid of unneccessary duplication of stats methods

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_engine.go | 83 ++++-----------------
 1 file changed, 16 insertions(+), 67 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go
index b0a3dc8fedb..735f95ab2fe 100644
--- a/go/vt/vttablet/tabletserver/query_engine.go
+++ b/go/vt/vttablet/tabletserver/query_engine.go
@@ -76,14 +76,6 @@ func (*TabletPlan) Size() int {
 	return 1
 }
 
-// buildAuthorized builds 'Authorized', which is the runtime part for 'Permissions'.
-func (ep *TabletPlan) buildAuthorized() {
-	ep.Authorized = make([]*tableacl.ACLResult, len(ep.Permissions))
-	for i, perm := range ep.Permissions {
-		ep.Authorized[i] = tableacl.Authorized(perm.TableName, perm.Role)
-	}
-}
-
 // AddStats updates the stats for the current TabletPlan.
 func (ep *TabletPlan) AddStats(queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
 	ep.mu.Lock()
@@ -107,6 +99,14 @@ func (ep *TabletPlan) Stats() (queryCount int64, duration, mysqlTime time.Durati
 	return
 }
 
+// buildAuthorized builds 'Authorized', which is the runtime part for 'Permissions'.
+func (ep *TabletPlan) buildAuthorized() {
+	ep.Authorized = make([]*tableacl.ACLResult, len(ep.Permissions))
+	for i, perm := range ep.Permissions {
+		ep.Authorized[i] = tableacl.Authorized(perm.TableName, perm.Role)
+	}
+}
+
 //_______________________________________________
 
 // QueryEngine implements the core functionality of tabletserver.
@@ -254,10 +254,10 @@ func NewQueryEngine(checker connpool.MySQLChecker, se *schema.Engine, config tab
 		stats.Publish("QueryCacheOldest", stats.StringFunc(func() string {
 			return fmt.Sprintf("%v", qe.plans.Oldest())
 		}))
-		_ = stats.NewCountersFuncWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}, qe.getQueryCountByTablePlan)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}, qe.getQueryTimeByTablePlan)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}, qe.getQueryRowCountByTablePlan)
-		_ = stats.NewCountersFuncWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}, qe.getQueryErrorCountByTablePlan)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}, qe.getQueryCount)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}, qe.getQueryTime)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}, qe.getQueryRowCount)
+		_ = stats.NewCountersFuncWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}, qe.getQueryErrorCount)
 
 		http.Handle("/debug/hotrows", qe.txSerializer)
 
@@ -482,57 +482,6 @@ func (qe *QueryEngine) QueryPlanCacheCap() int {
 	return int(qe.plans.Capacity())
 }
 
-func (qe *QueryEngine) getQueryCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		queryCount, _, _, _, _ := plan.Stats()
-		return queryCount
-	}
-	return qe.getQueryStats(f)
-}
-
-func (qe *QueryEngine) getQueryTime() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, time, _, _, _ := plan.Stats()
-		return int64(time)
-	}
-	return qe.getQueryStats(f)
-}
-
-func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, _, _, rowCount, _ := plan.Stats()
-		return rowCount
-	}
-	return qe.getQueryStats(f)
-}
-
-func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
-	f := func(plan *TabletPlan) int64 {
-		_, _, _, _, errorCount := plan.Stats()
-		return errorCount
-	}
-	return qe.getQueryStats(f)
-}
-
-type queryStatsFunc func(*TabletPlan) int64
-
-func (qe *QueryEngine) getQueryStats(f queryStatsFunc) map[string]int64 {
-	keys := qe.plans.Keys()
-	qstats := make(map[string]int64)
-	for _, v := range keys {
-		if plan := qe.peekQuery(v); plan != nil {
-			table := plan.TableName()
-			if table.IsEmpty() {
-				table = sqlparser.NewTableIdent("Join")
-			}
-			planType := plan.PlanID.String()
-			data := f(plan)
-			qstats[table.String()+"."+planType] += data
-		}
-	}
-	return qstats
-}
-
 // QueryStats tracks query stats for export per planName/tableName
 type QueryStats struct {
 	mu         sync.Mutex
@@ -571,7 +520,7 @@ func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, du
 	stats.mu.Unlock()
 }
 
-func (qe *QueryEngine) getQueryCountByTablePlan() map[string]int64 {
+func (qe *QueryEngine) getQueryCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -583,7 +532,7 @@ func (qe *QueryEngine) getQueryCountByTablePlan() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryTimeByTablePlan() map[string]int64 {
+func (qe *QueryEngine) getQueryTime() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -595,7 +544,7 @@ func (qe *QueryEngine) getQueryTimeByTablePlan() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryRowCountByTablePlan() map[string]int64 {
+func (qe *QueryEngine) getQueryRowCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()
@@ -607,7 +556,7 @@ func (qe *QueryEngine) getQueryRowCountByTablePlan() map[string]int64 {
 	return qstats
 }
 
-func (qe *QueryEngine) getQueryErrorCountByTablePlan() map[string]int64 {
+func (qe *QueryEngine) getQueryErrorCount() map[string]int64 {
 	qstats := make(map[string]int64)
 	qe.queryStatsMu.RLock()
 	defer qe.queryStatsMu.RUnlock()

From fc5f4de353fbc7e0ee5b36efeca5cdb70e60e5a0 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Thu, 30 Aug 2018 14:31:51 -0700
Subject: [PATCH 10/12] Get rid of helper function.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_executor.go | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index dfa0b2fbcd8..c8506fc64d4 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -69,11 +69,6 @@ var sequenceFields = []*querypb.Field{
 	},
 }
 
-func (qre *QueryExecutor) addStats(planName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) {
-	qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), queryCount, duration, mysqlTime, rowCount, errorCount)
-	qre.plan.AddStats(queryCount, duration, mysqlTime, rowCount, errorCount)
-}
-
 // Execute performs a non-streaming query execution.
 func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 	qre.logStats.TransactionID = qre.transactionID
@@ -84,11 +79,14 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.QueryStats.Add(planName, duration)
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
+		mysqlTime := qre.logStats.MysqlResponseTime
 		if reply == nil {
-			qre.addStats(planName, 1, duration, qre.logStats.MysqlResponseTime, 0, 1)
+			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, mysqlTime, 0, 1)
+			qre.plan.AddStats(1, duration, mysqlTime, 0, 1)
 			return
 		}
-		qre.addStats(planName, 1, duration, qre.logStats.MysqlResponseTime, int64(reply.RowsAffected), 0)
+		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, mysqlTime, int64(reply.RowsAffected), 1)
+		qre.plan.AddStats(1, duration, mysqlTime, int64(reply.RowsAffected), 1)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows
 		tabletenv.ResultStats.Add(int64(len(reply.Rows)))

From 8a5f81165d81933784618891e254dc4d3af5ed9c Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Fri, 31 Aug 2018 10:02:15 -0700
Subject: [PATCH 11/12] Fix error introduced in previous commit where I missed
 the right params for errorCounts when unrolling the helper function.

Also, sub "join" for unknown table names, as talked about in code
review.

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_executor.go | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index c8506fc64d4..8a3c8dbd53e 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -80,13 +80,18 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
 		mysqlTime := qre.logStats.MysqlResponseTime
+		table := qre.plan.TableName().String()
+		if table == "" {
+			table = "Join"
+		}
+
 		if reply == nil {
-			qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, mysqlTime, 0, 1)
+			qre.tsv.qe.AddStats(planName, table, 1, duration, mysqlTime, 0, 1)
 			qre.plan.AddStats(1, duration, mysqlTime, 0, 1)
 			return
 		}
-		qre.tsv.qe.AddStats(planName, qre.plan.TableName().String(), 1, duration, mysqlTime, int64(reply.RowsAffected), 1)
-		qre.plan.AddStats(1, duration, mysqlTime, int64(reply.RowsAffected), 1)
+		qre.tsv.qe.AddStats(planName, table, 1, duration, mysqlTime, int64(reply.RowsAffected), 0)
+		qre.plan.AddStats(1, duration, mysqlTime, int64(reply.RowsAffected), 0)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows
 		tabletenv.ResultStats.Add(int64(len(reply.Rows)))

From a000403d2dfe9b58062785e974da5d0d87a80102 Mon Sep 17 00:00:00 2001
From: Maggie Zhou <mzhou@slack-corp.com>
Date: Fri, 31 Aug 2018 10:36:22 -0700
Subject: [PATCH 12/12] s/table/tableName

Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
---
 go/vt/vttablet/tabletserver/query_executor.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go
index 8a3c8dbd53e..3ba89272379 100644
--- a/go/vt/vttablet/tabletserver/query_executor.go
+++ b/go/vt/vttablet/tabletserver/query_executor.go
@@ -80,17 +80,17 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		tabletenv.RecordUserQuery(qre.ctx, qre.plan.TableName(), "Execute", int64(duration))
 
 		mysqlTime := qre.logStats.MysqlResponseTime
-		table := qre.plan.TableName().String()
-		if table == "" {
-			table = "Join"
+		tableName := qre.plan.TableName().String()
+		if tableName == "" {
+			tableName = "Join"
 		}
 
 		if reply == nil {
-			qre.tsv.qe.AddStats(planName, table, 1, duration, mysqlTime, 0, 1)
+			qre.tsv.qe.AddStats(planName, tableName, 1, duration, mysqlTime, 0, 1)
 			qre.plan.AddStats(1, duration, mysqlTime, 0, 1)
 			return
 		}
-		qre.tsv.qe.AddStats(planName, table, 1, duration, mysqlTime, int64(reply.RowsAffected), 0)
+		qre.tsv.qe.AddStats(planName, tableName, 1, duration, mysqlTime, int64(reply.RowsAffected), 0)
 		qre.plan.AddStats(1, duration, mysqlTime, int64(reply.RowsAffected), 0)
 		qre.logStats.RowsAffected = int(reply.RowsAffected)
 		qre.logStats.Rows = reply.Rows