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.

Sign up for GitHub

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

Jump to bottom

*: support disaggregated tiflash #33535

Merged
merged 57 commits into from
Dec 8, 2022
Merged

*: support disaggregated tiflash #33535

merged 57 commits into from
Dec 8, 2022

Conversation

guo-shaoge
Copy link
Collaborator

@guo-shaoge guo-shaoge commented Mar 29, 2022 •
edited
Loading

Signed-off-by: guo-shaoge [email protected]

What problem does this PR solve?

Issue Number: close #34707

Problem Summary:

  1. Support dispatch mppTask to tiflash_compute nodes. (store/copr: fix build batchCop in disaggregated tiflash mode #40008 updates build batchCop)
  2. Disable cop/batchCop protocol in disaggregated tiflash mode.
  3. For now, tidb will update its tiflash_compute store cache from PD every 10 seconds. We will use better ways in the near future.

What is changed and how it works?

Check List

Tests

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors: Add a new config disaggregated-tiflash
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

*: support disaggregated tiflash

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Mar 29, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • XuHuaiyu
  • wshwsh12

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 29, 2022
@sre-bot
Copy link
Contributor

sre-bot commented Mar 29, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@sre-bot
Copy link
Contributor

sre-bot commented Mar 29, 2022 •
edited
Loading

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/186a7a8c04f76f2b02522c1994bc554247ac3352

@guo-shaoge
distribute task randomly
56f98ca 
Signed-off-by: guo-shaoge <[email protected]>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 29, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 27, 2022
@sre-bot
Copy link
Contributor

sre-bot commented Apr 27, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@guo-shaoge
fix fmt
f7f0578 
Signed-off-by: guo-shaoge <[email protected]>
@sre-bot
Copy link
Contributor

sre-bot commented Apr 27, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@sre-bot
Copy link
Contributor

sre-bot commented Apr 28, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@guo-shaoge
Copy link
Collaborator Author

/hold

@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2022
@guo-shaoge guo-shaoge mentioned this pull request May 16, 2022
Open
XuHuaiyu
XuHuaiyu reviewed May 16, 2022
View reviewed changes
store/copr/mpp.go Outdated Show resolved Hide resolved
@guo-shaoge guo-shaoge mentioned this pull request May 16, 2022
Merged
@guo-shaoge guo-shaoge requested a review from a team as a code owner May 16, 2022 13:27
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 16, 2022
@sre-bot
Copy link
Contributor

sre-bot commented May 16, 2022

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@guo-shaoge guo-shaoge changed the title store/copr, kv, planner, executor: support dispatch tasks to tiflash ReadNode *: support disaggregated tiflash Dec 6, 2022
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 8, 2022
@guo-shaoge
add case
f7e278a 
Signed-off-by: guo-shaoge <[email protected]>
XuHuaiyu
XuHuaiyu reviewed Dec 8, 2022
View reviewed changes
planner/core/task.go Show resolved Hide resolved
store/copr/batch_coprocessor.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 8, 2022
wshwsh12
wshwsh12 reviewed Dec 8, 2022
View reviewed changes
domain/domain.go
logCount++
s.GetRegionCache().InvalidateTiFlashComputeStores()
if logCount == 60 {
// Print log every 60*duration seconds.
Copy link
Contributor

Choose a reason for hiding this comment

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

How to print the log every 60s? Or you mean 600s?

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 8, 2022
@guo-shaoge
Copy link
Collaborator Author

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: a50994d

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 8, 2022
@ti-chi-bot ti-chi-bot merged commit 4c04abc into pingcap:master Dec 8, 2022
@sre-bot
Copy link
Contributor

sre-bot commented Dec 8, 2022

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name Result Duration Compare with Parent commit
idc-jenkins-ci/integration-cdc-test 🟥 failed 1, success 39, total 40 22 min New failing
idc-jenkins-ci-tidb/mybatis-test 🔴 failed 1, success 0, total 1 4 min 9 sec Existing failure
idc-jenkins-ci-tidb/integration-common-test 🟢 all 17 tests passed 12 min Existing passed
idc-jenkins-ci-tidb/common-test 🟢 all 11 tests passed 10 min Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2 🟢 all 28 tests passed 6 min 41 sec Existing passed
idc-jenkins-ci-tidb/tics-test 🟢 all 1 tests passed 6 min 29 sec Existing passed
idc-jenkins-ci-tidb/integration-ddl-test 🟢 all 6 tests passed 6 min 4 sec Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1 🟢 all 26 tests passed 5 min 56 sec Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test 🟢 all 1 tests passed 3 min 20 sec Existing passed
idc-jenkins-ci-tidb/plugin-test 🟢 build success, plugin test success 4min Existing passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuHuaiyu XuHuaiyu XuHuaiyu approved these changes

@wshwsh12 wshwsh12 wshwsh12 approved these changes

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support dispatch mppTask/copTask/batchCopTask to tiflash_mpp nodes
5 participants
@guo-shaoge @ti-chi-bot @sre-bot @XuHuaiyu @wshwsh12