Skip to content

Commit

Permalink
Revert "cluster: don't collect region stats in API mode (#7817)" (#7874)
Browse files Browse the repository at this point in the history
ref #5839

Signed-off-by: Cabinfever_B <[email protected]>
  • Loading branch information
CabinfeverB authored Mar 5, 2024
1 parent 3f67cac commit 41012b5
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 31 deletions.
13 changes: 9 additions & 4 deletions server/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,9 +1005,12 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error {
// Save to storage if meta is updated, except for flashback.
// Save to cache if meta or leader is updated, or contains any down/pending peer.
saveKV, saveCache, needSync := regionGuide(region, origin)
if !c.IsServiceIndependent(mcsutils.SchedulingServiceName) && !saveKV && !saveCache {
if !saveKV && !saveCache {
// Due to some config changes need to update the region stats as well,
// so we do some extra checks here.
// TODO: Due to the accuracy requirements of the API "/regions/check/xxx",
// region stats needs to be collected in API mode.
// We need to think of a better way to reduce this part of the cost in the future.
if hasRegionStats && c.regionStats.RegionStatsNeedUpdate(region) {
c.regionStats.Observe(region, c.getRegionStoresLocked(region))
}
Expand Down Expand Up @@ -1035,9 +1038,11 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error {
}
regionUpdateCacheEventCounter.Inc()
}
if !c.IsServiceIndependent(mcsutils.SchedulingServiceName) {
cluster.Collect(c, region, c.GetRegionStores(region), hasRegionStats)
}

// TODO: Due to the accuracy requirements of the API "/regions/check/xxx",
// region stats needs to be collected in API mode.
// We need to think of a better way to reduce this part of the cost in the future.
cluster.Collect(c, region, c.GetRegionStores(region), hasRegionStats)

if c.storage != nil {
// If there are concurrent heartbeats from the same region, the last write will win even if
Expand Down
27 changes: 0 additions & 27 deletions tests/server/api/region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/stretchr/testify/suite"
"github.com/tikv/pd/pkg/core"
"github.com/tikv/pd/pkg/schedule/placement"
"github.com/tikv/pd/pkg/statistics"
tu "github.com/tikv/pd/pkg/utils/testutil"
"github.com/tikv/pd/tests"
)
Expand Down Expand Up @@ -173,32 +172,6 @@ func (suite *regionTestSuite) checkAccelerateRegionsScheduleInRange(cluster *tes
re.Len(idList, 2, len(idList))
}

func (suite *regionTestSuite) TestRegionStats() {
env := tests.NewSchedulingTestEnvironment(suite.T())
env.RunTestInAPIMode(suite.checkRegionStats)
env.Cleanup()
}

func (suite *regionTestSuite) checkRegionStats(cluster *tests.TestCluster) {
re := suite.Require()
leader := cluster.GetLeaderServer()
rc := leader.GetRaftCluster()
re.NotNil(rc)
for i := 13; i <= 16; i++ {
s1 := &metapb.Store{
Id: uint64(i),
State: metapb.StoreState_Up,
NodeState: metapb.NodeState_Serving,
}
tests.MustPutStore(re, cluster, s1)
}
r := core.NewTestRegionInfo(1001, 13, []byte("b1"), []byte("b2"), core.SetApproximateSize(0))
r.GetMeta().Peers = append(r.GetMeta().Peers, &metapb.Peer{Id: 5, StoreId: 14}, &metapb.Peer{Id: 6, StoreId: 15})
tests.MustPutRegionInfo(re, cluster, r)
suite.checkRegionCount(re, cluster, 1)
re.False(rc.GetRegionStats().IsRegionStatsType(1001, statistics.EmptyRegion))
}

func (suite *regionTestSuite) TestAccelerateRegionsScheduleInRanges() {
suite.env.RunTestInTwoModes(suite.checkAccelerateRegionsScheduleInRanges)
}
Expand Down

0 comments on commit 41012b5

Please sign in to comment.