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.

Sign up for GitHub

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

Jump to bottom

session/statistics: discard feedbacks from delete / update #17452

Merged
merged 15 commits into from
Jun 8, 2020
Merged

session/statistics: discard feedbacks from delete / update #17452

merged 15 commits into from
Jun 8, 2020

Conversation

eurekaka
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #17438

Problem Summary:

Feedbacks from update, delete may be invalid.

What is changed and how it works?

What's Changed:

Discard those feedbacks.

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

N/A

Release note

  • Discard feedbacks generated from delete / update statements.

@eurekaka eurekaka requested a review from a team as a code owner May 27, 2020 08:19
@ghost ghost requested review from XuHuaiyu and removed request for a team May 27, 2020 08:19
@eurekaka
Copy link
Contributor Author

/run-unit-test

zz-jason
zz-jason reviewed May 27, 2020
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

It's better to not collect feedback in the execution phase instead of discarding the collected feedback.

session/session.go Outdated Show resolved Hide resolved
@zz-jason
Copy link
Member

@eurekaka please also cherry-pick this change to release 2.1, 3.0, 3.1 and 4.0. Since query feedback is enabled by default on these branches.

@eurekaka eurekaka requested a review from a team as a code owner May 27, 2020 09:33
@ghost ghost removed their request for review May 27, 2020 09:34
@github-actions github-actions bot added the sig/execution SIG execution label May 27, 2020
@eurekaka eurekaka requested a review from zz-jason May 27, 2020 11:59
@codecov
Copy link

codecov bot commented May 27, 2020 •
edited
Loading

Codecov Report

Merging #17452 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #17452   +/-   ##
===========================================
  Coverage   79.5350%   79.5350%           
===========================================
  Files           524        524           
  Lines        141598     141598           
===========================================
  Hits         112620     112620           
  Misses        19924      19924           
  Partials       9054       9054

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented May 28, 2020 •
edited
Loading

Should we also handle insert select

XuHuaiyu
XuHuaiyu reviewed May 28, 2020
View reviewed changes
statistics/feedback.go Show resolved Hide resolved
statistics/handle/update_test.go Show resolved Hide resolved
executor/distsql.go Outdated Show resolved Hide resolved
@eurekaka eurekaka requested a review from XuHuaiyu May 28, 2020 09:56
@eurekaka
Copy link
Contributor Author

eurekaka commented Jun 3, 2020

/run-check_dev_2

2 similar comments
@eurekaka
Copy link
Contributor Author

eurekaka commented Jun 3, 2020

/run-check_dev_2

@eurekaka
Copy link
Contributor Author

eurekaka commented Jun 3, 2020

/run-check_dev_2

zz-jason
zz-jason reviewed Jun 3, 2020
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 3, 2020
XuHuaiyu
XuHuaiyu approved these changes Jun 3, 2020
View reviewed changes
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason
Copy link
Member

zz-jason commented Jun 8, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

Your auto merge job has been accepted, waiting for:

  • 17026

@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

@eurekaka merge failed.

@eurekaka
Copy link
Contributor Author

eurekaka commented Jun 8, 2020

/run-integration-copr-test

@eurekaka eurekaka merged commit b053275 into pingcap:master Jun 8, 2020
@eurekaka eurekaka deleted the delete_feedback branch June 8, 2020 06:35
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 8, 2020
@eurekaka @sre-bot
cherry pick pingcap#17452 to release-2.1
7e1b0cb 
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot sre-bot mentioned this pull request Jun 8, 2020
Merged
@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

cherry pick to release-2.1 in PR #17840

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 8, 2020
@eurekaka @sre-bot
cherry pick pingcap#17452 to release-3.0
e2ac8bc 
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot sre-bot mentioned this pull request Jun 8, 2020
Merged
@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

cherry pick to release-3.0 in PR #17841

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 8, 2020
@eurekaka @sre-bot
cherry pick pingcap#17452 to release-3.1
e161182 
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot sre-bot mentioned this pull request Jun 8, 2020
Merged
@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

cherry pick to release-3.1 in PR #17842

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 8, 2020
@eurekaka @sre-bot
cherry pick pingcap#17452 to release-4.0
aefc903 
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot sre-bot mentioned this pull request Jun 8, 2020
Merged
@sre-bot
Copy link
Contributor

sre-bot commented Jun 8, 2020

cherry pick to release-4.0 in PR #17843

zz-jason pushed a commit that referenced this pull request Jun 19, 2020
@sre-bot
session/statistics: discard feedbacks from `delete ... (#17452) (#17843)
99dc723
ti-srebot added a commit that referenced this pull request Jun 22, 2020
@sre-bot @eurekaka @ti-srebot
session/statistics: discard feedbacks from delete / update (#17452)…
10140bb 
… (#17840)

* cherry pick #17452 to release-2.1

Signed-off-by: sre-bot <sre-bot@pingcap.com>

* resolve conflicts

Co-authored-by: Kenan Yao <cauchy1992@gmail.com>
Co-authored-by: ti-srebot <66930949+ti-srebot@users.noreply.github.com>
ti-srebot added a commit that referenced this pull request Jun 28, 2020
@sre-bot @eurekaka @ti-srebot
session/statistics: discard feedbacks from delete / update (#17452)…
b5ae1e7 
… (#17841)

Signed-off-by: sre-bot <sre-bot@pingcap.com>

Co-authored-by: Kenan Yao <cauchy1992@gmail.com>
Co-authored-by: ti-srebot <66930949+ti-srebot@users.noreply.github.com>
ti-srebot pushed a commit that referenced this pull request Jul 21, 2020
@sre-bot
session/statistics: discard feedbacks from delete / update (#17452)…
71dc437 
… (#17842)

Signed-off-by: sre-bot <sre-bot@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zz-jason zz-jason zz-jason left review comments

@XuHuaiyu XuHuaiyu XuHuaiyu approved these changes

Assignees
No one assigned
Labels
component/statistics epic/query-feedback-GA sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

discard feedbacks from delete / update statements
4 participants
@eurekaka @zz-jason @XuHuaiyu @sre-bot