-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: support automatically create sql baselines #12434
Changes from 5 commits
9139d41
7700dab
dbe8335
89ad543
a09ec75
6f5ec11
e8d9aa9
8cf72ec
84f1a88
f585216
62e7370
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -827,25 +827,34 @@ func (do *Domain) LoadBindInfoLoop(ctx sessionctx.Context) error { | |
return err | ||
} | ||
|
||
do.loadBindInfoLoop() | ||
do.globalBindHandleWorkerLoop() | ||
do.handleInvalidBindTaskLoop() | ||
return nil | ||
} | ||
|
||
func (do *Domain) loadBindInfoLoop() { | ||
func (do *Domain) globalBindHandleWorkerLoop() { | ||
do.wg.Add(1) | ||
go func() { | ||
defer do.wg.Done() | ||
defer recoverInDomain("loadBindInfoLoop", false) | ||
defer recoverInDomain("globalBindHandleWorkerLoop", false) | ||
loadTicker := time.NewTicker(bindinfo.Lease) | ||
defer loadTicker.Stop() | ||
captureBaselineTicker := time.NewTicker(bindinfo.Lease) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just use a single ticker for 2 actions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, we can. |
||
defer captureBaselineTicker.Stop() | ||
lzmhhh123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for { | ||
select { | ||
case <-do.exit: | ||
return | ||
case <-time.After(bindinfo.Lease): | ||
} | ||
err := do.bindHandle.Update(false) | ||
if err != nil { | ||
logutil.BgLogger().Error("update bindinfo failed", zap.Error(err)) | ||
case <-loadTicker.C: | ||
err := do.bindHandle.Update(false) | ||
if err != nil { | ||
logutil.BgLogger().Error("update bindinfo failed", zap.Error(err)) | ||
} | ||
case <-captureBaselineTicker.C: | ||
if !variable.TiDBOptOn(variable.CapturePlanBaseline.GetVal()) { | ||
continue | ||
} | ||
do.bindHandle.CaptureBaselines(do.InfoSchema()) | ||
} | ||
} | ||
}() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ func (gvc *GlobalVariableCache) Update(rows []chunk.Row, fields []*ast.ResultFie | |
gvc.fields = fields | ||
gvc.Unlock() | ||
|
||
checkEnableStmtSummary(rows, fields) | ||
checkEnableServerGlobalVar(rows) | ||
} | ||
|
||
// Get gets the global variables from cache. | ||
|
@@ -67,24 +67,22 @@ func (gvc *GlobalVariableCache) Disable() { | |
return | ||
} | ||
|
||
// checkEnableStmtSummary looks for TiDBEnableStmtSummary and notifies StmtSummary | ||
func checkEnableStmtSummary(rows []chunk.Row, fields []*ast.ResultField) { | ||
// checkEnableServerGlobalVar processes variables that acts in server and global level. | ||
func checkEnableServerGlobalVar(rows []chunk.Row) { | ||
for _, row := range rows { | ||
varName := row.GetString(0) | ||
if varName == variable.TiDBEnableStmtSummary { | ||
varVal := row.GetDatum(1, &fields[1].Column.FieldType) | ||
|
||
switch row.GetString(0) { | ||
case variable.TiDBEnableStmtSummary: | ||
sVal := "" | ||
if !varVal.IsNull() { | ||
var err error | ||
sVal, err = varVal.ToString() | ||
if err != nil { | ||
return | ||
} | ||
if !row.IsNull(1) { | ||
sVal = row.GetString(1) | ||
} | ||
|
||
stmtsummary.StmtSummaryByDigestMap.SetEnabled(sVal, false) | ||
break | ||
case variable.TiDBCapturePlanBaseline: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we add dependency check between these 2 variables? if not, would there be possible panics when accessing structures of statements? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it won't panic because the |
||
sVal := "" | ||
if !row.IsNull(1) { | ||
sVal = row.GetString(1) | ||
} | ||
variable.CapturePlanBaseline.Set(sVal, false) | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import ( | |
"math" | ||
"strconv" | ||
"strings" | ||
"sync" | ||
"sync/atomic" | ||
"time" | ||
|
||
|
@@ -611,7 +612,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, | |
return "off", nil | ||
} | ||
return value, ErrWrongValueForVar.GenWithStackByArgs(name, value) | ||
case TiDBEnableStmtSummary: | ||
case TiDBEnableStmtSummary, TiDBCapturePlanBaseline: | ||
switch { | ||
case strings.EqualFold(value, "ON") || value == "1": | ||
return "1", nil | ||
|
@@ -737,3 +738,31 @@ func setAnalyzeTime(s *SessionVars, val string) (string, error) { | |
} | ||
return t.Format(AnalyzeFullTimeFormat), nil | ||
} | ||
|
||
// serverGlobalVariable is used to handle variables that acts in server and global scope. | ||
type serverGlobalVariable struct { | ||
sync.Mutex | ||
serverVal string | ||
globalVal string | ||
} | ||
|
||
// Set sets the value according to variable scope. | ||
func (v *serverGlobalVariable) Set(val string, isServer bool) { | ||
v.Lock() | ||
if isServer { | ||
v.serverVal = val | ||
} else { | ||
v.globalVal = val | ||
} | ||
v.Unlock() | ||
} | ||
|
||
// GetVal gets the value. | ||
func (v *serverGlobalVariable) GetVal() string { | ||
v.Lock() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just set read lock here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is not necessary because we do not expect the server global variables be set or read very frequent, a |
||
defer v.Unlock() | ||
if v.serverVal != "" { | ||
return v.serverVal | ||
} | ||
return v.globalVal | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,6 +230,26 @@ func (ssMap *stmtSummaryByDigestMap) ToDatum() [][]types.Datum { | |
return rows | ||
} | ||
|
||
// GetMoreThanOnceSelect gets select SQLs that occurred more than once. | ||
func (ssMap *stmtSummaryByDigestMap) GetMoreThanOnceSelect() ([]string, []string) { | ||
ssMap.Lock() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a |
||
values := ssMap.summaryMap.Values() | ||
ssMap.Unlock() | ||
|
||
schemas := make([]string, 0, len(values)) | ||
sqls := make([]string, 0, len(values)) | ||
for _, value := range values { | ||
summary := value.(*stmtSummaryByDigest) | ||
summary.Lock() | ||
if strings.HasPrefix(summary.normalizedSQL, "select") && summary.execCount > 1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, can we make the SQL occurrent number under control instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we can, but we can do it when only when it is necessary because doing it would require another session variables, for now, it is enough. |
||
schemas = append(schemas, summary.schemaName) | ||
sqls = append(sqls, summary.sampleSQL) | ||
} | ||
summary.Unlock() | ||
} | ||
return schemas, sqls | ||
} | ||
|
||
// SetEnabled enables or disables statement summary in global(cluster) or session(server) scope. | ||
func (ssMap *stmtSummaryByDigestMap) SetEnabled(value string, inSession bool) { | ||
value = ssMap.normalizeEnableValue(value) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this function call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will create a new txn and the txn maybe used by the optimizer when generating hints, since it may execute some sub queries, and without it, the test will panic.