From d7aab949322ddf18c6ee7e31fbd14380532d9b11 Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 09:46:10 +0800 Subject: [PATCH 1/7] use err to instead err==nil Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 19 +++++++++++-------- metrics/server.go | 2 +- server/conn.go | 2 +- session/session.go | 2 +- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index 12cb76247e8cf..1d27c5cb45c8a 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -802,8 +802,7 @@ func FormatSQL(sql string, pps variable.PreparedParams) stringutil.StringerFunc var ( sessionExecuteRunDurationInternal = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblInternal) sessionExecuteRunDurationGeneral = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblGeneral) - totalTiFlashQueryFailCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblError) - totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblOK) + totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues("ok", metrics.LblOK) ) // FinishExecuteStmt is used to record some information after `ExecStmt` execution finished: @@ -811,7 +810,8 @@ var ( // 2. record summary statement. // 3. record execute duration metric. // 4. update the `PrevStmt` in session variable. -func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults bool) { +func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults bool) { + fmt.Println(err) sessVars := a.Ctx.GetSessionVars() execDetail := sessVars.StmtCtx.GetExecDetails() // Attach commit/lockKeys runtime stats to executor runtime stats. @@ -831,13 +831,16 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo a.Ctx.GetTxnWriteThroughputSLI().AddReadKeys(execDetail.ScanDetail.ProcessedKeys) } // `LowSlowQuery` and `SummaryStmt` must be called before recording `PrevStmt`. - a.LogSlowQuery(txnTS, succ, hasMoreResults) - a.SummaryStmt(succ) + a.LogSlowQuery(txnTS, err == nil, hasMoreResults) + a.SummaryStmt(err == nil) if sessVars.StmtCtx.IsTiFlash.Load() { - if succ { + if err == nil { + fmt.Println("succ") totalTiFlashQuerySuccCounter.Inc() } else { - totalTiFlashQueryFailCounter.Inc() + fmt.Println("fail") + fmt.Println(metrics.ExecuteErrorToLabel(err)) + metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc() } } prevStmt := a.GetTextToLog() @@ -858,7 +861,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo // CloseRecordSet will finish the execution of current statement and do some record work func (a *ExecStmt) CloseRecordSet(txnStartTS uint64, lastErr error) { - a.FinishExecuteStmt(txnStartTS, lastErr == nil, false) + a.FinishExecuteStmt(txnStartTS, lastErr, false) a.logAudit() // Detach the Memory and disk tracker for the previous stmtCtx from GlobalMemoryUsageTracker and GlobalDiskUsageTracker if stmtCtx := a.Ctx.GetSessionVars().StmtCtx; stmtCtx != nil { diff --git a/metrics/server.go b/metrics/server.go index 3f01eec30bea8..f83c7afee2732 100644 --- a/metrics/server.go +++ b/metrics/server.go @@ -227,7 +227,7 @@ var ( Subsystem: "server", Name: "tiflash_query_total", Help: "Counter of TiFlash queries.", - }, []string{LblResult}) + }, []string{LblType, LblResult}) ) // ExecuteErrorToLabel converts an execute error to label. diff --git a/server/conn.go b/server/conn.go index 5592ffb945e87..eea54b53326d9 100644 --- a/server/conn.go +++ b/server/conn.go @@ -1727,7 +1727,7 @@ func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, warns [ if handled { execStmt := cc.ctx.Value(session.ExecStmtVarKey) if execStmt != nil { - execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err == nil, false) + execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err, false) } } if err != nil { diff --git a/session/session.go b/session/session.go index f529647248af7..fcd9349fea95b 100644 --- a/session/session.go +++ b/session/session.go @@ -1550,7 +1550,7 @@ func runStmt(ctx context.Context, se *session, s sqlexec.Statement) (rs sqlexec. } else { // If it is not a select statement or special query, we record its slow log here, // then it could include the transaction commit time. - s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err == nil, false) + s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err, false) } return nil, err } From 93dc1d1400f0b543b205a683a977f17f3d938f9e Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 10:15:41 +0800 Subject: [PATCH 2/7] add error label for TiFlashQueryTotalCounter Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index b324fdb331a69..e6df010341a3f 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -19,6 +19,7 @@ import ( "fmt" "math" "runtime/trace" + "strconv" "strings" "sync/atomic" "time" @@ -856,8 +857,6 @@ func FormatSQL(sql string) stringutil.StringerFunc { var ( sessionExecuteRunDurationInternal = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblInternal) sessionExecuteRunDurationGeneral = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblGeneral) - totalTiFlashQueryFailCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblError) - totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblOK) ) // FinishExecuteStmt is used to record some information after `ExecStmt` execution finished: @@ -866,7 +865,7 @@ var ( // 3. record execute duration metric. // 4. update the `PrevStmt` in session variable. // 5. reset `DurationParse` in session variable. -func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults bool) { +func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults bool) { sessVars := a.Ctx.GetSessionVars() execDetail := sessVars.StmtCtx.GetExecDetails() // Attach commit/lockKeys runtime stats to executor runtime stats. @@ -886,13 +885,13 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo a.Ctx.GetTxnWriteThroughputSLI().AddReadKeys(execDetail.ScanDetail.ProcessedKeys) } // `LowSlowQuery` and `SummaryStmt` must be called before recording `PrevStmt`. - a.LogSlowQuery(txnTS, succ, hasMoreResults) - a.SummaryStmt(succ) + a.LogSlowQuery(txnTS, err == nil, hasMoreResults) + a.SummaryStmt(err == nil) if sessVars.StmtCtx.IsTiFlash.Load() { - if succ { - totalTiFlashQuerySuccCounter.Inc() + if err == nil { + metrics.TiFlashQueryTotalCounter.WithLabelValues(sessVars.StmtCtx.StmtType, metrics.LblOK).Inc() } else { - totalTiFlashQueryFailCounter.Inc() + metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc() } } sessVars.PrevStmt = FormatSQL(a.GetTextToLog()) @@ -909,7 +908,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo // CloseRecordSet will finish the execution of current statement and do some record work func (a *ExecStmt) CloseRecordSet(txnStartTS uint64, lastErr error) { - a.FinishExecuteStmt(txnStartTS, lastErr == nil, false) + a.FinishExecuteStmt(txnStartTS, lastErr, false) a.logAudit() // Detach the Memory and disk tracker for the previous stmtCtx from GlobalMemoryUsageTracker and GlobalDiskUsageTracker if stmtCtx := a.Ctx.GetSessionVars().StmtCtx; stmtCtx != nil { From f05d8551b7b7886fc5a9ea0354bed179a6aa77bd Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 10:19:52 +0800 Subject: [PATCH 3/7] remove useless import Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index e6df010341a3f..756a6c4a6240c 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -19,8 +19,7 @@ import ( "fmt" "math" "runtime/trace" - "strconv" - "strings" + g "strings" "sync/atomic" "time" From 51872720a2afe157933285a3adc94069f8708c31 Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 10:23:15 +0800 Subject: [PATCH 4/7] fix typo Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/adapter.go b/executor/adapter.go index 756a6c4a6240c..feed19adf6c82 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -19,7 +19,7 @@ import ( "fmt" "math" "runtime/trace" - g "strings" + "strings" "sync/atomic" "time" From 1baef80d81c3af9ac1ef28dd6aaf75f9178c7bda Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 11:53:29 +0800 Subject: [PATCH 5/7] use empty type Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/adapter.go b/executor/adapter.go index feed19adf6c82..fa1b12ad9425d 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -888,7 +888,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults boo a.SummaryStmt(err == nil) if sessVars.StmtCtx.IsTiFlash.Load() { if err == nil { - metrics.TiFlashQueryTotalCounter.WithLabelValues(sessVars.StmtCtx.StmtType, metrics.LblOK).Inc() + metrics.TiFlashQueryTotalCounter.WithLabelValues("", metrics.LblOK).Inc() } else { metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc() } From d50ad6d4b363610132d768a39450170626453237 Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 12:37:44 +0800 Subject: [PATCH 6/7] var Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/executor/adapter.go b/executor/adapter.go index fa1b12ad9425d..f57eb99e1a69c 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -856,6 +856,7 @@ func FormatSQL(sql string) stringutil.StringerFunc { var ( sessionExecuteRunDurationInternal = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblInternal) sessionExecuteRunDurationGeneral = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblGeneral) + totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues("", metrics.LblOK) ) // FinishExecuteStmt is used to record some information after `ExecStmt` execution finished: @@ -888,7 +889,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults boo a.SummaryStmt(err == nil) if sessVars.StmtCtx.IsTiFlash.Load() { if err == nil { - metrics.TiFlashQueryTotalCounter.WithLabelValues("", metrics.LblOK).Inc() + totalTiFlashQuerySuccCounter.Inc() } else { metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc() } From 7405df6162fdaa653d511c10f1d996acc6a0cd5e Mon Sep 17 00:00:00 2001 From: jyz0309 <45495947@qq.com> Date: Thu, 10 Jun 2021 12:51:48 +0800 Subject: [PATCH 7/7] fix comment Signed-off-by: jyz0309 <45495947@qq.com> --- executor/adapter.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index f57eb99e1a69c..1723d0f069f50 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -884,11 +884,12 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults boo // Only record the read keys in write statement which affect row more than 0. a.Ctx.GetTxnWriteThroughputSLI().AddReadKeys(execDetail.ScanDetail.ProcessedKeys) } + succ := err == nil // `LowSlowQuery` and `SummaryStmt` must be called before recording `PrevStmt`. - a.LogSlowQuery(txnTS, err == nil, hasMoreResults) - a.SummaryStmt(err == nil) + a.LogSlowQuery(txnTS, succ, hasMoreResults) + a.SummaryStmt(succ) if sessVars.StmtCtx.IsTiFlash.Load() { - if err == nil { + if succ { totalTiFlashQuerySuccCounter.Inc() } else { metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc()