statistics: fix panic when getStatsReader fail (pingcap#15651)

sre-bot · Mar 26, 2020 · 3443783 · 3443783
1 parent dc1152c
commit 3443783
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 6 deletions.
diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/pingcap/errors"
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/parser/ast"
 	"github.com/pingcap/parser/model"
 	"github.com/pingcap/parser/mysql"
@@ -283,15 +284,16 @@ func (sc statsCache) update(tables []*statistics.Table, deletedIDs []int64, newV
 func (h *Handle) LoadNeededHistograms() (err error) {
 	cols := statistics.HistogramNeededColumns.AllCols()
 	reader, err := h.getStatsReader(nil)
+	if err != nil {
+		return err
+	}
+
 	defer func() {
 		err1 := h.releaseStatsReader(reader)
 		if err1 != nil && err == nil {
 			err = err1
 		}
 	}()
-	if err != nil {
-		return err
-	}
 
 	for _, col := range cols {
 		statsCache := h.statsCache.Load().(statsCache)
@@ -498,15 +500,15 @@ func (h *Handle) columnStatsFromStorage(reader *statsReader, row chunk.Row, tabl
 // tableStatsFromStorage loads table stats info from storage.
 func (h *Handle) tableStatsFromStorage(tableInfo *model.TableInfo, physicalID int64, loadAll bool, historyStatsExec sqlexec.RestrictedSQLExecutor) (_ *statistics.Table, err error) {
 	reader, err := h.getStatsReader(historyStatsExec)
+	if err != nil {
+		return nil, err
+	}
 	defer func() {
 		err1 := h.releaseStatsReader(reader)
 		if err == nil && err1 != nil {
 			err = err1
 		}
 	}()
-	if err != nil {
-		return nil, err
-	}
 	table, ok := h.statsCache.Load().(statsCache).tables[physicalID]
 	// If table stats is pseudo, we also need to copy it, since we will use the column stats when
 	// the average error rate of it is small.
@@ -771,6 +773,11 @@ func (sr *statsReader) isHistory() bool {
 }
 
 func (h *Handle) getStatsReader(history sqlexec.RestrictedSQLExecutor) (*statsReader, error) {
+	failpoint.Inject("mockGetStatsReaderFail", func(val failpoint.Value) {
+		if val.(bool) {
+			failpoint.Return(nil, errors.New("gofail genStatsReader error"))
+		}
+	})
 	if history != nil {
 		return &statsReader{history: history}, nil
 	}

diff --git a/statistics/handle/handle_test.go b/statistics/handle/handle_test.go
@@ -20,6 +20,7 @@ import (
 
 	. "github.com/pingcap/check"
 	"github.com/pingcap/errors"
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/parser/model"
 	"github.com/pingcap/tidb/domain"
 	"github.com/pingcap/tidb/kv"
@@ -433,6 +434,11 @@ func (s *testStatsSuite) TestLoadStats(c *C) {
 	stat = h.GetTableStats(tableInfo)
 	hg = stat.Columns[tableInfo.Columns[2].ID].Histogram
 	c.Assert(hg.Len(), Greater, 0)
+	// Following test tests whether the LoadNeededHistograms would panic.
+	c.Assert(failpoint.Enable("github.com/pingcap/tidb/statistics/handle/mockGetStatsReaderFail", `return(true)`), IsNil)
+	err = h.LoadNeededHistograms()
+	c.Assert(err, NotNil)
+	c.Assert(failpoint.Disable("github.com/pingcap/tidb/statistics/handle/mockGetStatsReaderFail"), IsNil)
 }
 
 func newStoreWithBootstrap() (kv.Storage, *domain.Domain, error) {