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

infoschema: migrate dataForProcesslist from package infoschema to executor #15340

Merged
merged 4 commits into from
Mar 20, 2020
Merged

infoschema: migrate dataForProcesslist from package infoschema to executor #15340

merged 4 commits into from
Mar 20, 2020

Conversation

hsqlu
Copy link
Contributor

@hsqlu hsqlu commented Mar 12, 2020 •
edited
Loading

UCP: #15020

What problem does this PR solve?

fixes #15020

What is changed and how it works?

move dataForProcesslist from infoschema to executor.
move dataForClusterProcesslist from infoschema to executor.

Check List

Tests

  • Unit test

Code changes

  • Has exported variable/fields change

@hsqlu hsqlu requested a review from a team as a code owner March 12, 2020 10:37
@ghost ghost requested review from qw4990 and XuHuaiyu and removed request for a team March 12, 2020 10:37
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Mar 12, 2020
@github-actions github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution labels Mar 12, 2020
@codecov
Copy link

codecov bot commented Mar 12, 2020 •
edited
Loading

Codecov Report

Merging #15340 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15340   +/-   ##
===========================================
  Coverage   80.6811%   80.6811%           
===========================================
  Files           502        502           
  Lines        135360     135360           
===========================================
  Hits         109210     109210           
  Misses        17695      17695           
  Partials       8455       8455

@sre-bot
Copy link
Contributor

sre-bot commented Mar 13, 2020

Thanks for your pull request. Pick up issue #15020 and reopen this PR

@sre-bot sre-bot closed this Mar 13, 2020
@zz-jason zz-jason reopened this Mar 13, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 13, 2020

Thanks for your contribution. If your PR get merged, you will be rewarded 50 points.

@zz-jason zz-jason requested review from lonng and crazycs520 and removed request for qw4990 and XuHuaiyu March 15, 2020 02:09
lonng
lonng reviewed Mar 15, 2020
View reviewed changes
infoschema/tables_test.go Outdated
@@ -743,76 +740,6 @@ func (s *testTableSuite) TestForTableTiFlashReplica(c *C) {
tk.MustQuery("select TABLE_SCHEMA,TABLE_NAME,REPLICA_COUNT,LOCATION_LABELS,AVAILABLE, PROGRESS from information_schema.tiflash_replica").Check(testkit.Rows("test t 2 a,b 1 1"))
}

func (s *testClusterTableSuite) TestForClusterServerInfo(c *C) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the test cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By mistake, sorry! Just wondering why this unit test failed when I run go test under this package infoshema.

@hsqlu hsqlu force-pushed the ucp-migrate-dataForProcesslist branch from 3757e2b to 918df7d Compare March 19, 2020 03:49
lonng
lonng reviewed Mar 19, 2020
View reviewed changes
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

reafans
reafans previously approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@reafans reafans left a comment

Choose a reason for hiding this comment

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

LGTM

@reafans
Copy link
Contributor

reafans commented Mar 19, 2020

/merge

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

sre-bot commented Mar 19, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 19, 2020

@hsqlu merge failed.

@reafans
Copy link
Contributor

reafans commented Mar 20, 2020

/run-all-tests

@reafans
Copy link
Contributor

reafans commented Mar 20, 2020

Hi, @hsqlu, thanks for your contribution, please fix the lint below:

[2020-03-19T20:27:57.847Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:57.847Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:57.847Z] make: *** [importer] Error 2

[2020-03-19T20:27:58.105Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:58.105Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:58.105Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:58.105Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:58.105Z] make: *** [server] Error 2

[2020-03-19T20:27:58.105Z] make: *** [server] Error 2

script returned exit code 2

@hsqlu hsqlu force-pushed the ucp-migrate-dataForProcesslist branch from e4fda54 to 530dc8b Compare March 20, 2020 08:37
@hsqlu
Copy link
Contributor Author

hsqlu commented Mar 20, 2020

/run-all-tests

@hsqlu
Copy link
Contributor Author

hsqlu commented Mar 20, 2020

Hi, @hsqlu, thanks for your contribution, please fix the lint below:

[2020-03-19T20:27:57.847Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:57.847Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:57.847Z] make: *** [importer] Error 2

[2020-03-19T20:27:58.105Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:58.105Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:58.105Z] # github.com/pingcap/tidb/infoschema

[2020-03-19T20:27:58.105Z] infoschema/tables.go:35:2: imported and not used: "github.com/pingcap/tidb/privilege"

[2020-03-19T20:27:58.105Z] make: *** [server] Error 2

[2020-03-19T20:27:58.105Z] make: *** [server] Error 2

script returned exit code 2

Resloved. PTAL Thanks!

reafans
reafans approved these changes Mar 20, 2020
View reviewed changes
Copy link
Contributor

@reafans reafans left a comment

Choose a reason for hiding this comment

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

lgtm

@reafans
Copy link
Contributor

reafans commented Mar 20, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

/run-all-tests

@sre-bot sre-bot merged commit edd2810 into pingcap:master Mar 20, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

Congratulation! You have awarded a badge for usability challenge program! Please fill the form to get your reward! http://tidbcommunity.mikecrm.com/QMCv4QL

@sre-bot
Copy link
Contributor

sre-bot commented Mar 20, 2020

Team hsqlu complete task #15020 and get 50 score, currerent score 50

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

@lonng lonng lonng left review comments

@reafans reafans reafans approved these changes

@crazycs520 crazycs520 Awaiting requested review from crazycs520

Assignees
No one assigned
Labels
contribution This PR is from a community contributor. sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UCP: Migrate dataForProcesslist from package infoschema to executor
5 participants
@hsqlu @sre-bot @reafans @lonng @zz-jason