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

add real tables for global/session status in performance schema #4523

Merged
merged 8 commits into from Sep 18, 2017

Conversation

Projects
None yet
6 participants
@wentaoxu
Contributor

wentaoxu commented Sep 14, 2017

  1. add system variable tables for performance schema, this is very similar to the table in info schema, wish to merge this two tables in the furture
  2. use status got from session context to fill in the two tables, to support ssl for x-protocol
@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Sep 14, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Sep 14, 2017

CLA assistant check
All committers have signed the CLA.

@wentaoxu

This comment has been minimized.

Show comment
Hide comment
@wentaoxu
Contributor

wentaoxu commented Sep 14, 2017

@wentaoxu

This comment has been minimized.

Show comment
Hide comment
@wentaoxu

wentaoxu Sep 15, 2017

Contributor

fix all comments and PTAL @coocood @jackysp

Contributor

wentaoxu commented Sep 15, 2017

fix all comments and PTAL @coocood @jackysp

Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated table/tables/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated table/table.go
@wentaoxu

This comment has been minimized.

Show comment
Hide comment
@wentaoxu

wentaoxu Sep 15, 2017

Contributor

fix all comments, PTAL @coocood @jackysp

Contributor

wentaoxu commented Sep 15, 2017

fix all comments, PTAL @coocood @jackysp

rows := [][]types.Datum{}
for status, v := range statusVars {
if ds.globalScope && v.Scope == variable.ScopeSession {

This comment has been minimized.

@coocood

coocood Sep 15, 2017

Member

What if data source is session scope and v.Scop == variable.ScopeGlobal ?
And NoneScope seems should not be added too, I'm not sure about it.

@coocood

coocood Sep 15, 2017

Member

What if data source is session scope and v.Scop == variable.ScopeGlobal ?
And NoneScope seems should not be added too, I'm not sure about it.

This comment has been minimized.

@wentaoxu

wentaoxu Sep 16, 2017

Contributor

session status will show all status, including global andl local, global status will show global status only.
this detail is from mysql document
so the code is like this, as same as the command "show status"

@wentaoxu

wentaoxu Sep 16, 2017

Contributor

session status will show all status, including global andl local, global status will show global status only.
this detail is from mysql document
so the code is like this, as same as the command "show status"

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Sep 16, 2017

Member

LGTM

Member

coocood commented Sep 16, 2017

LGTM

Show outdated Hide outdated table/table.go
@@ -0,0 +1,169 @@
// Copyright 2017 PingCAP, Inc.

This comment has been minimized.

@jackysp

jackysp Sep 18, 2017

Member

Do we need a test file for virtual_tables?

@jackysp

jackysp Sep 18, 2017

Member

Do we need a test file for virtual_tables?

This comment has been minimized.

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

I think we should to add a mysql test case for this file instead of adding a test file, It is more light.

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

I think we should to add a mysql test case for this file instead of adding a test file, It is more light.

Show outdated Hide outdated perfschema/init.go
package perfschema
import (
"fmt"

This comment has been minimized.

@lamxTyler

lamxTyler Sep 18, 2017

Member

Add a blank line between system packages and custom packages like other files do.

@lamxTyler

lamxTyler Sep 18, 2017

Member

Add a blank line between system packages and custom packages like other files do.

This comment has been minimized.

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

ok

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

ok

Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
Show outdated Hide outdated perfschema/virtual_tables.go
@@ -27,6 +27,18 @@ import (
"github.com/pingcap/tidb/util/types"
)
// Type , the type of table, store data in different ways.

This comment has been minimized.

@lamxTyler

lamxTyler Sep 18, 2017

Member

Why add a space between Type and ,?

@lamxTyler

lamxTyler Sep 18, 2017

Member

Why add a space between Type and ,?

This comment has been minimized.

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

this is the rule of golint, otherwise it will report like this, "comment on exported type Type should be of the form "Type ..." (with optional leading article)"

@wentaoxu

wentaoxu Sep 18, 2017

Contributor

this is the rule of golint, otherwise it will report like this, "comment on exported type Type should be of the form "Type ..." (with optional leading article)"

@wentaoxu

This comment has been minimized.

Show comment
Hide comment
@wentaoxu
Contributor

wentaoxu commented Sep 18, 2017

@ngaut

This comment has been minimized.

Show comment
Hide comment
@ngaut

ngaut Sep 18, 2017

Member

Nice refactor.

Member

ngaut commented Sep 18, 2017

Nice refactor.

@jackysp

LGTM

@jackysp

This comment has been minimized.

Show comment
Hide comment
@jackysp

jackysp Sep 18, 2017

Member

/run-all-test

Member

jackysp commented Sep 18, 2017

/run-all-test

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Sep 18, 2017

Member

/run-common-test

Member

coocood commented Sep 18, 2017

/run-common-test

@coocood coocood merged commit 339c93e into master Sep 18, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@coocood coocood deleted the xwt/add_session_status_for_performance_schema branch Sep 18, 2017

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