Skip to content
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

GetExplainRowsForPlan called twice for the same query #30360

Open
mjonss opened this issue Dec 2, 2021 · 2 comments · May be fixed by #30968
Open

GetExplainRowsForPlan called twice for the same query #30360

mjonss opened this issue Dec 2, 2021 · 2 comments · May be fixed by #30968
Assignees
Labels
severity/moderate sig/planner SIG: Planner type/bug The issue is confirmed as a bug.

Comments

@mjonss
Copy link
Contributor

mjonss commented Dec 2, 2021

Bug Report

When profiling a SELECT * FROM partitioned_table I found that GetExplainRowsForPlan was called twice and took about 25% of the time when executing the query.

I wonder if it is even needed to call at all, since the plan is already stored, and could possibly be extended with GetExplainRowsForPlan when needed by EXPLAIN FOR ...

Found when first hacking out/disable reArrangeFallback in #30353 to see what else could be made faster.

1. Minimal reproduce step (Required)

create table tp (a int primary key, b varchar(255)) partition by hash (a) partitions 8192;
insert into tp values (1, 'row with id 1, partition p1 filler data'), (2, 'partition p2, filler data for row with id 2'), (9, 'filler data for row with id 9, partition p9');
-- start profiling and run this over and over again for profiling:
select * from tp;

2. What did you expect to see? (Required)

GetExplainRowsForPlan to be called once, or not at all, except when EXPLAIN FOR... would be called.

3. What did you see instead (Required)

GetExplainRowsForPlan was called twice per query.

4. What is your TiDB version? (Required)

tidb_version(): Release Version: v5.4.0-alpha-289-g92c7c075d5-dirty
Edition: Community
Git Commit Hash: 92c7c07
Git Branch: master
UTC Build Time: 2021-12-02 10:39:36
GoVersion: go1.17
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false

Some flame graphs and cumulative profile:
full-profile
zoom-profile-dispatch
cumulative-profile

@mjonss mjonss added the type/bug The issue is confirmed as a bug. label Dec 2, 2021
@aytrack aytrack added the sig/planner SIG: Planner label Dec 3, 2021
@Reminiscent Reminiscent linked a pull request Dec 23, 2021 that will close this issue
12 tasks
@Reminiscent
Copy link
Contributor

@mjonss How to disable reArrangeFallback for the test?

@mjonss
Copy link
Contributor Author

mjonss commented Jan 5, 2022

@mjonss How to disable reArrangeFallback for the test?

Something hackish like this:

diff --git a/util/memory/tracker.go b/util/memory/tracker.go
index 470029e309..9d861e81c7 100644
--- a/util/memory/tracker.go
+++ b/util/memory/tracker.go
@@ -159,17 +159,23 @@ func (t *Tracker) SetActionOnExceed(a ActionOnExceed) {
 // FallbackOldAndSetNewAction sets the action when memory usage exceeds bytesHardLimit
 // and set the original action as its fallback.
 func (t *Tracker) FallbackOldAndSetNewAction(a ActionOnExceed) {
-       t.actionMuForHardLimit.Lock()
-       defer t.actionMuForHardLimit.Unlock()
-       t.actionMuForHardLimit.actionOnExceed = reArrangeFallback(t.actionMuForHardLimit.actionOnExceed, a)
+       return
+       /*
+               t.actionMuForHardLimit.Lock()
+               defer t.actionMuForHardLimit.Unlock()
+               t.actionMuForHardLimit.actionOnExceed = reArrangeFallback(t.actionMuForHardLimit.actionOnExceed, a)
+       */
 }
 
 // FallbackOldAndSetNewActionForSoftLimit sets the action when memory usage exceeds bytesSoftLimit
 // and set the original action as its fallback.
 func (t *Tracker) FallbackOldAndSetNewActionForSoftLimit(a ActionOnExceed) {
-       t.actionMuForSoftLimit.Lock()
-       defer t.actionMuForSoftLimit.Unlock()
-       t.actionMuForSoftLimit.actionOnExceed = reArrangeFallback(t.actionMuForSoftLimit.actionOnExceed, a)
+       return
+       /*
+               t.actionMuForSoftLimit.Lock()
+               defer t.actionMuForSoftLimit.Unlock()
+               t.actionMuForSoftLimit.actionOnExceed = reArrangeFallback(t.actionMuForSoftLimit.actionOnExceed, a)
+       */
 }
 
 // GetFallbackForTest get the oom action used by test.
@@ -306,7 +312,8 @@ func (t *Tracker) ReplaceChild(oldChild, newChild *Tracker) {
 // which means this is a memory release operation. When memory usage of a tracker
 // exceeds its bytesSoftLimit/bytesHardLimit, the tracker calls its action, so does each of its ancestors.
 func (t *Tracker) Consume(bytes int64) {
-       if bytes == 0 {
+
+       if bytes != 999999999 {
                return
        }
        var rootExceed, rootExceedForSoftLimit *Tracker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/moderate sig/planner SIG: Planner type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants