stats: do not wait for data unchanged when auto analyze #7022
Conversation
|
What will happen if the following scene occur: Will it trigger multiple analyze in a row? |
statistics/update.go
Outdated
| @@ -533,28 +533,37 @@ const ( | |||
| // AutoAnalyzeMinCnt means if the count of table is less than this value, we needn't do auto analyze. | |||
| var AutoAnalyzeMinCnt int64 = 1000 | |||
|
|
|||
| func tableAnalyzed(tbl *Table) bool { | |||
|
@winoros Yes. |
statistics/update.go
Outdated
| } | ||
| } | ||
| for _, idx := range tbl.Indices { | ||
| if idx.Len() > 0 { |
There was a problem hiding this comment.
What is the idx.Len()? The length of histogram?
| analyzed := tableAnalyzed(tbl) | ||
| if !analyzed { | ||
| t := time.Unix(0, oracle.ExtractPhysical(tbl.Version)*int64(time.Millisecond)) | ||
| return time.Since(t) >= limit |
There was a problem hiding this comment.
The AutoAnalyzeMinCnt is not considered?
There was a problem hiding this comment.
It's considered in line 597.
|
/run-all-test |
|
/run-all-tests |
statistics/update.go
Outdated
| @@ -587,7 +594,7 @@ func (h *Handle) HandleAutoAnalyze(is infoschema.InfoSchema) error { | |||
| for _, tbl := range tbls { | |||
| tblInfo := tbl.Meta() | |||
| statsTbl := h.GetTableStats(tblInfo) | |||
| if statsTbl.Pseudo || statsTbl.Count == 0 { | |||
| if statsTbl.Pseudo || statsTbl.Count < AutoAnalyzeMinCnt || statsTbl.ModifyCount == 0 { | |||
There was a problem hiding this comment.
We can remove the statsTbl.Pseudo since `statsTble.ModifyCount == 0' covers the case.
There was a problem hiding this comment.
When adding an index, ModifyCount may be 0, but we need to analyze the newly added index.
statistics/update.go
Outdated
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // needAnalyzeTable checks if we need to analyze the table. If the table is not analyzed, |
There was a problem hiding this comment.
If the table has never been analyzed,
statistics/update.go
Outdated
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // needAnalyzeTable checks if we need to analyze the table. If the table is not analyzed, | ||
| // we need to analyze it when it has not been modified for a time. If the table is analyzed, |
There was a problem hiding this comment.
If the table had been analyzed before
|
LGTM |
statistics/update.go
Outdated
|
|
||
| // needAnalyzeTable checks if we need to analyze the table. If the table has never been analyzed, | ||
| // we need to analyze it when it has not been modified for a time. If the table had been analyzed before, | ||
| // we need to analyze it when the `modify count / count` is greater than autoAnalyzeRatio. |
There was a problem hiding this comment.
how about this:
// needAnalyzeTable checks if we need to analyze the table.
// 1. If the table has never been analyzed, we need to analyze it when it has
// not been modified for a time.
// 2. If the table had been analyzed before, we need to analyze it when
// "tbl.ModifyCount/tbl.Count > autoAnalyzeRatio".
|
/run-all-tests |
What have you changed? (mandatory)
Before this PR, if the table is continous updated, we cannot auto analyze the table even if
modify count / countis greater than auto-analyze ratio, because we have the limit that the table must not be modified within a time peroid. This PR removes the restriction.What are the type of the changes (mandatory)?
How has this PR been tested (mandatory)?
Unit test
PTAL @coocood @winoros @zz-jason @fipped