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 a9d441c
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 18 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
4 changes: 3 additions & 1 deletion tests/integrations/mcs/tso/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ func (suite *tsoAPITestSuite) SetupTest() {

var err error
suite.ctx, suite.cancel = context.WithCancel(context.Background())
suite.pdCluster, err = tests.NewTestCluster(suite.ctx, 1)
suite.pdCluster, err = tests.NewTestCluster(suite.ctx, 1, func(conf *config.Config, _ string) {
conf.Microservice.EnableMultiTimelines = true
})
re.NoError(err)
err = suite.pdCluster.RunInitialServers()
re.NoError(err)
Expand Down
15 changes: 11 additions & 4 deletions tests/integrations/mcs/tso/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,18 @@ type pdForward struct {
pdClient pd.Client
}

func NewPDForward(re *require.Assertions) pdForward {
func NewPDForward(re *require.Assertions, enableMultiTimelines ...bool) pdForward {
suite := pdForward{
re: re,
}
isMultiTimelinesEnabled := true
if len(enableMultiTimelines) > 0 {
isMultiTimelinesEnabled = enableMultiTimelines[0]
}
var err error
suite.ctx, suite.cancel = context.WithCancel(context.Background())
suite.cluster, err = tests.NewTestCluster(suite.ctx, 3, func(conf *config.Config, _ string) {
conf.Microservice.EnableMultiTimelines = true
conf.Microservice.EnableMultiTimelines = isMultiTimelinesEnabled
})
re.NoError(err)

Expand Down Expand Up @@ -273,9 +277,11 @@ func (suite *pdForward) ShutDown() {
re.NoError(failpoint.Disable("github.com/tikv/pd/client/servicediscovery/usePDServiceMode"))
}

// TestForwardTSO tests the behavior of forwarding TSO requests to the TSO server in non-serverless env.
func TestForwardTSO(t *testing.T) {
re := require.New(t)
suite := NewPDForward(re)
// non-serverless env should disable multi-timelines
suite := NewPDForward(re, false)
defer suite.ShutDown()
// If EnableTSODynamicSwitching is false, the tso server will be provided by PD.
// The tso server won't affect the PD.
Expand All @@ -294,7 +300,8 @@ func TestForwardTSO(t *testing.T) {
suite.checkAvailableTSO(re)
}

func TestForwardTSOWithKeyspaceGroup(t *testing.T) {
// TestForwardTSOWithMultipleTimelines tests the behavior of forwarding TSO requests to the TSO server in serverless env.
func TestForwardTSOWithMultipleTimelines(t *testing.T) {
re := require.New(t)
suite := NewPDForward(re)
defer suite.ShutDown()
Expand Down

0 comments on commit a9d441c

Please sign in to comment.