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

stats: support dump stats for partition table #7753

Merged
merged 2 commits into from
Sep 22, 2018

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Sep 20, 2018

What problem does this PR solve?

Support dump statistics for partition table.

What is changed and how it works?

Add a new field in the json to store the partition stats.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • None

Related changes

  • None

@alivxxx alivxxx added component/statistics type/enhancement The issue or PR belongs to an enhancement. labels Sep 20, 2018
for _, def := range pi.Definitions {
tbl := jsonTbl.Partitions[def.Name.L]
if tbl == nil {
continue
Copy link
Member

Choose a reason for hiding this comment

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

Should we continue rather than throwing an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, when we dump the stats, if some partition's stats is dropped, we just skip that partition, so we need to skip here too.

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@winoros winoros added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 21, 2018
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 21, 2018
@zz-jason
Copy link
Member

/run-all-tests

@zz-jason zz-jason merged commit 83a923e into pingcap:master Sep 22, 2018
@alivxxx alivxxx deleted the partition-dump branch September 25, 2018 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants