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

plan: add task profile. #2902

Merged
merged 6 commits into from
Mar 31, 2017
Merged

plan: add task profile. #2902

merged 6 commits into from
Mar 31, 2017

Conversation

hanfei1991
Copy link
Member

We are planning to split the physical plan to computation tasks. This pr only define the structure for kvTask and singleton Task.

@hanfei1991
Copy link
Member Author

@coocood
Copy link
Member

coocood commented Mar 20, 2017

This need a complete design document.

@shenli
Copy link
Member

shenli commented Mar 20, 2017

@coocood I am agree with you.

@coocood
Copy link
Member

coocood commented Mar 27, 2017

s/kvTask/copTask
s/singletonTask/rootTask

import "github.com/pingcap/tidb/expression"

// taskProfile is a new version of `PhysicalPlanInfo`. It stores cost information for a task.
// A task may be KVTask, SingletonTask, MPPTask or a ParallelTask.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the comment.


// taskProfile is a new version of `PhysicalPlanInfo`. It stores cost information for a task.
// A task may be KVTask, SingletonTask, MPPTask or a ParallelTask.
type taskProfile interface {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a toPB interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, toPB only required by kvTask.

}
}

// rootTaskProfile is a profile running on tidb with single goroutine.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is better?
"rootTaskProfile is the final sink node of a plan graph. It should be a single goroutine on tidb."

type copTaskProfile struct {
indexPlan PhysicalPlan
tablePlan PhysicalPlan
cst float64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use cost? cst is rarely used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cost is used in function name

func (t *copTaskProfile) finishTask() {
t.cst += float64(t.cnt) * netWorkFactor
if t.tablePlan != nil && t.indexPlan != nil {
t.cst += float64(t.cnt) * netWorkFactor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.cst *= 2?

tablePlan PhysicalPlan
cst float64
cnt uint64
addPlan2Index bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment for this.

return profile
}

func (sel *Selection) splitSelectionByIndexColumns(schema *expression.Schema) (indexSel *Selection, tableSel *Selection) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment for this function.

@hanfei1991
Copy link
Member Author

@shenli PTAL

@shenli
Copy link
Member

shenli commented Mar 30, 2017

LGTM
@coocood @zimulala @tiancaiamao PTAL

// taskProfile is a new version of `PhysicalPlanInfo`. It stores cost information for a task.
// A task may be CopTask, RootTask, MPPTask or a ParallelTask.
type taskProfile interface {
attachPlan(p PhysicalPlan) taskProfile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a function instead of interface method is more intuitive.

coocood
coocood previously approved these changes Mar 31, 2017
Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coocood coocood dismissed their stale review March 31, 2017 04:54

later

Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hanfei1991 hanfei1991 merged commit d0821da into master Mar 31, 2017
@hanfei1991 hanfei1991 deleted the hanfei/profile branch March 31, 2017 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants