-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
coprocessor: split ranges if the slice is too big #7454
Conversation
store/tikv/coprocessor.go
Outdated
cmdType: cmdType, | ||
}) | ||
rLen := ranges.len() | ||
for i := 0; i < rLen; { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add comments about why slice the task by range.
/run-all-tests |
store/tikv/coprocessor.go
Outdated
@@ -235,6 +236,8 @@ func (r *copRanges) split(key []byte) (*copRanges, *copRanges) { | |||
return r.slice(0, n), r.slice(n, r.len()) | |||
} | |||
|
|||
const rangesPerTask = 25000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to add some comment about what this value is used to do, why need it, why set it to this value.
store/tikv/coprocessor.go
Outdated
respChan: make(chan *copResponse, 1), | ||
cmdType: cmdType, | ||
}) | ||
// etcd will return error if the message is too large. So we need to limit the length of the ranges slice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tikv will return gRPC error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
What problem does this PR solve?
This PR aims to solve the issue that the TiKV timeout error is caused by the too large range in one request.
What is changed and how it works?
According to @coocood 's advice, split the range in one region when building
copTask
. So we won't send a message that is too large.Check List
Tests
select count(*) from t where key in (800k constant list)
.