From c8506b7f9c50bf9e7f47afbbbe136d8fb83aef75 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Mon, 13 Jan 2025 16:14:48 +0800 Subject: [PATCH] fix and make comment clearer Signed-off-by: Ryan Leung --- cmd/pd-server/main.go | 2 +- server/config/config.go | 3 +-- server/server.go | 24 ++++++++++++++++-------- server/server_test.go | 1 - tests/integrations/client/client_test.go | 4 +++- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cmd/pd-server/main.go b/cmd/pd-server/main.go index 7b7fd8f04f2..b85ff70d728 100644 --- a/cmd/pd-server/main.go +++ b/cmd/pd-server/main.go @@ -52,7 +52,7 @@ import ( ) const ( - // Only serverless use this variable, we can use it to determine whether the PD is running in keyspace mode. + // Only serverless use this variable, we can use it to determine whether the PD supports multiple timelines. // The name is a little misleading, but it's kept for backward compatibility. serviceModeEnv = "PD_SERVICE_MODE" ) diff --git a/server/config/config.go b/server/config/config.go index cfca0430cd4..f7e96a74e35 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -249,7 +249,7 @@ const ( maxCheckRegionSplitInterval = 100 * time.Millisecond defaultEnableSchedulingFallback = true - // In serverless environment, the default value of `enable-scheduling` is always false. + // In serverless environment, the default value of `enable-tso-dynamic-switching` is always false. defaultEnableTSODynamicSwitching = false ) @@ -856,7 +856,6 @@ func (c *MicroserviceConfig) IsTSODynamicSwitchingEnabled() bool { } // IsMultiTimelinesEnabled returns whether to enable multi-timelines. -// for testing purpose. // TODO: use it to replace system variable. func (c *MicroserviceConfig) IsMultiTimelinesEnabled() bool { return c.EnableMultiTimelines diff --git a/server/server.go b/server/server.go index 27e3aeecb5a..f3f26cd6eb2 100644 --- a/server/server.go +++ b/server/server.go @@ -238,14 +238,22 @@ type HandlerBuilder func(context.Context, *Server) (http.Handler, apiutil.APISer // CreateServer creates the UNINITIALIZED pd server with given configuration. func CreateServer(ctx context.Context, cfg *config.Config, legacyServiceBuilders ...HandlerBuilder) (*Server, error) { - // TODO: Currently, whether we enable microservice or not is determined by the service list. - // It's equal to whether we enable the keyspace group or not. - // There could be the following scenarios: - // 1. Enable microservice but disable keyspace group. (non-serverless scenario) - // 2. Enable microservice and enable keyspace group. (serverless scenario) - // 3. Disable microservice and disable keyspace group. (both serverless scenario and non-serverless scenario) - // But for case 1, we enable keyspace group which is misleading because non-serverless don't have keyspace related concept. - // The keyspace group should be independent of the microservice. + // TODO: Currently, we have following combinations: + // + // There could be the following scenarios for non-serverless: + // 1. microservice + single timelines + // 2. non-microservice + // we use `enable-tso-dynamic-switch` to control whether we enable microservice or not. + // non-serverless doesn't support multiple timelines but support dynamic switch. + // + // There could be the following scenarios for serverless: + // 1. microservice + single timelines + // 2. microservice + multiple timelines + // 3. non-microservice + // we use `enable-multi-timelines` to control whether we enable microservice or not. + // serverless supports multiple timelines but doesn't support dynamic switch. + // + // Besides, the current implementation for both serverless and non-serverless rely on keyspace group which should be independent of the microservice. // We should separate the keyspace group from the microservice later. log.Info("PD config", zap.Reflect("config", cfg)) serviceMiddlewareCfg := config.NewServiceMiddlewareConfig() diff --git a/server/server_test.go b/server/server_test.go index e1e53b50d17..4325a23a764 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -201,7 +201,6 @@ func (suite *leaderServerTestSuite) TestSourceIpForHeaderXReal() { ctx, cancel := context.WithCancel(context.Background()) svr, err := CreateServer(ctx, cfg, mockHandler) re.NoError(err) - re.Error(err) defer func() { cancel() svr.Close() diff --git a/tests/integrations/client/client_test.go b/tests/integrations/client/client_test.go index 3c0b9f366fa..05bd202da87 100644 --- a/tests/integrations/client/client_test.go +++ b/tests/integrations/client/client_test.go @@ -361,7 +361,9 @@ func TestTSOFollowerProxyWithTSOService(t *testing.T) { re.NoError(failpoint.Enable("github.com/tikv/pd/client/servicediscovery/fastUpdateServiceMode", `return(true)`)) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - cluster, err := tests.NewTestCluster(ctx, 1) + cluster, err := tests.NewTestCluster(ctx, 1, func(conf *config.Config, _ string) { + conf.Microservice.EnableMultiTimelines = true + }) re.NoError(err) defer cluster.Destroy() err = cluster.RunInitialServers()