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

*: optimize heartbeat process with concurrent runner - part 1 #8053

Merged
merged 10 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
14 changes: 12 additions & 2 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
GetLabelStats() *statistics.LabelStatistics
GetCoordinator() *schedule.Coordinator
GetRuleManager() *placement.RuleManager
GetBasicCluster() *core.BasicCluster
}

// HandleStatsAsync handles the flow asynchronously.
Expand Down Expand Up @@ -55,8 +56,17 @@
}

// Collect collects the cluster information.
func Collect(c Cluster, region *core.RegionInfo, stores []*core.StoreInfo, hasRegionStats bool) {
func Collect(c Cluster, region *core.RegionInfo, hasRegionStats bool) {
if hasRegionStats {
c.GetRegionStats().Observe(region, stores)
// get region again from root tree. make sure the observed region is the latest.
bc := c.GetBasicCluster()
if bc == nil {
return

Check warning on line 64 in pkg/cluster/cluster.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster.go#L64

Added line #L64 was not covered by tests
}
region = bc.GetRegion(region.GetID())
if region == nil {
return
}
c.GetRegionStats().Observe(region, c.GetBasicCluster().GetRegionStores(region))
}
}
87 changes: 67 additions & 20 deletions pkg/core/region.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import (
"bytes"
"context"
"encoding/hex"
"fmt"
"math"
Expand All @@ -35,6 +36,8 @@
"github.com/pingcap/kvproto/pkg/replication_modepb"
"github.com/pingcap/log"
"github.com/tikv/pd/pkg/errs"
"github.com/tikv/pd/pkg/ratelimit"
"github.com/tikv/pd/pkg/utils/ctxutil"
"github.com/tikv/pd/pkg/utils/logutil"
"github.com/tikv/pd/pkg/utils/syncutil"
"github.com/tikv/pd/pkg/utils/typeutil"
Expand Down Expand Up @@ -711,20 +714,51 @@

// RegionGuideFunc is a function that determines which follow-up operations need to be performed based on the origin
// and new region information.
type RegionGuideFunc func(region, origin *RegionInfo) (saveKV, saveCache, needSync bool)
type RegionGuideFunc func(ctx context.Context, region, origin *RegionInfo) (saveKV, saveCache, needSync bool)

// GenerateRegionGuideFunc is used to generate a RegionGuideFunc. Control the log output by specifying the log function.
// nil means do not print the log.
func GenerateRegionGuideFunc(enableLog bool) RegionGuideFunc {
noLog := func(string, ...zap.Field) {}
debug, info := noLog, noLog
d, i := noLog, noLog
debug, info := d, i
if enableLog {
debug = log.Debug
info = log.Info
d = log.Debug
i = log.Info
debug, info = d, i
}
// Save to storage if meta is updated.
// Save to cache if meta or leader is updated, or contains any down/pending peer.
return func(region, origin *RegionInfo) (saveKV, saveCache, needSync bool) {
return func(ctx context.Context, region, origin *RegionInfo) (saveKV, saveCache, needSync bool) {
taskRunner, ok := ctx.Value(ctxutil.TaskRunnerKey).(ratelimit.Runner)
limiter, _ := ctx.Value(ctxutil.LimiterKey).(*ratelimit.ConcurrencyLimiter)
nolouch marked this conversation as resolved.
Show resolved Hide resolved
// print log asynchronously
if ok {
debug = func(msg string, fields ...zap.Field) {
taskRunner.RunTask(
Copy link
Member

Choose a reason for hiding this comment

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

Is the error needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add metrics about this error in next PR.

ctx,
ratelimit.TaskOpts{
TaskName: "Log",
Limit: limiter,
},
func(_ context.Context) {
HuSharp marked this conversation as resolved.
Show resolved Hide resolved
d(msg, fields...)
},

Check warning on line 746 in pkg/core/region.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/region.go#L737-L746

Added lines #L737 - L746 were not covered by tests
)
}
info = func(msg string, fields ...zap.Field) {
taskRunner.RunTask(
ctx,
ratelimit.TaskOpts{
TaskName: "Log",
Limit: limiter,
},
func(_ context.Context) {
i(msg, fields...)
},

Check warning on line 758 in pkg/core/region.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/region.go#L749-L758

Added lines #L749 - L758 were not covered by tests
)
}
}
if origin == nil {
if log.GetLevel() <= zap.DebugLevel {
debug("insert new region",
Expand Down Expand Up @@ -789,7 +823,7 @@
}
if !SortedPeersStatsEqual(region.GetDownPeers(), origin.GetDownPeers()) {
if log.GetLevel() <= zap.DebugLevel {
debug("down-peers changed", zap.Uint64("region-id", region.GetID()))
debug("down-peers changed", zap.Uint64("region-id", region.GetID()), zap.Reflect("before", origin.GetDownPeers()), zap.Reflect("after", region.GetDownPeers()))

Check warning on line 826 in pkg/core/region.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/region.go#L826

Added line #L826 was not covered by tests
}
saveCache, needSync = true, true
return
Expand Down Expand Up @@ -912,7 +946,7 @@
if origin == nil || !bytes.Equal(origin.GetStartKey(), region.GetStartKey()) || !bytes.Equal(origin.GetEndKey(), region.GetEndKey()) {
ols = r.tree.overlaps(&regionItem{RegionInfo: region})
}
err := check(region, origin, ols)
err := check(region, origin, convertItemsToRegions(ols))
if err != nil {
log.Debug("region is stale", zap.Stringer("origin", origin.GetMeta()), errs.ZapError(err))
// return the state region to delete.
Expand All @@ -933,48 +967,62 @@
}

// PreCheckPutRegion checks if the region is valid to put.
func (r *RegionsInfo) PreCheckPutRegion(region *RegionInfo, trace RegionHeartbeatProcessTracer) (*RegionInfo, []*regionItem, error) {
origin, overlaps := r.GetRelevantRegions(region, trace)
func (r *RegionsInfo) PreCheckPutRegion(region *RegionInfo) (*RegionInfo, []*RegionInfo, error) {
origin, overlaps := r.GetRelevantRegions(region)
err := check(region, origin, overlaps)
return origin, overlaps, err
}

func convertItemsToRegions(items []*regionItem) []*RegionInfo {
regions := make([]*RegionInfo, 0, len(items))
for _, item := range items {
regions = append(regions, item.RegionInfo)
}
return regions
}

// AtomicCheckAndPutRegion checks if the region is valid to put, if valid then put.
func (r *RegionsInfo) AtomicCheckAndPutRegion(region *RegionInfo, trace RegionHeartbeatProcessTracer) ([]*RegionInfo, error) {
func (r *RegionsInfo) AtomicCheckAndPutRegion(ctx context.Context, region *RegionInfo) ([]*RegionInfo, error) {
tracer, ok := ctx.Value(ctxutil.HeartbeatTracerKey).(RegionHeartbeatProcessTracer)
if !ok {
tracer = NewNoopHeartbeatProcessTracer()
}
r.t.Lock()
var ols []*regionItem
origin := r.getRegionLocked(region.GetID())
if origin == nil || !bytes.Equal(origin.GetStartKey(), region.GetStartKey()) || !bytes.Equal(origin.GetEndKey(), region.GetEndKey()) {
ols = r.tree.overlaps(&regionItem{RegionInfo: region})
}
trace.OnCheckOverlapsFinished()
err := check(region, origin, ols)
tracer.OnCheckOverlapsFinished()
err := check(region, origin, convertItemsToRegions(ols))
if err != nil {
r.t.Unlock()
trace.OnValidateRegionFinished()
tracer.OnValidateRegionFinished()
return nil, err
}
trace.OnValidateRegionFinished()
tracer.OnValidateRegionFinished()
origin, overlaps, rangeChanged := r.setRegionLocked(region, true, ols...)
r.t.Unlock()
trace.OnSetRegionFinished()
tracer.OnSetRegionFinished()
r.UpdateSubTree(region, origin, overlaps, rangeChanged)
trace.OnUpdateSubTreeFinished()
tracer.OnUpdateSubTreeFinished()
return overlaps, nil
}

// GetRelevantRegions returns the relevant regions for a given region.
func (r *RegionsInfo) GetRelevantRegions(region *RegionInfo, _ RegionHeartbeatProcessTracer) (origin *RegionInfo, overlaps []*regionItem) {
func (r *RegionsInfo) GetRelevantRegions(region *RegionInfo) (origin *RegionInfo, overlaps []*RegionInfo) {
r.t.RLock()
defer r.t.RUnlock()
origin = r.getRegionLocked(region.GetID())
if origin == nil || !bytes.Equal(origin.GetStartKey(), region.GetStartKey()) || !bytes.Equal(origin.GetEndKey(), region.GetEndKey()) {
overlaps = r.tree.overlaps(&regionItem{RegionInfo: region})
for _, item := range r.tree.overlaps(&regionItem{RegionInfo: region}) {
overlaps = append(overlaps, item.RegionInfo)
}
}
return
}

func check(region, origin *RegionInfo, overlaps []*regionItem) error {
func check(region, origin *RegionInfo, overlaps []*RegionInfo) error {
for _, item := range overlaps {
// PD ignores stale regions' heartbeats, unless it is recreated recently by unsafe recover operation.
if region.GetRegionEpoch().GetVersion() < item.GetRegionEpoch().GetVersion() && !region.isRegionRecreated() {
Expand Down Expand Up @@ -1043,7 +1091,6 @@
item = &regionItem{RegionInfo: region}
r.regions[region.GetID()] = item
}

var overlaps []*RegionInfo
if rangeChanged {
overlaps = r.tree.update(item, withOverlaps, ol...)
Expand Down
7 changes: 4 additions & 3 deletions pkg/core/region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package core

import (
"context"
"crypto/rand"
"fmt"
"math"
Expand Down Expand Up @@ -363,7 +364,7 @@ func TestNeedSync(t *testing.T) {
for _, testCase := range testCases {
regionA := region.Clone(testCase.optionsA...)
regionB := region.Clone(testCase.optionsB...)
_, _, needSync := RegionGuide(regionA, regionB)
_, _, needSync := RegionGuide(context.TODO(), regionA, regionB)
re.Equal(testCase.needSync, needSync)
}
}
Expand Down Expand Up @@ -459,9 +460,9 @@ func TestSetRegionConcurrence(t *testing.T) {
regions := NewRegionsInfo()
region := NewTestRegionInfo(1, 1, []byte("a"), []byte("b"))
go func() {
regions.AtomicCheckAndPutRegion(region, NewNoopHeartbeatProcessTracer())
regions.AtomicCheckAndPutRegion(context.TODO(), region)
}()
regions.AtomicCheckAndPutRegion(region, NewNoopHeartbeatProcessTracer())
regions.AtomicCheckAndPutRegion(context.TODO(), region)
re.NoError(failpoint.Disable("github.com/tikv/pd/pkg/core/UpdateSubTree"))
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/core/region_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ func (r *regionItem) GetStartKey() []byte {
return r.meta.StartKey
}

// GetID returns the ID of the region.
func (r *regionItem) GetID() uint64 {
return r.meta.GetId()
}

// GetEndKey returns the end key of the region.
func (r *regionItem) GetEndKey() []byte {
return r.meta.EndKey
Expand Down
2 changes: 2 additions & 0 deletions pkg/core/region_tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ func TestRegionTree(t *testing.T) {

updateNewItem(tree, regionA)
updateNewItem(tree, regionC)
re.Nil(tree.overlaps(newRegionItem([]byte("b"), []byte("c"))))
re.Equal(regionC, tree.overlaps(newRegionItem([]byte("a"), []byte("cc")))[1].RegionInfo)
re.Nil(tree.search([]byte{}))
re.Equal(regionA, tree.search([]byte("a")))
re.Nil(tree.search([]byte("b")))
Expand Down
Loading
Loading