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 partition table in tiflash #14735

Merged
merged 14 commits into from
Feb 13, 2020

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Feb 11, 2020

Signed-off-by: crazycs [email protected]

What problem does this PR solve?

Support set tiflash replica for partition table.

For the partition table, need a separate variable for separate partition to indicate the partition tiflash replica was available. Only when the all partition table replica was available, then the table tiflash replica was available.

Related Parser PR: pingcap/parser#742

What is changed and how it works?

For partition table tiflash replica.

  • When getting data from HTTP API /tiflash/replica should return partition ID, not table ID.
  • When post data to HTTP API /tiflash/replica, it should treat the id as partition ID.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Support partition table in tiflash.

@crazycs520
Copy link
Contributor Author

/rebuild

@zanmato1984
Copy link
Contributor

/run-build-image

ddl/table.go Show resolved Hide resolved
ddl/db_test.go Outdated Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
server/http_handler_test.go Show resolved Hide resolved
Signed-off-by: crazycs <[email protected]>
@crazycs520 crazycs520 added this to the v3.1.0-rc milestone Feb 12, 2020
Deardrops
Deardrops previously approved these changes Feb 12, 2020
Copy link
Contributor

@Deardrops Deardrops left a comment

Choose a reason for hiding this comment

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

LGTM

@Deardrops Deardrops added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 12, 2020
@Deardrops
Copy link
Contributor

LGTM

ddl/db_test.go Outdated Show resolved Hide resolved
Co-Authored-By: tangenta <[email protected]>
Copy link
Contributor

@tangenta tangenta 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

ddl/table.go Show resolved Hide resolved
ddl/table.go Show resolved Hide resolved
Signed-off-by: crazycs <[email protected]>
tangenta
tangenta previously approved these changes Feb 12, 2020
Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

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

LGTM

@tangenta tangenta added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 12, 2020
infoschema/infoschema.go Show resolved Hide resolved
ddl/table.go Outdated
break
}
}
tblInfo.TiFlashReplica.Available = false
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we deal with it when the physical ID is not found in tblInfo.TiFlashReplica.AvailablePartitionIDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, this line should put in line #765;

If physical ID is not found, do nothing, just ignore it will be ok.

Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala zimulala added status/LGT3 The PR has already had 3 LGTM. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Feb 13, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 13, 2020

/run-all-tests

@sre-bot sre-bot merged commit 9ffea35 into pingcap:master Feb 13, 2020
crazycs520 added a commit to lzmhhh123/tidb that referenced this pull request Feb 13, 2020
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants