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

distsql: check the killed flag in the distsql package #15592

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Problem Summary:

Fix an issue from the forum https://asktug.com/t/tidb-new-feature-max-execution-time/309

select user_id,user_name,user_content  from test where user_content = '123456';

There is no index in this query and TiKV scans the whole table.
max_excution_time doesn't work because the killed flag is checked in each Next function,
the code doesn't run to Next until it read sufficient rows in a chunk.

What is changed and how it works?

Check the killed flag in the distsql package level, so the query could break after reading each region result. The code looks like that:

for  {
        if region result == nil {
               fetch next region
        }
        fillChunkWithRegionResult
}

https://github.com/pingcap/tidb/compare/master...tiancaiamao:coprocessor-kill?expand=1#diff-7492d01b7aac81533194642850a45b8eR206

Related changes

#14552
#12852

  • Need to cherry-pick to the release branch

Check List

Tests

  • No code

Release note

Improve max-execution-time usability by checking the interrupt signal when a new region data is read.

@tiancaiamao tiancaiamao added type/enhancement The issue or PR belongs to an enhancement. component/server labels Mar 23, 2020
@tiancaiamao tiancaiamao requested a review from a team as a code owner March 23, 2020 12:52
@ghost ghost requested review from SunRunAway and wshwsh12 and removed request for a team March 23, 2020 12:52
@tiancaiamao
Copy link
Contributor Author

PTAL @lysu @jackysp

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu added status/LGT1 Indicates that a PR has LGTM 1. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 24, 2020
@tiancaiamao tiancaiamao merged commit 00dc69c into pingcap:master Mar 24, 2020
@tiancaiamao tiancaiamao deleted the coprocessor-kill branch March 24, 2020 03:24
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 24, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 24, 2020

cherry pick to release-3.0 in PR #15615

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 24, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 24, 2020

cherry pick to release-3.1 in PR #15616

@sre-bot
Copy link
Contributor

sre-bot commented Mar 24, 2020

cherry pick to release-4.0 in PR #15617

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants