core: use hex format for region meta key (#1627) (#1639)

* core: use hex format for region meta key (#1627) Signed-off-by: Ryan Leung <[email protected]>= * add CHANGELOG Signed-off-by: Ryan Leung <[email protected]>
tikv · Jul 15, 2019 · 6ae581e · 6ae581e
1 parent d149356
commit 6ae581e
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 + Set influence for the operator according to the region size [#1613](https://github.com/pingcap/pd/pull/1613)
 + Enlarge the default limit of the hot region scheduler [#1616](https://github.com/pingcap/pd/pull/1616)
 + Fix the issue about ignoring the pending peer when balancing regions [#1617](https://github.com/pingcap/pd/pull/1617)
++ Fix the issue about some Region meta keys in log output are not in the hex format [#1627](https://github.com/pingcap/pd/pull/1627)
 + Fix the issue about `random-merge` and `admin-merge-region` cannot be added [#1633](https://github.com/pingcap/pd/pull/1633)
 
 ## v3.0.0

diff --git a/server/cluster_info.go b/server/cluster_info.go
@@ -532,7 +532,7 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
 	if origin == nil {
 		log.Debug("insert new region",
 			zap.Uint64("region-id", region.GetID()),
-			zap.Reflect("meta-region", core.HexRegionMeta(region.GetMeta())),
+			zap.Stringer("meta-region", core.RegionToHexMeta(region.GetMeta())),
 		)
 		saveKV, saveCache, isNew = true, true, true
 	} else {
@@ -595,7 +595,7 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
 			// after restart. Here we only log the error then go on updating cache.
 			log.Error("fail to save region to kv",
 				zap.Uint64("region-id", region.GetID()),
-				zap.Reflect("region-meta", core.HexRegionMeta(region.GetMeta())),
+				zap.Stringer("region-meta", core.RegionToHexMeta(region.GetMeta())),
 				zap.Error(err))
 		}
 		select {
@@ -620,7 +620,7 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
 				if err := c.kv.DeleteRegion(item); err != nil {
 					log.Error("fail to delete region from kv",
 						zap.Uint64("region-id", item.GetId()),
-						zap.Reflect("region-meta", core.HexRegionMeta(item)),
+						zap.Stringer("region-meta", core.RegionToHexMeta(item)),
 						zap.Error(err))
 				}
 			}

diff --git a/server/cluster_worker.go b/server/cluster_worker.go
@@ -36,8 +36,8 @@ func (c *RaftCluster) HandleRegionHeartbeat(region *core.RegionInfo) error {
 
 	// If the region peer count is 0, then we should not handle this.
 	if len(region.GetPeers()) == 0 {
-		log.Warn("invalid region, zero region peer count", zap.Reflect("region-meta", core.HexRegionMeta(region.GetMeta())))
-		return errors.Errorf("invalid region, zero region peer count: %v", core.HexRegionMeta(region.GetMeta()))
+		log.Warn("invalid region, zero region peer count", zap.Stringer("region-meta", core.RegionToHexMeta(region.GetMeta())))
+		return errors.Errorf("invalid region, zero region peer count: %v", core.RegionToHexMeta(region.GetMeta()))
 	}
 
 	c.coordinator.opController.Dispatch(region, schedule.DispatchFromHeartBeat)
@@ -81,7 +81,7 @@ func (c *RaftCluster) validRequestRegion(reqRegion *metapb.Region) error {
 	startKey := reqRegion.GetStartKey()
 	region, _ := c.GetRegionByKey(startKey)
 	if region == nil {
-		return errors.Errorf("region not found, request region: %v", core.HexRegionMeta(reqRegion))
+		return errors.Errorf("region not found, request region: %v", core.RegionToHexMeta(reqRegion))
 	}
 	// If the request epoch is less than current region epoch, then returns an error.
 	reqRegionEpoch := reqRegion.GetRegionEpoch()
@@ -172,8 +172,8 @@ func (c *RaftCluster) handleReportSplit(request *pdpb.ReportSplitRequest) (*pdpb
 	err := c.checkSplitRegion(left, right)
 	if err != nil {
 		log.Warn("report split region is invalid",
-			zap.Reflect("left-region", core.HexRegionMeta(left)),
-			zap.Reflect("right-region", core.HexRegionMeta(right)),
+			zap.Stringer("left-region", core.RegionToHexMeta(left)),
+			zap.Stringer("right-region", core.RegionToHexMeta(right)),
 			zap.Error(err))
 		return nil, err
 	}
@@ -184,29 +184,27 @@ func (c *RaftCluster) handleReportSplit(request *pdpb.ReportSplitRequest) (*pdpb
 	originRegion.StartKey = left.GetStartKey()
 	log.Info("region split, generate new region",
 		zap.Uint64("region-id", originRegion.GetId()),
-		zap.Reflect("region-meta", core.HexRegionMeta(left)))
+		zap.Stringer("region-meta", core.RegionToHexMeta(left)))
 	return &pdpb.ReportSplitResponse{}, nil
 }
 
 func (c *RaftCluster) handleBatchReportSplit(request *pdpb.ReportBatchSplitRequest) (*pdpb.ReportBatchSplitResponse, error) {
 	regions := request.GetRegions()
-	hexRegionMetas := make([]*metapb.Region, len(regions))
-	for i, region := range regions {
-		hexRegionMetas[i] = core.HexRegionMeta(region)
-	}
 
+	hrm := core.RegionsToHexMeta(regions)
 	err := c.checkSplitRegions(regions)
 	if err != nil {
 		log.Warn("report batch split region is invalid",
-			zap.Reflect("region-meta", hexRegionMetas),
+			zap.Stringer("region-meta", hrm),
 			zap.Error(err))
 		return nil, err
 	}
 	last := len(regions) - 1
 	originRegion := proto.Clone(regions[last]).(*metapb.Region)
+	hrm = core.RegionsToHexMeta(regions[:last])
 	log.Info("region batch split, generate new regions",
 		zap.Uint64("region-id", originRegion.GetId()),
-		zap.Reflect("origin", hexRegionMetas[:last]),
+		zap.Stringer("origin", hrm),
 		zap.Int("total", last))
 	return &pdpb.ReportBatchSplitResponse{}, nil
 }
diff --git a/server/core/region.go b/server/core/region.go
@@ -820,14 +820,49 @@ func HexRegionKey(key []byte) []byte {
 	return []byte(strings.ToUpper(hex.EncodeToString(key)))
 }
 
-// HexRegionMeta converts a region meta's keys to hex format. Used for formating
+// RegionToHexMeta converts a region meta's keys to hex format. Used for formating
 // region in logs.
-func HexRegionMeta(meta *metapb.Region) *metapb.Region {
+func RegionToHexMeta(meta *metapb.Region) HexRegionMeta {
 	if meta == nil {
-		return nil
+		return HexRegionMeta{}
 	}
 	meta = proto.Clone(meta).(*metapb.Region)
 	meta.StartKey = HexRegionKey(meta.StartKey)
 	meta.EndKey = HexRegionKey(meta.EndKey)
-	return meta
+	return HexRegionMeta{meta}
+}
+
+// HexRegionMeta is a region meta in the hex format. Used for formating region in logs.
+type HexRegionMeta struct {
+	*metapb.Region
+}
+
+func (h HexRegionMeta) String() string {
+	return strings.TrimSpace(proto.CompactTextString(h.Region))
+}
+
+// RegionsToHexMeta converts regions' meta keys to hex format. Used for formating
+// region in logs.
+func RegionsToHexMeta(regions []*metapb.Region) HexRegionsMeta {
+	hexRegionMetas := make([]*metapb.Region, len(regions))
+	for i, region := range regions {
+		meta := proto.Clone(region).(*metapb.Region)
+		meta.StartKey = HexRegionKey(meta.StartKey)
+		meta.EndKey = HexRegionKey(meta.EndKey)
+
+		hexRegionMetas[i] = meta
+	}
+	return HexRegionsMeta(hexRegionMetas)
+}
+
+// HexRegionsMeta is a slice of regions' meta in the hex format. Used for formating
+// region in logs.
+type HexRegionsMeta []*metapb.Region
+
+func (h HexRegionsMeta) String() string {
+	var b strings.Builder
+	for _, r := range h {
+		b.WriteString(proto.CompactTextString(r))
+	}
+	return strings.TrimSpace(b.String())
 }
diff --git a/server/core/region_test.go b/server/core/region_test.go
@@ -118,7 +118,7 @@ func (*testRegionKey) TestRegionKey(c *C) {
 	for _, t := range testCase {
 		got, err := strconv.Unquote(t.key)
 		c.Assert(err, IsNil)
-		s := fmt.Sprintln(HexRegionMeta(&metapb.Region{StartKey: []byte(got)}))
+		s := fmt.Sprintln(RegionToHexMeta(&metapb.Region{StartKey: []byte(got)}))
 		c.Assert(strings.Contains(s, t.expect), IsTrue)
 
 		// start key changed

diff --git a/server/core/region_tree.go b/server/core/region_tree.go
@@ -92,8 +92,8 @@ func (t *regionTree) update(region *metapb.Region) []*metapb.Region {
 	for _, item := range overlaps {
 		log.Debug("overlapping region",
 			zap.Uint64("region-id", item.GetId()),
-			zap.Reflect("delete-region", HexRegionMeta(item)),
-			zap.Reflect("update-region", HexRegionMeta(region)))
+			zap.Stringer("delete-region", RegionToHexMeta(item)),
+			zap.Stringer("update-region", RegionToHexMeta(region)))
 		t.tree.Delete(&regionItem{item})
 	}
 

diff --git a/server/schedule/merge_checker.go b/server/schedule/merge_checker.go
@@ -113,7 +113,7 @@ func (m *MergeChecker) Check(region *core.RegionInfo) []*Operator {
 		return nil
 	}
 
-	log.Debug("try to merge region", zap.Reflect("from", core.HexRegionMeta(region.GetMeta())), zap.Reflect("to", core.HexRegionMeta(target.GetMeta())))
+	log.Debug("try to merge region", zap.Stringer("from", core.RegionToHexMeta(region.GetMeta())), zap.Stringer("to", core.RegionToHexMeta(target.GetMeta())))
 	ops, err := CreateMergeRegionOperator("merge-region", m.cluster, region, target, OpMerge)
 	if err != nil {
 		return nil