Skip to content

Commit

Permalink
fix and make comment clearer
Browse files Browse the repository at this point in the history
Signed-off-by: Ryan Leung <[email protected]>
  • Loading branch information
rleungx committed Jan 13, 2025
1 parent 0af7f26 commit c8506b7
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 13 deletions.
2 changes: 1 addition & 1 deletion cmd/pd-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
3 changes: 1 addition & 2 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand Down
24 changes: 16 additions & 8 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion tests/integrations/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit c8506b7

Please sign in to comment.