Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
33 changes: 21 additions & 12 deletions pkg/keyspace/keyspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/tikv/pd/pkg/storage/kv"
"github.com/tikv/pd/pkg/utils/etcdutil"
"github.com/tikv/pd/pkg/utils/keypath"
"github.com/tikv/pd/pkg/utils/logutil"
"github.com/tikv/pd/pkg/utils/syncutil"
"github.com/tikv/pd/pkg/versioninfo/kerneltype"
)
Expand Down Expand Up @@ -74,6 +73,7 @@ const (
type Config interface {
GetPreAlloc() []string
ToWaitRegionSplit() bool
GetDisableRawKVRegionSplit() bool
GetWaitRegionSplitTimeout() time.Duration
GetCheckRegionSplitInterval() time.Duration
}
Expand Down Expand Up @@ -506,7 +506,8 @@ func (manager *Manager) splitKeyspaceRegion(id uint32, waitRegionSplit bool) (er
})

start := time.Now()
keyspaceRule := MakeLabelRule(id)
skipRaw := manager.config.GetDisableRawKVRegionSplit()
keyspaceRule := makeLabelRule(id, skipRaw)
cl, ok := manager.cluster.(interface{ GetRegionLabeler() *labeler.RegionLabeler })
if !ok {
return errors.New("cluster does not support region label")
Expand All @@ -527,11 +528,17 @@ func (manager *Manager) splitKeyspaceRegion(id uint32, waitRegionSplit bool) (er
zap.Error(err),
)
}
return
}
log.Info("[keyspace] added region label for keyspace",
zap.Uint32("keyspace-id", id),
zap.Any("label-rule", keyspaceRule),
zap.Duration("takes", time.Since(start)),
)
}()

