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

*: support read tidb cluster memory table #13065

Merged
merged 81 commits into from
Dec 4, 2019
Merged

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Oct 31, 2019

What problem does this PR solve?

*support read tidb cluster memory table in TiDB. This comes from PingCAP hackathon 2019.

  • target: Read all TiDB memory-table as 1 cluster table.

Demo

mysql>select count(*),node_id from `TiDB_CLUSTER_SLOW_QUERY` group by node_id;
+----------+---------+
| count(*) | node_id |
+----------+---------+
| 1184     | tidb9   |
| 22       | tidb10  |
| 15       | tidb11  |
+----------+---------+
mysql>desc select count(*)  from `TiDB_CLUSTER_SLOW_QUERY`;
+----------------------------+----------+----------------+-----------------------------------------------------------------------------+
| id                         | count    | task           | operator info                                                               |
+----------------------------+----------+----------------+-----------------------------------------------------------------------------+
| StreamAgg_19               | 1.00     | root           | funcs:count(Column#35)                                                      |
| └─ClusterMemTableReader_20 | 1.00     | root           | data:StreamAgg_8                                                            |
|   └─StreamAgg_8            | 1.00     | cop[tidbs_mem] | funcs:count(1)                                                              |
|     └─MemTableScan_17      | 10000.00 | cop[tidbs_mem] | table:SLOW_QUERY_CLUSTER, range:[-inf,+inf], keep order:false, stats:pseudo |
+----------------------------+----------+----------------+-----------------------------------------------------------------------------+
  • cop[tidbs_mem] means the table was stored in the all tidb server memory. And we can push the StreamAgg executor to another TiDB coprocessor to distributed execution, Just like TiKV.

What is changed and how it works?

Architecture

image

  • Start an RPC server, share the 10080 port with HTTP services.
  • Support the same Coprocessor with TiKV. The coprocessor will reuse the tidb executor.
  • Reuse the tipb.TableScan to read memory table.
    • Since tipb.TableScan use table id to locate table, and before this PR, the memory table id maybe not same with another tidb nodes, because the memory table id was allocated in local. So this PR also allocates a constant id for system schema/table.

TiDB coprocessor work flow

  1. Get a Cop request.
  2. build physical plan from protobuf DAG.
  3. build executor from physical plan.
  4. execute.
  5. encode result data to cop response.
  6. return.

Attension
Since TiDB coprocessor reuses the TiDB executor, but the tidb aggregation executor is a little different from coprocessor aggregation executor:

TiDB aggregation executor won't out the group by value, but coprocessor aggregation executor does. So we need a special physical plan for TiDB coprocessor when
TiDB coprocessor aggregation executor was used. We need to add firstrow for every group by expression, then the TiDB coprocessor aggregation executor will output the group by value.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change

Side effects

Related changes

Release note

  • Support read TiDB cluster memory table

@crazycs520
Copy link
Contributor Author

/run-all-tests

@lonng
Copy link
Contributor

lonng commented Nov 1, 2019

IMO, the SLOW_QUERY_CLUSTER is not a good name, how about tidb_cluster_slow_query?

@crazycs520
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520 crazycs520 added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 3, 2019
@lonng
Copy link
Contributor

lonng commented Dec 4, 2019

The TiDB prefix indicates this table is only exists in TiDB

@lonng @crazycs520 Why do we need to mark only exists in TiDB?

It's used to distinguish to MySQL system table (But you are right, maybe we don't need this prefix and should remove this prefix).

Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

Rest LGTM

Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng
Copy link
Contributor

lonng commented Dec 4, 2019

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

@crazycs520 merge failed.

@crazycs520
Copy link
Contributor Author

/run-integration-compatibility-test

@crazycs520
Copy link
Contributor Author

/run-check_dev_2

@lonng
Copy link
Contributor

lonng commented Dec 4, 2019

/run-all-tests

@lonng
Copy link
Contributor

lonng commented Dec 4, 2019

/run-unit_test

@lonng
Copy link
Contributor

lonng commented Dec 4, 2019

/test

@crazycs520
Copy link
Contributor Author

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/new-feature type/usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.