*: make flow-round-by-digit compatible with trace-region-flow (tikv#3747

) * *: make flow-round-by-digit compatible with trace-region-flow Signed-off-by: nolouch <[email protected]> * fix test Signed-off-by: nolouch <[email protected]> * fix test Signed-off-by: nolouch <[email protected]>
bufferflies · Jul 9, 2021 · ad1dfed · ad1dfed
1 parent 7554b71
commit ad1dfed
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 12 deletions.
diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go
@@ -659,8 +659,8 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error {
 		}
 		// Once flow has changed, will update the cache.
 		// Because keys and bytes are strongly related, only bytes are judged.
-		if c.traceRegionFlow && (region.GetRoundBytesWritten() != origin.GetRoundBytesWritten() ||
-			region.GetRoundBytesRead() != origin.GetRoundBytesRead()) {
+		if region.GetRoundBytesWritten() != origin.GetRoundBytesWritten() ||
+			region.GetRoundBytesRead() != origin.GetRoundBytesRead() {
 			saveCache, needSync = true, true
 		}
 

diff --git a/server/cluster/cluster_test.go b/server/cluster/cluster_test.go
@@ -622,13 +622,6 @@ func (s *testClusterInfoSuite) TestRegionFlowChanged(c *C) {
 	processRegions(regions)
 	newRegion := cluster.GetRegion(region.GetID())
 	c.Assert(newRegion.GetBytesRead(), Equals, uint64(1000))
-
-	// do not trace the flow changes
-	cluster.traceRegionFlow = false
-	processRegions([]*core.RegionInfo{region})
-	newRegion = cluster.GetRegion(region.GetID())
-	c.Assert(region.GetBytesRead(), Equals, uint64(0))
-	c.Assert(newRegion.GetBytesRead(), Not(Equals), uint64(0))
 }
 
 func (s *testClusterInfoSuite) TestConcurrentRegionHeartbeat(c *C) {

diff --git a/server/config/config.go b/server/config/config.go
@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
+	"math"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -1076,9 +1077,9 @@ type PDServerConfig struct {
 	MetricStorage string `toml:"metric-storage" json:"metric-storage"`
 	// There are some values supported: "auto", "none", or a specific address, default: "auto"
 	DashboardAddress string `toml:"dashboard-address" json:"dashboard-address"`
-	// TraceRegionFlow the option to update flow information of regions
-	// TODO: deprecate
-	TraceRegionFlow bool `toml:"trace-region-flow" json:"trace-region-flow,string"`
+	// TraceRegionFlow the option to update flow information of regions.
+	// WARN: TraceRegionFlow is deprecated.
+	TraceRegionFlow bool `toml:"trace-region-flow" json:"trace-region-flow,string,omitempty"`
 	// FlowRoundByDigit used to discretization processing flow information.
 	FlowRoundByDigit int `toml:"flow-round-by-digit" json:"flow-round-by-digit"`
 }
@@ -1103,9 +1104,35 @@ func (c *PDServerConfig) adjust(meta *configMetaData) error {
 	if !meta.IsDefined("flow-round-by-digit") {
 		adjustInt(&c.FlowRoundByDigit, defaultFlowRoundByDigit)
 	}
+	c.migrateConfigurationFromFile(meta)
 	return c.Validate()
 }
 
+func (c *PDServerConfig) migrateConfigurationFromFile(meta *configMetaData) error {
+	oldName, newName := "trace-region-flow", "flow-round-by-digit"
+	defineOld, defineNew := meta.IsDefined(oldName), meta.IsDefined(newName)
+	switch {
+	case defineOld && defineNew:
+		if c.TraceRegionFlow && (c.FlowRoundByDigit == defaultFlowRoundByDigit) {
+			return errors.Errorf("config item %s and %s(deprecated) are conflict", newName, oldName)
+		}
+	case defineOld && !defineNew:
+		if !c.TraceRegionFlow {
+			c.FlowRoundByDigit = math.MaxInt8
+		}
+	}
+	return nil
+}
+
+// MigrateDeprecatedFlags updates new flags according to deprecated flags.
+func (c *PDServerConfig) MigrateDeprecatedFlags() {
+	if !c.TraceRegionFlow {
+		c.FlowRoundByDigit = math.MaxInt8
+	}
+	// json omity the false. next time will not persist to the kv.
+	c.TraceRegionFlow = false
+}
+
 // Clone returns a cloned PD server config.
 func (c *PDServerConfig) Clone() *PDServerConfig {
 	runtimeServices := append(c.RuntimeServices[:0:0], c.RuntimeServices...)

diff --git a/server/config/config_test.go b/server/config/config_test.go
@@ -16,6 +16,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"math"
 	"os"
 	"path"
 	"strings"
@@ -284,13 +285,16 @@ func (s *testConfigSuite) TestMigrateFlags(c *C) {
 		return cfg, err
 	}
 	cfg, err := load(`
+[pd-server]
+trace-region-flow = false
 [schedule]
 disable-remove-down-replica = true
 enable-make-up-replica = false
 disable-remove-extra-replica = true
 enable-remove-extra-replica = false
 `)
 	c.Assert(err, IsNil)
+	c.Assert(cfg.PDServerCfg.FlowRoundByDigit, Equals, math.MaxInt8)
 	c.Assert(cfg.Schedule.EnableReplaceOfflineReplica, IsTrue)
 	c.Assert(cfg.Schedule.EnableRemoveDownReplica, IsFalse)
 	c.Assert(cfg.Schedule.EnableMakeUpReplica, IsFalse)

diff --git a/server/config/persist_options.go b/server/config/persist_options.go
@@ -580,6 +580,7 @@ func (o *PersistOptions) Reload(storage *core.Storage) error {
 		return err
 	}
 	o.adjustScheduleCfg(&cfg.Schedule)
+	cfg.PDServerCfg.MigrateDeprecatedFlags()
 	if isExist {
 		o.schedule.Store(&cfg.Schedule)
 		o.replication.Store(&cfg.Replication)