-
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
parser,ast: parse admin show slow
statement
#7688
Conversation
Add the following new syntax: * admin show log top [user | internal | all] N * admin show log recent N
parser/parser.y
Outdated
@@ -3606,6 +3611,10 @@ FunctionCallNonKeyword: | |||
Args: []ast.ExprNode{ast.NewValueExpr($3), $5}, | |||
} | |||
} | |||
| "LOG" '(' ExpressionListOpt ')' |
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 add this?
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.
After this PR, "LOG" change to a token from an identifier, it's a not keyword token.
There is no rule for builtin function "select log(...)" , so I add this rule.
parser/parser.y
Outdated
@@ -5221,6 +5230,53 @@ AdminStmt: | |||
JobIDs: $6.([]int64), | |||
} | |||
} | |||
| "ADMIN" "SHOW" "LOG" AdminShowLog |
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.
"LOG" is too general, "SLOW_QUERY" is better.
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.
@winkyao 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.
I vote for the slow query
.
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.
Support for using "LOG"
.
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.
How about ADMIN SHOW SLOW
? I agree that log is too generic, but with slow
it could be implied.
} | ||
|
||
AdminShowLog: | ||
"RECENT" NUM |
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.
We can remove the RECENT
and make the NUM
optional
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.
There will be two containers, one heap for TopN , one queue for Recent.
Suggested by @shenli
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.
No "TOP" means recent.
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.
TOP doesn't mean recent
. It means order by x limit n
.
parser/parser.y
Outdated
Count: getUint64FromNUM($2), | ||
} | ||
} | ||
| "TOP" "USER" NUM |
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.
"USER" can be removed, default only show "USER"
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.
Good point!
Count: getUint64FromNUM($3), | ||
} | ||
} | ||
| "TOP" "INTERNAL" NUM |
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.
This syntax can be removed, use ALL
is enough.
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.
Or Keep this and remove "ALL"
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.
How to represent the result "user" + "internal" ?
We have three cases:
- "user"
- "internal"
- "user" + "internal"
one keyword is not enough
|
mysql> admin show log recent 3;
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------+------------------------------------------------------------------------------------------------------+------+---------+--------------------+----------------+------+-----------------+----------------+----------+
| SQL | START | DURATION | DETAILS | SUCC | CONN_ID | TXNTS | USER | DB | TABLE_IDS | INDEX_IDS | INTERNAL |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------+------------------------------------------------------------------------------------------------------+------+---------+--------------------+----------------+------+-----------------+----------------+----------+
| select count(*) from t limit 110000 | 2018-09-13 14:20:55.715194 | 00:00:09 | process_time:1m6.188s wait_time:30.196s request_count:14 total_keys:20000014 processed_keys:20000000 | 1 | 1 | 402868051819626497 | root@127.0.0.1 | test | table_ids:[32], | | 1 |
| SELECT version, table_id, modify_count, count from mysql.stats_meta where version > 400040410672791554 order by version | 2018-09-13 14:20:57.148856 | 00:00:03 | wait_time:3.884s request_count:2 total_keys:266 processed_keys:144 | 1 | 0 | 402868052199735298 | | | table_ids:[19], | index_ids:[1], | 0 |
| select variable_name, variable_value from mysql.global_variables where variable_name in ('tidb_auto_analyze_ratio', 'tidb_auto_analyze_start_time', 'tidb_auto_analyze_end_time') | 2018-09-13 14:20:57.148658 | 00:00:03 | wait_time:3.884s request_count:1 total_keys:3 | 1 | 0 | 402868052199735297 | | | table_ids:[13], | index_ids:[1], | 0 |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------+------------------------------------------------------------------------------------------------------+------+---------+--------------------+----------------+------+-----------------+----------------+----------+
3 rows in set (0.00 sec) |
1 similar comment
ast/misc.go
Outdated
type ShowSlow struct { | ||
Tp ShowSlowType | ||
Count uint64 | ||
// May be "internal" or "all", default is "" |
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.
what about only showing general type?
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.
What do you mean by "general" type? is that "user + internal"?
"user" == "" (the default)
"user + internal" == "all"
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.
Please add some comment to explain what "user"
is, and the default.
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've defined a ShowSlowKind
type, instead of using string. @winkyao @crazycs520
admin show log
statementadmin show slow
statement
LGTM |
parser/parser.y
Outdated
{ | ||
$$ = &ast.ShowSlow{ | ||
Tp: ast.ShowSlowTop, | ||
Kind: "internal", |
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.
How about define a const
: KindInternal, KindAll, KindDefault ?
parser/parser.y
Outdated
{ | ||
$$ = &ast.ShowSlow{ | ||
Tp: ast.ShowSlowTop, | ||
Kind: "all", |
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.
dito
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 |
/run-all-tests |
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
What problem does this PR solve?
Add the following new syntax:
admin show slow top [internal | all] N
admin show slow recent N
I expect to use it like this:
What is changed and how it works?
Update
parser.y
Check List
Related changes
PTAL @winkyao @coocood