-
Notifications
You must be signed in to change notification settings - Fork 409
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
[DNM] refine handle table scan #4674
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/run-integration-test |
@@ -48,11 +48,12 @@ class TiDBTableScan | |||
} | |||
String getTableScanExecutorID() const | |||
{ | |||
return table_scan->executor_id(); | |||
return executor_id; |
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.
Why not just use the table_scan->executor_id()
but save a copy of query_block.source_name
as the executor_id?
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.
For coprocess read, table_scan->executor_id() may be empty.
query_block.source_name is guaranteed not to be empty(If executor_id is empty, an id will be assigned).
Would it be better to share the same executor_id for local read and remote read?
/run-integration-test |
/run-integration-test |
This pr locks the table before doing learner read, this is the same as our first implemention, but we met some deadlock related issue, and this pr moved the |
Maybe I need to move |
@SeaRise I prefer to do it this way SeaRise#1
|
/hold |
What problem does this PR solve?
Issue Number: ref #4118
Problem Summary:
What is changed and how it works?
DAGQueryBlock
inDAGStorageInterpreter
StorageWithStructureLock
(table metadata and table lock) fromDAGStorageInterpreter
toTiDBStorageTable
.Check List
Tests
Side effects
Documentation
Release note