-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
domain: add recent slow queries queue and ShowSlowQuery() function #7692
Conversation
Why put it in the domain package? |
Where should I put it ? @shenli |
PTAL @coocood |
domain/topn_slow_query.go
Outdated
count = len(q.data) | ||
} | ||
ret := make([]*SlowQueryInfo, 0, count) | ||
tail := (q.tail - 1 + len(q.data)) % len(q.data) |
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 is simpler
if q.head > q.tail {
ret = append(ret, q.data[head:]...)
ret = append(ret, q.data[:head]...)
} else {
ret = append(ret, q.data[head:tail+1]...)
}
ret = ret[len(ret)-count:]
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's not simpler, it brings more mental burden.
domain/topn_slow_query.go
Outdated
ret = append(ret, h.data[i]) | ||
} | ||
|
||
// Sort breaks the data, recover it here. |
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.
The sorted array still maintains the heap property, we don't need to copy and recover.
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 are two problems here:
- To keep top N slow queries, we maintains a min heap, but sort will use a reverse order
- There is a
count
argument, so thesorted[count]
is not the same withheap[len]
PTAL @coocood |
domain/topn_slow_query.go
Outdated
@@ -102,8 +156,46 @@ func (q *topNSlowQueries) Append(info *SlowQueryInfo) { | |||
} | |||
|
|||
func (q *topNSlowQueries) Refresh(now time.Time) { |
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 Refresh may not be a good name for its functionality.
What it does is cleaning up outdated entries, but Refresh
usually means getting something new.
How about RemoveExpired
.
PTAL @coocood |
LGTM |
PTAL @winkyao |
domain/topn_slow_query.go
Outdated
tmp1 := slowQueryHeap{tmp} | ||
sort.Sort(&tmp1) | ||
ret = make([]*SlowQueryInfo, 0, count) | ||
for i := len(tmp) - 1; i >= 0 && len(ret) <= count; i-- { |
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.
len(ret) < count ?
ast/misc.go
Outdated
@@ -626,6 +626,26 @@ type HandleRange struct { | |||
End int64 | |||
} | |||
|
|||
// ShowLogType defines the type for SlowLog. | |||
type ShowLogType int |
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.
s/ShowLogType/ShowSlowQueryType/
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.
When I merge master, this would be solved.
ast/misc.go
Outdated
// ShowLog is used for the following command: | ||
// admin show log top [user | internal | all] N | ||
// admin show log recent N | ||
type ShowLog struct { |
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.
s/ShowLog/ShowQueries/
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.
ditto
/run-all-tests |
PTAL @crazycs520 @zz-jason |
/run-all-tests |
return | ||
} | ||
|
||
q.data = append(q.data, info)[1:] |
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.
If you just use slice here, the q.data will keep the continuous memory, it may cause oom. What about using a list?
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.
Do you do a large amount insert test?
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.
@coocood suggest me to use this.
The code is the simplest queue implementation, although it double the memory usage.
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.
The queue size will be 500.
We just keep the most recent queries in the queue.
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?
Make user easier to debug slow queries.
What is changed and how it works?
ShowSlowQuery()
functionCheck List
Tests
Code changes
domain.ShowSlowQuery()
Related changes
It's provided to support
admin show log
in #7688@winkyao @coocood