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

Make index serial scan concurrency configurable and fix golint errors. #2928

Merged
merged 5 commits into from
Mar 26, 2017

Conversation

shenli
Copy link
Member

@shenli shenli commented Mar 26, 2017

When doing index double read operation with keepOrder as true, we use only one worker. If we need to read a lot of data, it will be quite slow. So I make it configurable. For OLAP TiDB cluster, we could set the concurrency to 4 or 5.

I will add test in the next commit.

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.

Set the default value in NewSessionVars.

@@ -70,6 +70,10 @@ const (
// Set this value higher may reduce the latency but consumes more system resource.
TiDBIndexLookupConcurrency = "tidb_index_lookup_concurrency"

// tidb_index_serial_scan_concurrency is user for controling the concurrency of index scan operation
Copy link
Member

Choose a reason for hiding this comment

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

is used

@shenli shenli changed the title [DNM] Make index serial scan concurrency configurable and fix golint errors. Make index serial scan concurrency configurable and fix golint errors. Mar 26, 2017
@shenli
Copy link
Member Author

shenli commented Mar 26, 2017

@coocood @tiancaiamao @zimulala PTAL

@ngaut
Copy link
Member

ngaut commented Mar 26, 2017

Hi, do we have any bench results ?

@coocood
Copy link
Member

coocood commented Mar 26, 2017

LGTM

@shenli
Copy link
Member Author

shenli commented Mar 26, 2017

@ngaut In one of SMJ case, I increase the concurrency from 1 to 3 and query time drop from 2min to 1min 30sec. But this will cost more memory.

Copy link
Member

@hanfei1991 hanfei1991 left a comment

Choose a reason for hiding this comment

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

LGTM

@ngaut
Copy link
Member

ngaut commented Mar 26, 2017

Cool

@shenli shenli merged commit 6aa45e4 into master Mar 26, 2017
@shenli shenli deleted the shenli/double-read-concurrency branch March 26, 2017 16:57
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.

4 participants