From b76cbc57d97f8874e99e092f733be6082bd1e0fc Mon Sep 17 00:00:00 2001
From: Haibin Xie <xiehaibin@pingcap.com>
Date: Fri, 7 Sep 2018 15:40:19 +0800
Subject: [PATCH] stats: fix panic when updating by feedback

---
 domain/domain.go          | 12 ++++++++----
 statistics/feedback.go    |  4 ++--
 statistics/update.go      |  8 ++++----
 statistics/update_test.go | 16 +++++++++++++++-
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/domain/domain.go b/domain/domain.go
index 1f6a5a341eac6..156e1e84e656f 100644
--- a/domain/domain.go
+++ b/domain/domain.go
@@ -668,7 +668,10 @@ func (do *Domain) updateStatsWorker(ctx sessionctx.Context, owner owner.Manager)
 	} else {
 		log.Info("[stats] init stats info takes ", time.Now().Sub(t))
 	}
-	defer recoverInDomain("updateStatsWorker", false)
+	defer func() {
+		recoverInDomain("updateStatsWorker", false)
+		do.wg.Done()
+	}()
 	for {
 		select {
 		case <-loadTicker.C:
@@ -678,7 +681,6 @@ func (do *Domain) updateStatsWorker(ctx sessionctx.Context, owner owner.Manager)
 			}
 		case <-do.exit:
 			statsHandle.FlushStats()
-			do.wg.Done()
 			return
 			// This channel is sent only by ddl owner.
 		case t := <-statsHandle.DDLEventCh():
@@ -727,7 +729,10 @@ func (do *Domain) autoAnalyzeWorker(owner owner.Manager) {
 	statsHandle := do.StatsHandle()
 	analyzeTicker := time.NewTicker(do.statsLease)
 	defer analyzeTicker.Stop()
-	defer recoverInDomain("autoAnalyzeWorker", false)
+	defer func() {
+		recoverInDomain("autoAnalyzeWorker", false)
+		do.wg.Done()
+	}()
 	for {
 		select {
 		case <-analyzeTicker.C:
@@ -738,7 +743,6 @@ func (do *Domain) autoAnalyzeWorker(owner owner.Manager) {
 				}
 			}
 		case <-do.exit:
-			do.wg.Done()
 			return
 		}
 	}
diff --git a/statistics/feedback.go b/statistics/feedback.go
index 1ad32cd77304c..fce82dbb151d9 100644
--- a/statistics/feedback.go
+++ b/statistics/feedback.go
@@ -880,13 +880,13 @@ func (q *QueryFeedback) logDetailedInfo(h *Handle) {
 	logPrefix := fmt.Sprintf("[stats-feedback] %s,", t.name)
 	if isIndex {
 		idx := t.Indices[q.hist.ID]
-		if idx == nil {
+		if idx == nil || idx.Histogram.Len() == 0 {
 			return
 		}
 		logForIndex(logPrefix, t, idx, ranges, actual, idx.getIncreaseFactor(t.Count))
 	} else {
 		c := t.Columns[q.hist.ID]
-		if c == nil {
+		if c == nil || c.Histogram.Len() == 0 {
 			return
 		}
 		logForPK(logPrefix, c, ranges, actual, c.getIncreaseFactor(t.Count))
diff --git a/statistics/update.go b/statistics/update.go
index e84165d7e1534..dd2eeef0514a1 100644
--- a/statistics/update.go
+++ b/statistics/update.go
@@ -417,7 +417,7 @@ func (h *Handle) UpdateStatsByLocalFeedback(is infoschema.InfoSchema) {
 		newTblStats := tblStats.copy()
 		if fb.hist.isIndexHist() {
 			idx, ok := tblStats.Indices[fb.hist.ID]
-			if !ok {
+			if !ok || idx.Histogram.Len() == 0 {
 				continue
 			}
 			newIdx := *idx
@@ -428,7 +428,7 @@ func (h *Handle) UpdateStatsByLocalFeedback(is infoschema.InfoSchema) {
 			newTblStats.Indices[fb.hist.ID] = &newIdx
 		} else {
 			col, ok := tblStats.Columns[fb.hist.ID]
-			if !ok {
+			if !ok || col.Histogram.Len() == 0 {
 				continue
 			}
 			newCol := *col
@@ -528,14 +528,14 @@ func (h *Handle) handleSingleHistogramUpdate(is infoschema.InfoSchema, rows []ch
 	var hist *Histogram
 	if isIndex == 1 {
 		idx, ok := tbl.Indices[histID]
-		if ok {
+		if ok && idx.Histogram.Len() > 0 {
 			idxHist := idx.Histogram
 			hist = &idxHist
 			cms = idx.CMSketch.copy()
 		}
 	} else {
 		col, ok := tbl.Columns[histID]
-		if ok {
+		if ok && col.Histogram.Len() > 0 {
 			colHist := col.Histogram
 			hist = &colHist
 		}
diff --git a/statistics/update_test.go b/statistics/update_test.go
index 6e9bce3747dc3..8b0f7cf82d763 100644
--- a/statistics/update_test.go
+++ b/statistics/update_test.go
@@ -661,13 +661,23 @@ func (s *testStatsUpdateSuite) TestQueryFeedback(c *C) {
 		c.Assert(len(feedback), Equals, 0)
 	}
 
-	// Test that the outdated feedback won't cause panic.
+	// Test that after drop stats, the feedback won't cause panic.
 	statistics.FeedbackProbability = 1
 	for _, t := range tests {
 		testKit.MustQuery(t.sql)
 	}
 	c.Assert(h.DumpStatsDeltaToKV(statistics.DumpAll), IsNil)
 	c.Assert(h.DumpStatsFeedbackToKV(), IsNil)
+	testKit.MustExec("drop stats t")
+	c.Assert(h.HandleUpdateStats(s.do.InfoSchema()), IsNil)
+
+	// Test that the outdated feedback won't cause panic.
+	testKit.MustExec("analyze table t")
+	for _, t := range tests {
+		testKit.MustQuery(t.sql)
+	}
+	c.Assert(h.DumpStatsDeltaToKV(statistics.DumpAll), IsNil)
+	c.Assert(h.DumpStatsFeedbackToKV(), IsNil)
 	testKit.MustExec("drop table t")
 	c.Assert(h.HandleUpdateStats(s.do.InfoSchema()), IsNil)
 }
@@ -845,6 +855,10 @@ func (s *testStatsUpdateSuite) TestUpdateStatsByLocalFeedback(c *C) {
 
 	// Test that it won't cause panic after update.
 	testKit.MustQuery("select * from t use index(idx) where b > 0")
+
+	// Test that after drop stats, it won't cause panic.
+	testKit.MustExec("drop stats t")
+	h.UpdateStatsByLocalFeedback(s.do.InfoSchema())
 }
 
 type logHook struct {