Skip to content

Commit

Permalink
stats: fix panic when updating by feedback (#7640)
Browse files Browse the repository at this point in the history
  • Loading branch information
alivxxx authored and ngaut committed Sep 9, 2018
1 parent b3d4ed7 commit a34d699
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
12 changes: 8 additions & 4 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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():
Expand Down Expand Up @@ -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:
Expand All @@ -738,7 +743,6 @@ func (do *Domain) autoAnalyzeWorker(owner owner.Manager) {
}
}
case <-do.exit:
do.wg.Done()
return
}
}
Expand Down
4 changes: 2 additions & 2 deletions statistics/feedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions statistics/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 15 additions & 1 deletion statistics/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,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)
}
Expand Down Expand Up @@ -844,6 +854,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 {
Expand Down

0 comments on commit a34d699

Please sign in to comment.