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.

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

*: remove cluster dependency from keyspace #6249

Merged
merged 3 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 25 additions & 17 deletions server/keyspace/keyspace.go → pkg/keyspace/keyspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import (
"github.com/pingcap/kvproto/pkg/keyspacepb"
"github.com/pingcap/log"
"github.com/tikv/pd/pkg/id"
"github.com/tikv/pd/pkg/schedule"
"github.com/tikv/pd/pkg/schedule/labeler"
"github.com/tikv/pd/pkg/slice"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/pkg/storage/kv"
"github.com/tikv/pd/pkg/utils/syncutil"
"github.com/tikv/pd/server/cluster"
"github.com/tikv/pd/server/config"
"go.uber.org/zap"
)

Expand All @@ -48,6 +48,11 @@ const (
regionLabelKey = "id"
)

// Config is the interface for keyspace config.
type Config interface {
GetPreAlloc() []string
}

// Manager manages keyspace related data.
// It validates requests and provides concurrency control.
type Manager struct {
Expand All @@ -58,11 +63,11 @@ type Manager struct {
// store is the storage for keyspace related information.
store endpoint.KeyspaceStorage
// rc is the raft cluster of the server.
rc *cluster.RaftCluster
cluster schedule.Cluster
// ctx is the context of the manager, to be used in transaction.
ctx context.Context
// config is the configurations of the manager.
config config.KeyspaceConfig
config Config
}

// CreateKeyspaceRequest represents necessary arguments to create a keyspace.
Expand All @@ -77,15 +82,15 @@ type CreateKeyspaceRequest struct {

// NewKeyspaceManager creates a Manager of keyspace related data.
func NewKeyspaceManager(store endpoint.KeyspaceStorage,
rc *cluster.RaftCluster,
cluster schedule.Cluster,
idAllocator id.Allocator,
config config.KeyspaceConfig,
config Config,
) *Manager {
return &Manager{
metaLock: syncutil.NewLockGroup(syncutil.WithHash(keyspaceIDHash)),
idAllocator: idAllocator,
store: store,
rc: rc,
cluster: cluster,
ctx: context.TODO(),
config: config,
}
Expand Down Expand Up @@ -113,7 +118,7 @@ func (manager *Manager) Bootstrap() error {
}

// Initialize pre-alloc keyspace.
preAlloc := manager.config.PreAlloc
preAlloc := manager.config.GetPreAlloc()
for _, keyspaceName := range preAlloc {
_, err = manager.CreateKeyspace(&CreateKeyspaceRequest{
Name: keyspaceName,
Expand Down Expand Up @@ -207,18 +212,21 @@ func (manager *Manager) splitKeyspaceRegion(id uint32) error {
})

keyspaceRule := makeLabelRule(id)
err := manager.rc.GetRegionLabeler().SetLabelRule(keyspaceRule)
if err != nil {
log.Warn("[keyspace] failed to add region label for keyspace",
if cl, ok := manager.cluster.(interface{ GetRegionLabeler() *labeler.RegionLabeler }); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

why add this judgment?

Copy link
Member Author

Choose a reason for hiding this comment

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

remove cluster dependency

err := cl.GetRegionLabeler().SetLabelRule(keyspaceRule)
if err != nil {
log.Warn("[keyspace] failed to add region label for keyspace",
zap.Uint32("keyspaceID", id),
zap.Error(err),
)
}
log.Info("[keyspace] added region label for keyspace",
zap.Uint32("keyspaceID", id),
zap.Error(err),
zap.Any("LabelRule", keyspaceRule),
)
return nil
}
log.Info("[keyspace] added region label for keyspace",
zap.Uint32("keyspaceID", id),
zap.Any("LabelRule", keyspaceRule),
)
return nil
return errors.New("cluster does not support region label")
}

// LoadKeyspace returns the keyspace specified by name.
Expand Down
14 changes: 10 additions & 4 deletions server/keyspace/keyspace_test.go → pkg/keyspace/keyspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/tikv/pd/pkg/mock/mockid"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/pkg/storage/kv"
"github.com/tikv/pd/server/config"
)

const (
Expand All @@ -47,18 +46,25 @@ func TestKeyspaceTestSuite(t *testing.T) {
suite.Run(t, new(keyspaceTestSuite))
}

type mockConfig struct {
PreAlloc []string
}

func (m *mockConfig) GetPreAlloc() []string { return m.PreAlloc }

func (suite *keyspaceTestSuite) SetupTest() {
store := endpoint.NewStorageEndpoint(kv.NewMemoryKV(), nil)
allocator := mockid.NewIDAllocator()
suite.manager = NewKeyspaceManager(store, nil, allocator, config.KeyspaceConfig{})
suite.manager = NewKeyspaceManager(store, nil, allocator, &mockConfig{})
suite.NoError(suite.manager.Bootstrap())
}

func (suite *keyspaceTestSuite) SetupSuite() {
suite.NoError(failpoint.Enable("github.com/tikv/pd/server/keyspace/skipSplitRegion", "return(true)"))
suite.NoError(failpoint.Enable("github.com/tikv/pd/pkg/keyspace/skipSplitRegion", "return(true)"))
}

func (suite *keyspaceTestSuite) TearDownSuite() {
suite.NoError(failpoint.Disable("github.com/tikv/pd/server/keyspace/skipSplitRegion"))
suite.NoError(failpoint.Disable("github.com/tikv/pd/pkg/keyspace/skipSplitRegion"))
}

func makeCreateKeyspaceRequests(count int) []*CreateKeyspaceRequest {
Expand Down
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions server/api/region_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ func (suite *regionLabelTestSuite) SetupSuite() {

addr := suite.svr.GetAddr()
suite.urlPrefix = fmt.Sprintf("%s%s/api/v1/config/region-label/", addr, apiPrefix)
suite.NoError(failpoint.Enable("github.com/tikv/pd/server/keyspace/skipSplitRegion", "return(true)"))
suite.NoError(failpoint.Enable("github.com/tikv/pd/pkg/keyspace/skipSplitRegion", "return(true)"))
mustBootstrapCluster(re, suite.svr)
}

func (suite *regionLabelTestSuite) TearDownSuite() {
suite.cleanup()
suite.NoError(failpoint.Disable("github.com/tikv/pd/server/keyspace/skipSplitRegion"))
suite.NoError(failpoint.Disable("github.com/tikv/pd/pkg/keyspace/skipSplitRegion"))
}

func (suite *regionLabelTestSuite) TestGetSet() {
Expand Down
2 changes: 1 addition & 1 deletion server/apiv2/handlers/keyspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import (
"github.com/pingcap/errors"
"github.com/pingcap/kvproto/pkg/keyspacepb"
"github.com/tikv/pd/pkg/errs"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/server"
"github.com/tikv/pd/server/apiv2/middlewares"
"github.com/tikv/pd/server/keyspace"
)

// RegisterKeyspace register keyspace related handlers to router paths.
Expand Down
5 changes: 5 additions & 0 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1389,3 +1389,8 @@ type KeyspaceConfig struct {
// PreAlloc contains the keyspace to be allocated during keyspace manager initialization.
PreAlloc []string `toml:"pre-alloc" json:"pre-alloc"`
}

// GetPreAlloc returns the keyspace to be allocated during keyspace manager initialization.
func (c *KeyspaceConfig) GetPreAlloc() []string {
return c.PreAlloc
}
2 changes: 1 addition & 1 deletion server/keyspace_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (
"github.com/gogo/protobuf/proto"
"github.com/pingcap/kvproto/pkg/keyspacepb"
"github.com/pingcap/kvproto/pkg/pdpb"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/server/keyspace"
"go.etcd.io/etcd/clientv3"
)

Expand Down
12 changes: 6 additions & 6 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
"github.com/tikv/pd/pkg/encryption"
"github.com/tikv/pd/pkg/errs"
"github.com/tikv/pd/pkg/id"
"github.com/tikv/pd/pkg/keyspace"
ms_server "github.com/tikv/pd/pkg/mcs/meta_storage/server"
"github.com/tikv/pd/pkg/mcs/registry"
rm_server "github.com/tikv/pd/pkg/mcs/resource_manager/server"
Expand Down Expand Up @@ -73,7 +74,6 @@ import (
"github.com/tikv/pd/server/cluster"
"github.com/tikv/pd/server/config"
"github.com/tikv/pd/server/gc"
"github.com/tikv/pd/server/keyspace"
syncer "github.com/tikv/pd/server/region_syncer"
"go.etcd.io/etcd/clientv3"
"go.etcd.io/etcd/embed"
Expand Down Expand Up @@ -440,7 +440,7 @@ func (s *Server) startServer(ctx context.Context) error {
Member: s.member.MemberValue(),
Step: keyspace.AllocStep,
})
s.keyspaceManager = keyspace.NewKeyspaceManager(s.storage, s.cluster, keyspaceIDAllocator, s.cfg.Keyspace)
s.keyspaceManager = keyspace.NewKeyspaceManager(s.storage, s.cluster, keyspaceIDAllocator, &s.cfg.Keyspace)
s.keyspaceGroupManager = keyspace.NewKeyspaceGroupManager(s.ctx, s.storage)
s.hbStreams = hbstream.NewHeartbeatStreams(ctx, s.clusterID, s.cluster)
// initial hot_region_storage in here.
Expand Down Expand Up @@ -698,12 +698,12 @@ func (s *Server) bootstrapCluster(req *pdpb.BootstrapRequest) (*pdpb.BootstrapRe
return nil, err
}

if err = s.GetKeyspaceManager().Bootstrap(); err != nil {
log.Warn("bootstrap keyspace manager failed", errs.ZapError(err))
if err := s.GetKeyspaceGroupManager().Bootstrap(); err != nil {
log.Warn("bootstrapping keyspace group manager failed", errs.ZapError(err))
}

if err = s.GetKeyspaceGroupManager().Bootstrap(); err != nil {
log.Warn("bootstrap keyspace group manager failed", errs.ZapError(err))
if err = s.GetKeyspaceManager().Bootstrap(); err != nil {
log.Warn("bootstrapping keyspace manager failed", errs.ZapError(err))
}

return &pdpb.BootstrapResponse{
Expand Down
2 changes: 1 addition & 1 deletion tests/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/tikv/pd/pkg/dashboard"
"github.com/tikv/pd/pkg/errs"
"github.com/tikv/pd/pkg/id"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/pkg/mcs/utils"
"github.com/tikv/pd/pkg/schedule/schedulers"
"github.com/tikv/pd/pkg/swaggerserver"
Expand All @@ -44,7 +45,6 @@ import (
"github.com/tikv/pd/server/cluster"
"github.com/tikv/pd/server/config"
"github.com/tikv/pd/server/join"
"github.com/tikv/pd/server/keyspace"
"go.etcd.io/etcd/clientv3"
)

Expand Down
2 changes: 1 addition & 1 deletion tests/integrations/client/keyspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import (

"github.com/pingcap/kvproto/pkg/keyspacepb"
"github.com/stretchr/testify/require"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/pkg/slice"
"github.com/tikv/pd/server"
"github.com/tikv/pd/server/keyspace"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion tests/server/apiv2/handlers/keyspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import (
"github.com/pingcap/kvproto/pkg/keyspacepb"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/pkg/utils/testutil"
"github.com/tikv/pd/server/apiv2/handlers"
"github.com/tikv/pd/server/keyspace"
"github.com/tikv/pd/tests"
"go.uber.org/goleak"
)
Expand Down
2 changes: 1 addition & 1 deletion tests/server/keyspace/keyspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/tikv/pd/pkg/codec"
"github.com/tikv/pd/pkg/keyspace"
"github.com/tikv/pd/pkg/schedule/labeler"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/server/config"
"github.com/tikv/pd/server/keyspace"
"github.com/tikv/pd/tests"
)

Expand Down