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

topsql: deprecate single target #31214

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+33 −32
Open

topsql: deprecate single target #31214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03ca9ef
topsql: deprecate single target
zhongzc Dec 31, 2021
35d74ec
Merge branch 'master' into topsql-deprecate-single-target
zhongzc Dec 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ type Config struct {
DelayCleanTableLock uint64 `toml:"delay-clean-table-lock" json:"delay-clean-table-lock"`
SplitRegionMaxNum uint64 `toml:"split-region-max-num" json:"split-region-max-num"`
StmtSummary StmtSummary `toml:"stmt-summary" json:"stmt-summary"`
TopSQL TopSQL `toml:"top-sql" json:"top-sql"`
// RepairMode indicates that the TiDB is in the repair mode for table meta.
RepairMode bool `toml:"repair-mode" json:"repair-mode"`
RepairTableList []string `toml:"repair-table-list" json:"repair-table-list"`
Expand Down Expand Up @@ -605,12 +604,6 @@ type StmtSummary struct {
HistorySize int `toml:"history-size" json:"history-size"`
}

// TopSQL is the config for TopSQL.
type TopSQL struct {
// The TopSQL's data receiver address.
ReceiverAddress string `toml:"receiver-address" json:"receiver-address"`
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this will cause deserialization problems, e.g. an outdated config result in fail to start?

}

// IsolationRead is the config for isolation read.
type IsolationRead struct {
// Engines filters tidb-server access paths by engine type.
Expand Down
3 changes: 0 additions & 3 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ spilled-file-encryption-method = "plaintext"
[pessimistic-txn]
deadlock-history-capacity = 123
deadlock-history-collect-retryable = true
[top-sql]
receiver-address = "127.0.0.1:10100"
[status]
grpc-keepalive-time = 20
grpc-keepalive-timeout = 10
Expand Down Expand Up @@ -322,7 +320,6 @@ grpc-max-send-msg-size = 40960
require.Equal(t, uint(123), conf.PessimisticTxn.DeadlockHistoryCapacity)
require.True(t, conf.PessimisticTxn.DeadlockHistoryCollectRetryable)
require.False(t, conf.Experimental.EnableNewCharset)
require.Equal(t, "127.0.0.1:10100", conf.TopSQL.ReceiverAddress)
require.True(t, conf.Experimental.AllowsExpressionIndex)
require.Equal(t, uint(20), conf.Status.GRPCKeepAliveTime)
require.Equal(t, uint(10), conf.Status.GRPCKeepAliveTimeout)
Expand Down
4 changes: 1 addition & 3 deletions executor/executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8766,9 +8766,7 @@ func (s *testResourceTagSuite) TestResourceGroupTag(c *C) {

// Enable Top SQL
topsqlstate.GlobalState.Enable.Store(true)
config.UpdateGlobal(func(conf *config.Config) {
conf.TopSQL.ReceiverAddress = "mock-agent"
})
topsqlstate.GlobalState.ReceiverAddress.Store("mock-agent")

c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/unistore/unistoreRPCClientSendHook", `return(true)`), IsNil)
defer failpoint.Disable("github.com/pingcap/tidb/store/mockstore/unistore/unistoreRPCClientSendHook")
Expand Down
10 changes: 3 additions & 7 deletions server/tidb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"github.com/go-sql-driver/mysql"
"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/parser"
Expand Down Expand Up @@ -1300,9 +1299,8 @@ func TestTopSQLCPUProfile(t *testing.T) {
dbt.MustExec("create table t1 (a int auto_increment, b int, unique index idx(a));")
dbt.MustExec("create table t2 (a int auto_increment, b int, unique index idx(a));")
dbt.MustExec("set @@global.tidb_enable_top_sql='On';")
config.UpdateGlobal(func(conf *config.Config) {
conf.TopSQL.ReceiverAddress = "127.0.0.1:4001"
})
topsqlstate.GlobalState.ReceiverAddress.Store("127.0.0.1:4001")

dbt.MustExec("set @@global.tidb_top_sql_precision_seconds=1;")
dbt.MustExec("set @@global.tidb_txn_mode = 'pessimistic'")

Expand Down Expand Up @@ -1545,9 +1543,7 @@ func TestTopSQLStatementStats(t *testing.T) {

// Enable TopSQL
topsqlstate.GlobalState.Enable.Store(true)
config.UpdateGlobal(func(conf *config.Config) {
conf.TopSQL.ReceiverAddress = "mock-agent"
})
topsqlstate.GlobalState.ReceiverAddress.Store("mock-agent")
dbt.MustExec("set @@global.tidb_enable_top_sql='On';")

const ExecCountPerSQL = 3
Expand Down
5 changes: 1 addition & 4 deletions util/topsql/reporter/reporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"testing"
"time"

"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/util/topsql/collector"
"github.com/pingcap/tidb/util/topsql/reporter/mock"
topsqlstate "github.com/pingcap/tidb/util/topsql/state"
Expand Down Expand Up @@ -86,9 +85,7 @@ func setupRemoteTopSQLReporter(maxStatementsNum, interval int, addr string) (*Re
topsqlstate.GlobalState.MaxStatementCount.Store(int64(maxStatementsNum))
topsqlstate.GlobalState.MaxCollect.Store(10000)
topsqlstate.GlobalState.ReportIntervalSeconds.Store(int64(interval))
config.UpdateGlobal(func(conf *config.Config) {
conf.TopSQL.ReceiverAddress = addr
})
topsqlstate.GlobalState.ReceiverAddress.Store(addr)

topsqlstate.EnableTopSQL()
ts := NewRemoteTopSQLReporter(mockPlanBinaryDecoderFunc)
Expand Down
8 changes: 4 additions & 4 deletions util/topsql/reporter/single_target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
"sync"
"time"

"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/topsql/state"
"github.com/pingcap/tipb/go-tipb"
"go.uber.org/atomic"
"go.uber.org/zap"
Expand Down Expand Up @@ -68,7 +68,7 @@ func NewSingleTargetDataSink(registerer DataSinkRegisterer) *SingleTargetDataSin

// Start starts to run SingleTargetDataSink.
func (ds *SingleTargetDataSink) Start() {
addr := config.GetGlobalConfig().TopSQL.ReceiverAddress
addr := state.GlobalState.ReceiverAddress.Load()
if addr != "" {
ds.curRPCAddr = addr
err := ds.registerer.Register(ds)
Expand Down Expand Up @@ -117,10 +117,10 @@ func (ds *SingleTargetDataSink) run() (rerun bool) {
case <-ds.ctx.Done():
return false
case task := <-ds.sendTaskCh:
targetRPCAddr = config.GetGlobalConfig().TopSQL.ReceiverAddress
targetRPCAddr = state.GlobalState.ReceiverAddress.Load()
ds.doSend(targetRPCAddr, task)
case <-ticker.C:
targetRPCAddr = config.GetGlobalConfig().TopSQL.ReceiverAddress
targetRPCAddr = state.GlobalState.ReceiverAddress.Load()
}

if err := ds.trySwitchRegistration(targetRPCAddr); err != nil {
Expand Down
23 changes: 23 additions & 0 deletions util/topsql/state/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ const (
DefTiDBTopSQLMaxStatementCount = 200
DefTiDBTopSQLMaxCollect = 5000
DefTiDBTopSQLReportIntervalSeconds = 60

// DefTiDBTopSQLReceiverAddress is temporary for testing
//
// Deprecated
//
// TiDB has deprecated the single target reporting mode. However, since TopSQL tests rely heavily
// on single target, this global variable is introduced to make the tests work properly before
// refactoring them. This variable will be removed later after refactoring is totally done.
DefTiDBTopSQLReceiverAddress = ""
)

// GlobalState is the global Top-SQL state.
Expand All @@ -32,6 +41,13 @@ var GlobalState = State{
MaxStatementCount: atomic.NewInt64(DefTiDBTopSQLMaxStatementCount),
MaxCollect: atomic.NewInt64(DefTiDBTopSQLMaxCollect),
ReportIntervalSeconds: atomic.NewInt64(DefTiDBTopSQLReportIntervalSeconds),

// Deprecated
//
// TiDB has deprecated the single target reporting mode. However, since TopSQL tests rely heavily
// on single target, this global variable is introduced to make the tests work properly before
// refactoring them. This variable will be removed later after refactoring is totally done.
ReceiverAddress: atomic.NewString(DefTiDBTopSQLReceiverAddress),
}

// State is the state for control top sql feature.
Expand All @@ -46,6 +62,13 @@ type State struct {
MaxCollect *atomic.Int64
// The report data interval of top-sql.
ReportIntervalSeconds *atomic.Int64

// Deprecated
//
// TiDB has deprecated the single target reporting mode. However, since TopSQL tests rely heavily
// on single target, this global variable is introduced to make the tests work properly before
// refactoring them. This variable will be removed later after refactoring is totally done.
ReceiverAddress *atomic.String
}

// EnableTopSQL enables the top SQL feature.
Expand Down
5 changes: 1 addition & 4 deletions util/topsql/topsql_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"testing"
"time"

"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/parser"
"github.com/pingcap/tidb/util/cpuprofile"
"github.com/pingcap/tidb/util/topsql"
Expand Down Expand Up @@ -92,9 +91,7 @@ func TestTopSQLReporter(t *testing.T) {
require.NoError(t, err)
topsqlstate.GlobalState.MaxStatementCount.Store(200)
topsqlstate.GlobalState.ReportIntervalSeconds.Store(1)
config.UpdateGlobal(func(conf *config.Config) {
conf.TopSQL.ReceiverAddress = server.Address()
})
topsqlstate.GlobalState.ReceiverAddress.Store(server.Address())

topsqlstate.EnableTopSQL()
report := reporter.NewRemoteTopSQLReporter(mockPlanBinaryDecoderFunc)
Expand Down