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

planner: adjust the cost model of Apply #13550

Merged
merged 7 commits into from
Jan 15, 2020
Merged

Conversation

francis0407
Copy link
Member

What problem does this PR solve?

The cost of Apply was only calculated by selfCost + leftCost, which did not consider the cost of the outer side.

What is changed and how it works?

Integrate the cost of outer side into the cost model of Apply.

Check List

Tests

  • No code

Code changes

  • Has exported function/method change

@francis0407 francis0407 added the sig/planner SIG: Planner label Nov 18, 2019
@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@1a5c19b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             master    #13550   +/-   ##
==========================================
  Coverage          ?   80.251%           
==========================================
  Files             ?       472           
  Lines             ?    114725           
  Branches          ?         0           
==========================================
  Hits              ?     92068           
  Misses            ?     15438           
  Partials          ?      7219

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

planner/core/task.go Outdated Show resolved Hide resolved
planner/core/task.go Outdated Show resolved Hide resolved
@sre-bot
Copy link
Contributor

sre-bot commented Jan 5, 2020

@francis0407, please update your pull request.

@francis0407 francis0407 requested a review from a team as a code owner January 8, 2020 07:05
@ghost ghost requested review from lzmhhh123 and winoros and removed request for a team January 8, 2020 07:05
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM

@sre-bot
Copy link
Contributor

sre-bot commented Jan 15, 2020

@francis0407, please update your pull request.

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka
Copy link
Contributor

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 15, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 15, 2020

Your auto merge job has been accepted, waiting for 14263

@sre-bot
Copy link
Contributor

sre-bot commented Jan 15, 2020

/run-all-tests

@sre-bot sre-bot merged commit e39b504 into pingcap:master Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants