-
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
executor: replace logger with zap logger #9521
Conversation
/rebuild |
Codecov Report
@@ Coverage Diff @@
## master #9521 +/- ##
================================================
- Coverage 67.3389% 67.3334% -0.0055%
================================================
Files 378 378
Lines 79535 79540 +5
================================================
- Hits 53558 53557 -1
- Misses 21194 21201 +7
+ Partials 4783 4782 -1 |
executor/admin.go
Outdated
@@ -393,8 +395,9 @@ func (e *RecoverIndexExec) batchMarkDup(txn kv.Transaction, rows []recoverRows) | |||
} | |||
|
|||
if handle != rows[i].handle { | |||
log.Warnf("[recover-index] The constraint of unique index:%v is broken, handle:%v is not equal handle:%v with idxKey:%v.", | |||
e.index.Meta().Name.O, handle, rows[i].handle, key) | |||
log.Warn("The constraint of unique index is broken when recovering index", |
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.
Should we print [recover-index]
prefix? @zimulala What's your opinion?
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.
LGTM
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.
LGTM
Co-Authored-By: lamxTyler <[email protected]>
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.
LGTM
/run-all-tests |
@@ -390,13 +390,11 @@ func (a *ExecStmt) LogSlowQuery(txnTS uint64, succ bool) { | |||
} | |||
execDetail := sessVars.StmtCtx.GetExecDetails() | |||
if costTime < threshold { | |||
if logutil.SlowQueryLogger.IsLevelEnabled(log.DebugLevel) { |
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.
I think this is useful 🤣
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 is already guaranteed by https://github.com/pingcap/tidb/pull/9521/files#diff-350127760839dbfd52d23927f7ff2d95R374.
What problem does this PR solve?
Unify log format in package
executor
.What is changed and how it works?
Replace log package with zap logger and follow the new log format.
Check List
Tests
Code changes
Side effects
Related changes