if waitRegionSplit {
err = manager.waitKeyspaceRegionSplit(id)
err = manager.waitKeyspaceRegionSplit(id, skipRaw)
if err != nil {
log.Warn("[keyspace] wait region split meets error",
zap.Uint32("keyspace-id", id),
Expand All @@ -540,16 +547,10 @@ func (manager *Manager) splitKeyspaceRegion(id uint32, waitRegionSplit bool) (er
}
return err
}

log.Info("[keyspace] added region label for keyspace",
zap.Uint32("keyspace-id", id),
logutil.ZapRedactString("label-rule", keyspaceRule.String()),
zap.Duration("takes", time.Since(start)),
)
return
return nil
}

func (manager *Manager) waitKeyspaceRegionSplit(id uint32) error {
func (manager *Manager) waitKeyspaceRegionSplit(id uint32, skipRaw bool) error {
ticker := time.NewTicker(manager.config.GetCheckRegionSplitInterval())
timer := time.NewTimer(manager.config.GetWaitRegionSplitTimeout())
defer func() {
Expand All @@ -561,7 +562,7 @@ func (manager *Manager) waitKeyspaceRegionSplit(id uint32) error {
case <-manager.ctx.Done():
return errors.New("[keyspace] wait region split canceled")
case <-ticker.C:
if manager.CheckKeyspaceRegionBound(id) {
if manager.checkKeyspaceRegionBound(id, skipRaw) {
log.Info("[keyspace] wait region split successfully", zap.Uint32("keyspace-id", id))
return nil
}
Expand All @@ -576,7 +577,15 @@ func (manager *Manager) waitKeyspaceRegionSplit(id uint32) error {

// CheckKeyspaceRegionBound checks whether the keyspace region has been split.
func (manager *Manager) CheckKeyspaceRegionBound(id uint32) bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If a keyspace created with disable-raw-kv-region-split=true only has txn bounds. If the config is later changed to false, it may never pass CheckKeyspaceRegionBound again and can become effectively invisible to LoadKeyspace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the existing keyspace isn't affected by the configuration by design, and later add notes on the user docs.

Copy link
Copy Markdown
Contributor

@lhy1024 lhy1024 Apr 3, 2026

Choose a reason for hiding this comment

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

If this is intended to be immutable after keyspace creation, can we enforce that in code (for example, reject dynamic updates for this field)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch, I will create new issue to follow it #10572

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to add TODO here?

return manager.checkKeyspaceRegionBound(id, manager.config.GetDisableRawKVRegionSplit())
}

func (manager *Manager) checkKeyspaceRegionBound(id uint32, skipRaw bool) bool {
regionBound := MakeRegionBound(id)
if skipRaw {
return manager.checkBound(regionBound.TxnLeftBound) &&
manager.checkBound(regionBound.TxnRightBound)
}
return manager.checkBound(regionBound.RawLeftBound) &&
manager.checkBound(regionBound.RawRightBound) &&
manager.checkBound(regionBound.TxnLeftBound) &&
Expand Down
5 changes: 5 additions & 0 deletions pkg/keyspace/keyspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestKeyspaceTestSuite(t *testing.T) {
type mockConfig struct {
PreAlloc []string
WaitRegionSplit bool
DisableRawKVRegionSplit bool
WaitRegionSplitTimeout typeutil.Duration
CheckRegionSplitInterval typeutil.Duration
}
Expand All @@ -86,6 +87,10 @@ func (m *mockConfig) GetCheckRegionSplitInterval() time.Duration {
return m.CheckRegionSplitInterval.Duration
}

func (m *mockConfig) GetDisableRawKVRegionSplit() bool {
return m.DisableRawKVRegionSplit
}

func (suite *keyspaceTestSuite) SetupTest() {
re := suite.Require()
suite.ctx, suite.cancel = context.WithCancel(context.Background())
Expand Down
18 changes: 13 additions & 5 deletions pkg/keyspace/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,17 @@ func MakeRegionBound(id uint32) *RegionBound {
}
}

// MakeKeyRanges encodes keyspace ID to correct LabelRule data.
func MakeKeyRanges(id uint32) []any {
// makeKeyRanges encodes keyspace ID to correct LabelRule data.
func makeKeyRanges(id uint32, skipRaw bool) any {
regionBound := MakeRegionBound(id)
if skipRaw {
return []any{
map[string]any{
"start_key": hex.EncodeToString(regionBound.TxnLeftBound),
"end_key": hex.EncodeToString(regionBound.TxnRightBound),
},
}
}
return []any{
map[string]any{
"start_key": hex.EncodeToString(regionBound.RawLeftBound),
Expand All @@ -146,8 +154,8 @@ func getRegionLabelID(id uint32) string {
return regionLabelIDPrefix + strconv.FormatUint(uint64(id), endpoint.SpaceIDBase)
}

// MakeLabelRule makes the label rule for the given keyspace id.
func MakeLabelRule(id uint32) *labeler.LabelRule {
// makeLabelRule makes the label rule for the given keyspace id.
func makeLabelRule(id uint32, skipRaw bool) *labeler.LabelRule {
return &labeler.LabelRule{
ID: getRegionLabelID(id),
Index: 0,
Expand All @@ -158,7 +166,7 @@ func MakeLabelRule(id uint32) *labeler.LabelRule {
},
},
RuleType: labeler.KeyRange,
Data: MakeKeyRanges(id),
Data: makeKeyRanges(id, skipRaw),
}
}

Expand Down
53 changes: 49 additions & 4 deletions pkg/keyspace/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ func TestMakeLabelRule(t *testing.T) {
re := require.New(t)
testCases := []struct {
id uint32
skipRaw bool
expectedLabelRule *labeler.LabelRule
}{
{
id: 0,
id: 0,
skipRaw: false,
expectedLabelRule: &labeler.LabelRule{
ID: getRegionLabelID(0),
Index: 0,
Expand All @@ -171,7 +173,8 @@ func TestMakeLabelRule(t *testing.T) {
},
},
{
id: 4242,
id: 4242,
skipRaw: false,
expectedLabelRule: &labeler.LabelRule{
ID: getRegionLabelID(4242),
Index: 0,
Expand All @@ -194,9 +197,51 @@ func TestMakeLabelRule(t *testing.T) {
},
},
},
{
id: 0,
skipRaw: true,
expectedLabelRule: &labeler.LabelRule{
ID: "keyspaces/0",
Index: 0,
Labels: []labeler.RegionLabel{
{
Key: "id",
Value: "0",
},
},
RuleType: "key-range",
Data: []any{
map[string]any{
"start_key": hex.EncodeToString(codec.EncodeBytes([]byte{'x', 0, 0, 0})),
"end_key": hex.EncodeToString(codec.EncodeBytes([]byte{'x', 0, 0, 1})),
},
},
},
},
{
id: 4242,
skipRaw: true,
expectedLabelRule: &labeler.LabelRule{
ID: "keyspaces/4242",
Index: 0,
Labels: []labeler.RegionLabel{
{
Key: "id",
Value: "4242",
},
},
RuleType: "key-range",
Data: []any{
map[string]any{
"start_key": hex.EncodeToString(codec.EncodeBytes([]byte{'x', 0, 0x10, 0x92})),
"end_key": hex.EncodeToString(codec.EncodeBytes([]byte{'x', 0, 0x10, 0x93})),
},
},
},
},
}
for _, testCase := range testCases {
re.Equal(testCase.expectedLabelRule, MakeLabelRule(testCase.id))
re.Equal(testCase.expectedLabelRule, makeLabelRule(testCase.id, testCase.skipRaw))
}
}

Expand All @@ -209,7 +254,7 @@ func TestParseKeyspaceIDFromLabelRule(t *testing.T) {
}{
// Valid keyspace label rule.
{
labelRule: MakeLabelRule(1),
labelRule: makeLabelRule(1, false),
expectedID: 1,
expectedOK: true,
},
Expand Down
9 changes: 8 additions & 1 deletion pkg/mcs/scheduling/server/rule/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package rule

import (
"context"
"encoding/hex"
"encoding/json"
"os"
"strconv"
Expand Down Expand Up @@ -99,11 +100,17 @@ func prepare(t require.TestingT) (context.Context, *clientv3.Client, func()) {
<-etcd.Server.ReadyNotify()

for i := 1; i < rulesNum+1; i++ {
regionBound := keyspace.MakeRegionBound(uint32(i))
rule := &labeler.LabelRule{
ID: "test_" + strconv.Itoa(i),
Labels: []labeler.RegionLabel{{Key: "test", Value: "test"}},
RuleType: labeler.KeyRange,
Data: keyspace.MakeKeyRanges(uint32(i)),
Data: labeler.MakeKeyRanges(
hex.EncodeToString(regionBound.TxnLeftBound),
hex.EncodeToString(regionBound.TxnRightBound),
hex.EncodeToString(regionBound.RawLeftBound),
hex.EncodeToString(regionBound.RawRightBound),
),
}
value, err := json.Marshal(rule)
re.NoError(err)
Expand Down
11 changes: 11 additions & 0 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ const (
defaultGCTunerThreshold = 0.6
minGCTunerThreshold = 0
maxGCTunerThreshold = 0.9
defaultDisableRawKVRegionSplit = false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we change to enablexxx?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It may introduce configuration compatibility issues.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But usually we use enablexxx for a switch.


defaultWaitRegionSplitTimeout = 30 * time.Second
defaultCheckRegionSplitInterval = 50 * time.Millisecond
Expand Down Expand Up @@ -877,6 +878,8 @@ type KeyspaceConfig struct {
WaitRegionSplit bool `toml:"wait-region-split" json:"wait-region-split"`
// WaitRegionSplitTimeout indicates the max duration to wait region split.
WaitRegionSplitTimeout typeutil.Duration `toml:"wait-region-split-timeout" json:"wait-region-split-timeout"`
// DisableRawKVRegionSplit indicates whether to skip raw kv region split.
DisableRawKVRegionSplit bool `toml:"disable-raw-kv-region-split" json:"disable-raw-kv-region-split,string"`
// CheckRegionSplitInterval indicates the interval to check whether the region split is complete
CheckRegionSplitInterval typeutil.Duration `toml:"check-region-split-interval" json:"check-region-split-interval"`
}
Expand All @@ -903,6 +906,9 @@ func (c *KeyspaceConfig) adjust(meta *configutil.ConfigMetaData) {
if !meta.IsDefined("check-region-split-interval") {
c.CheckRegionSplitInterval = typeutil.NewDuration(defaultCheckRegionSplitInterval)
}
if !meta.IsDefined("disable-raw-kv-region-split") {
c.DisableRawKVRegionSplit = defaultDisableRawKVRegionSplit
}
}

// Clone makes a deep copy of the keyspace config.
Expand All @@ -923,6 +929,11 @@ func (c *KeyspaceConfig) ToWaitRegionSplit() bool {
return c.WaitRegionSplit
}

// GetDisableRawKVRegionSplit returns whether to skip raw kv region split.
func (c *KeyspaceConfig) GetDisableRawKVRegionSplit() bool {
return c.DisableRawKVRegionSplit
}

// GetWaitRegionSplitTimeout returns the max duration to wait region split.
func (c *KeyspaceConfig) GetWaitRegionSplitTimeout() time.Duration {
return c.WaitRegionSplitTimeout.Duration
Expand Down
Loading