From a10da20024f42ef5ce0b580e5834df4a1c620304 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Mon, 28 Oct 2024 11:18:15 +0800 Subject: [PATCH 1/8] skip empty tidb nodes Signed-off-by: Jianjun Liao --- pkg/store/copr/coprocessor.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/store/copr/coprocessor.go b/pkg/store/copr/coprocessor.go index f3dd58c911ce5..d6a3e3fbfcd53 100644 --- a/pkg/store/copr/coprocessor.go +++ b/pkg/store/copr/coprocessor.go @@ -49,6 +49,7 @@ import ( "github.com/pingcap/tidb/pkg/store/driver/options" util2 "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/execdetails" + "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "github.com/pingcap/tidb/pkg/util/memory" "github.com/pingcap/tidb/pkg/util/paging" @@ -589,6 +590,9 @@ func buildTiDBMemCopTasks(ranges *KeyRanges, req *kv.Request) ([]*copTask, error if req.TiDBServerID > 0 && req.TiDBServerID != ser.ServerIDGetter() { continue } + if len(ser.IP) == 0 && !intest.InTest { + continue + } addr := net.JoinHostPort(ser.IP, strconv.FormatUint(uint64(ser.StatusPort), 10)) tasks = append(tasks, &copTask{ From 52caea9767eb6fc091f06bb299121b5e5ad7dee4 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Wed, 30 Oct 2024 18:30:53 +0800 Subject: [PATCH 2/8] add unit test Signed-off-by: Jianjun Liao --- pkg/executor/infoschema_cluster_table_test.go | 9 +++++++++ pkg/store/copr/coprocessor.go | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/executor/infoschema_cluster_table_test.go b/pkg/executor/infoschema_cluster_table_test.go index 9e25309369d67..aa7592a0af100 100644 --- a/pkg/executor/infoschema_cluster_table_test.go +++ b/pkg/executor/infoschema_cluster_table_test.go @@ -208,6 +208,15 @@ func (s *mockStore) StartGCWorker() error { panic("not implemented") } func (s *mockStore) Name() string { return "mockStore" } func (s *mockStore) Describe() string { return "" } +func TestSkipEmptyIPNodesForTiDBTypeCoprocessor(t *testing.T) { + store, _ := testkit.CreateMockStoreAndDomain(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + rows := tk.MustQuery("select * from information_schema.cluster_slow_query").Rows() + // the TiDB node is skipped because it does not has IP + require.Equal(t, 0, len(rows)) +} + func TestTiDBClusterInfo(t *testing.T) { s := createInfosSchemaClusterTableSuite(t) diff --git a/pkg/store/copr/coprocessor.go b/pkg/store/copr/coprocessor.go index d6a3e3fbfcd53..0b58b42b0b64d 100644 --- a/pkg/store/copr/coprocessor.go +++ b/pkg/store/copr/coprocessor.go @@ -49,7 +49,6 @@ import ( "github.com/pingcap/tidb/pkg/store/driver/options" util2 "github.com/pingcap/tidb/pkg/util" "github.com/pingcap/tidb/pkg/util/execdetails" - "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "github.com/pingcap/tidb/pkg/util/memory" "github.com/pingcap/tidb/pkg/util/paging" @@ -590,7 +589,7 @@ func buildTiDBMemCopTasks(ranges *KeyRanges, req *kv.Request) ([]*copTask, error if req.TiDBServerID > 0 && req.TiDBServerID != ser.ServerIDGetter() { continue } - if len(ser.IP) == 0 && !intest.InTest { + if len(ser.IP) == 0 { continue } From f532923d345314a2cfbd642d6145ddee3bc33466 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Thu, 31 Oct 2024 18:00:02 +0800 Subject: [PATCH 3/8] fix unit test Signed-off-by: Jianjun Liao --- tests/realtikvtest/sessiontest/session_fail_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/realtikvtest/sessiontest/session_fail_test.go b/tests/realtikvtest/sessiontest/session_fail_test.go index b4220ce56a2ff..3a172f6a63b8a 100644 --- a/tests/realtikvtest/sessiontest/session_fail_test.go +++ b/tests/realtikvtest/sessiontest/session_fail_test.go @@ -127,6 +127,13 @@ func TestKillFlagInBackoff(t *testing.T) { } func TestClusterTableSendError(t *testing.T) { + gconf := config.GetGlobalConfig() + originIP := gconf.AdvertiseAddress + defer func() { + gconf.AdvertiseAddress = originIP + }() + // Set an unavailable IP to prevent the coprocessor task from being skipped + gconf.AdvertiseAddress = "" store := realtikvtest.CreateMockStoreAndSetup(t) tk := testkit.NewTestKit(t, store) From 4dcc86f47a511ba49c55cb74821b4837acce4179 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Mon, 4 Nov 2024 12:24:58 +0800 Subject: [PATCH 4/8] use unavailable ip to skip coprocessor tasks Signed-off-by: Jianjun Liao --- br/cmd/br/backup.go | 9 +++++++-- br/cmd/br/restore.go | 9 +++++++-- pkg/config/config.go | 1 + pkg/executor/infoschema_cluster_table_test.go | 3 +++ pkg/store/copr/coprocessor.go | 3 ++- pkg/util/intest/no_assert.go | 2 +- pkg/util/intest/not_in_unittest.go | 2 +- 7 files changed, 22 insertions(+), 7 deletions(-) diff --git a/br/cmd/br/backup.go b/br/cmd/br/backup.go index 925cf58c132b4..d8b313ce504f3 100644 --- a/br/cmd/br/backup.go +++ b/br/cmd/br/backup.go @@ -46,8 +46,13 @@ func runBackupCommand(command *cobra.Command, cmdName string) error { return nil } - // No need to cache the coproceesor result - config.GetGlobalConfig().TiKVClient.CoprCache.CapacityMB = 0 + config.UpdateGlobal(func(conf *config.Config) { + // Need to be skipped when the cluster has TiDB type coprocessor tasks + conf.AdvertiseAddress = config.UnavailableIP + + // No need to cache the coproceesor result + conf.TiKVClient.CoprCache.CapacityMB = 0 + }) // Disable the memory limit tuner. That's because the server memory is get from TiDB node instead of BR node. gctuner.GlobalMemoryLimitTuner.DisableAdjustMemoryLimit() diff --git a/br/cmd/br/restore.go b/br/cmd/br/restore.go index f991163813af2..cbf9ccff5f329 100644 --- a/br/cmd/br/restore.go +++ b/br/cmd/br/restore.go @@ -65,8 +65,13 @@ func runRestoreCommand(command *cobra.Command, cmdName string) error { return nil } - // No need to cache the coproceesor result - config.GetGlobalConfig().TiKVClient.CoprCache.CapacityMB = 0 + config.UpdateGlobal(func(conf *config.Config) { + // Need to be skipped when the cluster has TiDB type coprocessor tasks + conf.AdvertiseAddress = config.UnavailableIP + + // No need to cache the coproceesor result + conf.TiKVClient.CoprCache.CapacityMB = 0 + }) // Disable the memory limit tuner. That's because the server memory is get from TiDB node instead of BR node. gctuner.GlobalMemoryLimitTuner.DisableAdjustMemoryLimit() diff --git a/pkg/config/config.go b/pkg/config/config.go index 56f6fd66a2a5f..262fd24d43b56 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -100,6 +100,7 @@ const ( // MaxTokenLimit is the max token limit value. MaxTokenLimit = 1024 * 1024 DefSchemaLease = 45 * time.Second + UnavailableIP = "" ) // Valid config maps diff --git a/pkg/executor/infoschema_cluster_table_test.go b/pkg/executor/infoschema_cluster_table_test.go index aa7592a0af100..d0f6dfffed329 100644 --- a/pkg/executor/infoschema_cluster_table_test.go +++ b/pkg/executor/infoschema_cluster_table_test.go @@ -209,6 +209,9 @@ func (s *mockStore) Name() string { return "mockStore" } func (s *mockStore) Describe() string { return "" } func TestSkipEmptyIPNodesForTiDBTypeCoprocessor(t *testing.T) { + originIP := config.GetGlobalConfig().AdvertiseAddress + config.GetGlobalConfig().AdvertiseAddress = config.UnavailableIP + defer func() { config.GetGlobalConfig().AdvertiseAddress = originIP }() store, _ := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") diff --git a/pkg/store/copr/coprocessor.go b/pkg/store/copr/coprocessor.go index 0b58b42b0b64d..40b4e2cb2a35a 100644 --- a/pkg/store/copr/coprocessor.go +++ b/pkg/store/copr/coprocessor.go @@ -589,7 +589,8 @@ func buildTiDBMemCopTasks(ranges *KeyRanges, req *kv.Request) ([]*copTask, error if req.TiDBServerID > 0 && req.TiDBServerID != ser.ServerIDGetter() { continue } - if len(ser.IP) == 0 { + // skip some nodes, such as BR created when backup/restore + if ser.IP == config.UnavailableIP { continue } diff --git a/pkg/util/intest/no_assert.go b/pkg/util/intest/no_assert.go index 7d1d268a6ca30..1c527c1420883 100644 --- a/pkg/util/intest/no_assert.go +++ b/pkg/util/intest/no_assert.go @@ -17,7 +17,7 @@ package intest // EnableAssert checks if the code is running in integration test. -const EnableAssert = false +const EnableAssert = true // Assert is a stub function in release build. // See the same function in `util/intest/assert.go` for the real implement in test. diff --git a/pkg/util/intest/not_in_unittest.go b/pkg/util/intest/not_in_unittest.go index 0955803b56ea8..a66481980c692 100644 --- a/pkg/util/intest/not_in_unittest.go +++ b/pkg/util/intest/not_in_unittest.go @@ -17,4 +17,4 @@ package intest // InTest checks if the code is running in test. -const InTest = false +const InTest = true From fa23ecb9c2db844f0988f782066ee427faa35b13 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Mon, 4 Nov 2024 12:28:50 +0800 Subject: [PATCH 5/8] fix typos Signed-off-by: Jianjun Liao --- pkg/util/intest/assert.go | 2 +- pkg/util/intest/not_in_unittest.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/intest/assert.go b/pkg/util/intest/assert.go index 3b4e604a58ba4..8b54f4b8849d7 100644 --- a/pkg/util/intest/assert.go +++ b/pkg/util/intest/assert.go @@ -22,7 +22,7 @@ import ( ) // EnableAssert checks if the assert function should work. -const EnableAssert = true +const EnableAssert = false // Assert asserts a condition is true func Assert(cond bool, msgAndArgs ...any) { diff --git a/pkg/util/intest/not_in_unittest.go b/pkg/util/intest/not_in_unittest.go index a66481980c692..0955803b56ea8 100644 --- a/pkg/util/intest/not_in_unittest.go +++ b/pkg/util/intest/not_in_unittest.go @@ -17,4 +17,4 @@ package intest // InTest checks if the code is running in test. -const InTest = true +const InTest = false From 311bc878bad18f739d022658ebac884b0316c0c2 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Mon, 4 Nov 2024 13:31:46 +0800 Subject: [PATCH 6/8] fix typos Signed-off-by: Jianjun Liao --- pkg/util/intest/assert.go | 2 +- pkg/util/intest/no_assert.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/intest/assert.go b/pkg/util/intest/assert.go index 8b54f4b8849d7..3b4e604a58ba4 100644 --- a/pkg/util/intest/assert.go +++ b/pkg/util/intest/assert.go @@ -22,7 +22,7 @@ import ( ) // EnableAssert checks if the assert function should work. -const EnableAssert = false +const EnableAssert = true // Assert asserts a condition is true func Assert(cond bool, msgAndArgs ...any) { diff --git a/pkg/util/intest/no_assert.go b/pkg/util/intest/no_assert.go index 1c527c1420883..7d1d268a6ca30 100644 --- a/pkg/util/intest/no_assert.go +++ b/pkg/util/intest/no_assert.go @@ -17,7 +17,7 @@ package intest // EnableAssert checks if the code is running in integration test. -const EnableAssert = true +const EnableAssert = false // Assert is a stub function in release build. // See the same function in `util/intest/assert.go` for the real implement in test. From 7143727b3c43c43e92ecdb163c6f7aefc3c67894 Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Mon, 4 Nov 2024 13:34:32 +0800 Subject: [PATCH 7/8] fix unit test Signed-off-by: Jianjun Liao --- tests/realtikvtest/sessiontest/session_fail_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/realtikvtest/sessiontest/session_fail_test.go b/tests/realtikvtest/sessiontest/session_fail_test.go index 3a172f6a63b8a..b4220ce56a2ff 100644 --- a/tests/realtikvtest/sessiontest/session_fail_test.go +++ b/tests/realtikvtest/sessiontest/session_fail_test.go @@ -127,13 +127,6 @@ func TestKillFlagInBackoff(t *testing.T) { } func TestClusterTableSendError(t *testing.T) { - gconf := config.GetGlobalConfig() - originIP := gconf.AdvertiseAddress - defer func() { - gconf.AdvertiseAddress = originIP - }() - // Set an unavailable IP to prevent the coprocessor task from being skipped - gconf.AdvertiseAddress = "" store := realtikvtest.CreateMockStoreAndSetup(t) tk := testkit.NewTestKit(t, store) From 56b3b2fa9a175c91d71df728fffeb4f53488df8a Mon Sep 17 00:00:00 2001 From: Jianjun Liao Date: Tue, 5 Nov 2024 11:02:17 +0800 Subject: [PATCH 8/8] fix unit test Signed-off-by: Jianjun Liao --- pkg/executor/infoschema_cluster_table_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/executor/infoschema_cluster_table_test.go b/pkg/executor/infoschema_cluster_table_test.go index d0f6dfffed329..568a381762197 100644 --- a/pkg/executor/infoschema_cluster_table_test.go +++ b/pkg/executor/infoschema_cluster_table_test.go @@ -216,6 +216,7 @@ func TestSkipEmptyIPNodesForTiDBTypeCoprocessor(t *testing.T) { tk := testkit.NewTestKit(t, store) tk.MustExec("use test") rows := tk.MustQuery("select * from information_schema.cluster_slow_query").Rows() + require.Equal(t, tk.Session().GetSessionVars().StmtCtx.WarningCount(), uint16(0)) // the TiDB node is skipped because it does not has IP require.Equal(t, 0, len(rows)) }