Skip to content

Commit 28bb6b0

Browse files
bufferfliesbufferflies
committed
keyspace: address meta service group review comments
Signed-off-by: bufferflies <1045931706@qq.com>
1 parent ae453a6 commit 28bb6b0

File tree

8 files changed

+130
-42
lines changed

8 files changed

+130
-42
lines changed

‎pkg/keyspace/keyspace.go‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,13 @@ func (manager *Manager) CreateKeyspace(request *CreateKeyspaceRequest) (*keyspac
271271
tracer.SetKeyspace(newID, request.Name)
272272
tracer.OnAllocateIDFinished()
273273

274+
if request.Config != nil {
275+
delete(request.Config, MetaServiceGroupAddressesKey)
276+
}
274277
// assign meta-service group for the new keyspace if meta-service groups exist.
275278
assignToMetaServiceGroup := manager.mgm != nil && len(manager.mgm.GetGroups()) > 0
276279
if assignToMetaServiceGroup {
277-
metaServiceGroup, err := manager.mgm.AssignToGroup(1)
280+
metaServiceGroup, err := manager.mgm.SelectGroup()
278281
if err != nil {
279282
return nil, err
280283
}
@@ -340,7 +343,13 @@ func (manager *Manager) CreateKeyspace(request *CreateKeyspaceRequest) (*keyspac
340343
if e != nil {
341344
return e
342345
}
343-
return txn.Remove(metaPath)
346+
if e = txn.Remove(metaPath); e != nil {
347+
return e
348+
}
349+
if assignToMetaServiceGroup {
350+
return manager.mgm.updateAssignmentTxn(txn, request.Config[MetaServiceGroupIDKey], "")
351+
}
352+
return nil
344353
})
345354
if err2 != nil {
346355
log.Warn("[create-keyspace] failed to remove pre-created keyspace after split failed",
@@ -419,6 +428,9 @@ func (manager *Manager) CreateKeyspaceByID(request *CreateKeyspaceByIDRequest) (
419428
if err != nil {
420429
return nil, err
421430
}
431+
if request.Config != nil {
432+
delete(request.Config, MetaServiceGroupAddressesKey)
433+
}
422434
userKind := endpoint.StringUserKind(request.Config[UserKindKey])
423435
config, err := manager.kgm.GetKeyspaceConfigByKind(userKind)
424436
if err != nil {
@@ -443,7 +455,7 @@ func (manager *Manager) CreateKeyspaceByID(request *CreateKeyspaceByIDRequest) (
443455
}
444456
assignToMetaServiceGroup := manager.mgm != nil && len(manager.mgm.GetGroups()) > 0
445457
if assignToMetaServiceGroup {
446-
metaServiceGroup, err := manager.mgm.AssignToGroup(1)
458+
metaServiceGroup, err := manager.mgm.SelectGroup()
447459
if err != nil {
448460
return nil, err
449461
}
@@ -474,8 +486,18 @@ func (manager *Manager) CreateKeyspaceByID(request *CreateKeyspaceByIDRequest) (
474486
err = manager.splitKeyspaceRegion(id, manager.config.ToWaitRegionSplit())
475487
if err != nil {
476488
err2 := manager.store.RunInTxn(manager.ctx, func(txn kv.Txn) error {
489+
idPath := keypath.KeyspaceIDPath(name)
477490
metaPath := keypath.KeyspaceMetaPath(id)
478-
return txn.Remove(metaPath)
491+
if err := txn.Remove(idPath); err != nil {
492+
return err
493+
}
494+
if err := txn.Remove(metaPath); err != nil {
495+
return err
496+
}
497+
if assignToMetaServiceGroup {
498+
return manager.mgm.updateAssignmentTxn(txn, request.Config[MetaServiceGroupIDKey], "")
499+
}
500+
return nil
479501
})
480502
if err2 != nil {
481503
log.Warn("[keyspace] failed to remove pre-created keyspace after split failed",
@@ -540,6 +562,13 @@ func (manager *Manager) saveNewKeyspace(keyspace *keyspacepb.KeyspaceMeta) error
540562
if loadedMeta != nil {
541563
return errs.ErrKeyspaceExists
542564
}
565+
if manager.mgm != nil {
566+
if groupID := keyspace.GetConfig()[MetaServiceGroupIDKey]; groupID != "" {
567+
if err := manager.mgm.updateAssignmentTxn(txn, "", groupID); err != nil {
568+
return err
569+
}
570+
}
571+
}
543572
return manager.store.SaveKeyspaceMeta(txn, keyspace)
544573
})
545574
}
@@ -807,6 +836,7 @@ func (manager *Manager) updateKeyspaceConfigTxn(name string, update func(meta *k
807836
if err := update(meta); err != nil {
808837
return err
809838
}
839+
delete(meta.Config, MetaServiceGroupAddressesKey)
810840
newConfig := meta.GetConfig()
811841
oldUserKind := endpoint.StringUserKind(oldConfig[UserKindKey])
812842
newUserKind := endpoint.StringUserKind(newConfig[UserKindKey])
@@ -821,7 +851,10 @@ func (manager *Manager) updateKeyspaceConfigTxn(name string, update func(meta *k
821851
oldMetaServiceGroup := oldConfig[MetaServiceGroupIDKey]
822852
newMetaServiceGroup := newConfig[MetaServiceGroupIDKey]
823853
if manager.mgm != nil && oldMetaServiceGroup != newMetaServiceGroup {
824-
if err := manager.mgm.UpdateAssignment(oldMetaServiceGroup, newMetaServiceGroup); err != nil {
854+
if newMetaServiceGroup != "" && manager.mgm.GetGroups()[newMetaServiceGroup] == "" {
855+
return errUnknownMetaServiceGroup
856+
}
857+
if err := manager.mgm.updateAssignmentTxn(txn, oldMetaServiceGroup, newMetaServiceGroup); err != nil {
825858
return err
826859
}
827860
}

‎pkg/keyspace/meta_service_group.go‎

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -54,42 +54,79 @@ func (m *MetaServiceGroupManager) GetAssignmentCounts() (map[string]int, error)
5454
)
5555
err = m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
5656
count, err = m.store.GetAssignmentCount(txn, m.metaServiceGroups)
57-
if err != nil {
58-
return nil
59-
}
60-
return nil
57+
return err
6158
})
6259
return count, err
6360
}
6461

62+
func (m *MetaServiceGroupManager) selectGroupTxn(txn kv.Txn) (string, error) {
63+
countMap, err := m.store.GetAssignmentCount(txn, m.metaServiceGroups)
64+
if err != nil {
65+
return "", err
66+
}
67+
minCount := math.MaxInt
68+
var assignedGroup string
69+
for currentGroup, currentCount := range countMap {
70+
if currentCount < minCount {
71+
minCount = currentCount
72+
assignedGroup = currentGroup
73+
}
74+
}
75+
if assignedGroup == "" {
76+
return "", errNoAvailableMetaServiceGroups
77+
}
78+
return assignedGroup, nil
79+
}
80+
81+
// SelectGroup returns the meta-service group with the least assigned keyspaces
82+
// without updating the persisted assignment count.
83+
func (m *MetaServiceGroupManager) SelectGroup() (string, error) {
84+
m.RLock()
85+
defer m.RUnlock()
86+
var assignedGroup string
87+
if err := m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
88+
var err error
89+
assignedGroup, err = m.selectGroupTxn(txn)
90+
return err
91+
}); err != nil {
92+
return "", err
93+
}
94+
return assignedGroup, nil
95+
}
96+
6597
// AssignToGroup increments count of the meta-service group with least assigned keyspaces.
6698
// It returns the assigned meta-service group and an error if any.
6799
func (m *MetaServiceGroupManager) AssignToGroup(count int) (string, error) {
68100
m.RLock()
69101
defer m.RUnlock()
70102
var assignedGroup string
71103
if err := m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
72-
countMap, err := m.store.GetAssignmentCount(txn, m.metaServiceGroups)
104+
var err error
105+
assignedGroup, err = m.selectGroupTxn(txn)
73106
if err != nil {
74107
return err
75108
}
76-
minCount := math.MaxInt
77-
for currentGroup, currentCount := range countMap {
78-
if currentCount < minCount {
79-
minCount = currentCount
80-
assignedGroup = currentGroup
81-
}
82-
}
83-
if assignedGroup == "" {
84-
return errNoAvailableMetaServiceGroups
85-
}
86109
return m.store.IncrementAssignmentCount(txn, assignedGroup, count)
87110
}); err != nil {
88111
return "", err
89112
}
90113
return assignedGroup, nil
91114
}
92115

116+
func (m *MetaServiceGroupManager) updateAssignmentTxn(txn kv.Txn, oldGroupID, newGroupID string) error {
117+
if oldGroupID != "" {
118+
if err := m.store.IncrementAssignmentCount(txn, oldGroupID, -1); err != nil {
119+
return err
120+
}
121+
}
122+
if newGroupID != "" {
123+
if err := m.store.IncrementAssignmentCount(txn, newGroupID, 1); err != nil {
124+
return err
125+
}
126+
}
127+
return nil
128+
}
129+
93130
// UpdateAssignment moves a keyspace from one meta-service group to another.
94131
// It returns an error if any.
95132
func (m *MetaServiceGroupManager) UpdateAssignment(oldGroupID, newGroupID string) error {
@@ -100,17 +137,7 @@ func (m *MetaServiceGroupManager) UpdateAssignment(oldGroupID, newGroupID string
100137
return errUnknownMetaServiceGroup
101138
}
102139
return m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
103-
if oldGroupID != "" {
104-
if err := m.store.IncrementAssignmentCount(txn, oldGroupID, -1); err != nil {
105-
return err
106-
}
107-
}
108-
if newGroupID != "" {
109-
if err := m.store.IncrementAssignmentCount(txn, newGroupID, 1); err != nil {
110-
return err
111-
}
112-
}
113-
return nil
140+
return m.updateAssignmentTxn(txn, oldGroupID, newGroupID)
114141
})
115142
}
116143

‎pkg/keyspace/meta_service_group_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.

‎pkg/storage/endpoint/meta_service_group.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.

‎pkg/storage/meta_service_group_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.

‎server/apiv2/handlers/meta_service_group.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -73,18 +73,26 @@ func AddMetaServiceGroups(c *gin.Context) {
7373
}
7474

7575
var requests []AddMetaServiceGroupRequest
76-
err := c.BindJSON(&requests)
76+
err := c.ShouldBindJSON(&requests)
7777
if err != nil {
78-
c.AbortWithStatusJSON(http.StatusBadRequest, errs.ErrBindJSON.Wrap(err).GenWithStackByCause())
78+
c.AbortWithStatusJSON(http.StatusBadRequest, errs.ErrBindJSON.Wrap(err).GenWithStackByCause().Error())
7979
return
8080
}
8181
// Constructs new meta-service groups.
8282
currentGroups := manager.GetGroups()
83-
newGroups := make(map[string]string)
83+
newGroups := make(map[string]string, len(currentGroups)+len(requests))
8484
for _, request := range requests {
85+
if request.ID == "" || request.Addresses == "" {
86+
c.AbortWithStatusJSON(http.StatusBadRequest, "id and addresses must be non-empty")
87+
return
88+
}
89+
if _, exists := newGroups[request.ID]; exists {
90+
c.AbortWithStatusJSON(http.StatusBadRequest, fmt.Sprintf("meta-service group %s is duplicated in request", request.ID))
91+
return
92+
}
8593
// Update existing newGroups is not allowed via the post-method.
8694
if _, exists := currentGroups[request.ID]; exists {
87-
c.AbortWithStatusJSON(http.StatusBadRequest, fmt.Errorf(metaServiceGroupAlreadyExistsErr, request.ID))
95+
c.AbortWithStatusJSON(http.StatusBadRequest, fmt.Sprintf(metaServiceGroupAlreadyExistsErr, request.ID))
8896
return
8997
}
9098
newGroups[request.ID] = request.Addresses

‎tests/server/apiv2/handlers/meta_service_group_test.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -148,4 +148,24 @@ func (suite *metaServiceGroupTestSuite) TestMetaServiceGroupOperations() {
148148
// Add the same keyspace group should result in error.
149149
code, _ := tryAddMetaServiceGroups(re, suite.server, newGroups)
150150
re.Equal(http.StatusBadRequest, code)
151+
152+
// Empty fields should be rejected.
153+
code, _ = tryAddMetaServiceGroups(re, suite.server, []*handlers.AddMetaServiceGroupRequest{{
154+
ID: "",
155+
Addresses: "etcd-group-6.tidb-serverless.cluster.svc.local",
156+
}})
157+
re.Equal(http.StatusBadRequest, code)
158+
159+
// Duplicates within the same request should be rejected.
160+
code, _ = tryAddMetaServiceGroups(re, suite.server, []*handlers.AddMetaServiceGroupRequest{
161+
{
162+
ID: "etcd-group-6",
163+
Addresses: "etcd-group-6.tidb-serverless.cluster.svc.local",
164+
},
165+
{
166+
ID: "etcd-group-6",
167+
Addresses: "etcd-group-6-dup.tidb-serverless.cluster.svc.local",
168+
},
169+
})
170+
re.Equal(http.StatusBadRequest, code)
151171
}

‎tools/pd-ctl/pdctl/command/meta_service_group_command.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 TiKV Project Authors.
1+
// Copyright 2026 TiKV Project Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.

0 commit comments

Comments
 (0)
⚡