From 564a9ab7cbbe1763f2dbdbf35d1261f86a5edaa9 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Mon, 21 Oct 2019 11:38:51 +0800 Subject: [PATCH 01/17] restore-util: Implement split/scatter (#274) * implement split/scatter Signed-off-by: 5kbpers * init test Signed-off-by: 5kbpers * redesign output/input of the lib Signed-off-by: 5kbpers * update dependency Signed-off-by: 5kbpers * add commments and more tests Signed-off-by: 5kbpers * add ScanRegions interface to Client Signed-off-by: 5kbpers * fix potential data race Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * Apply suggestions from code review Co-Authored-By: kennytm * Update pkg/restore-util/client.go Co-Authored-By: kennytm * address comments Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * update dependency Signed-off-by: 5kbpers * resolve conflicts Signed-off-by: 5kbpers * fix prefix rewrite Signed-off-by: 5kbpers * add RewriteRule/skip failed scatter region/retry the SplitRegion Signed-off-by: 5kbpers * fix test Signed-off-by: 5kbpers * check if region has peer Signed-off-by: 5kbpers * more logs Signed-off-by: 5kbpers --- client.go | 180 ++++++++++++++++++++++++++++++++++++++ range.go | 92 +++++++++++++++++++ range_test.go | 54 ++++++++++++ split.go | 238 ++++++++++++++++++++++++++++++++++++++++++++++++++ split_test.go | 235 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 799 insertions(+) create mode 100644 client.go create mode 100644 range.go create mode 100644 range_test.go create mode 100644 split.go create mode 100644 split_test.go diff --git a/client.go b/client.go new file mode 100644 index 000000000..fc4ab47d6 --- /dev/null +++ b/client.go @@ -0,0 +1,180 @@ +package restore_util + +import ( + "bytes" + "context" + "sync" + + "github.com/pingcap/errors" + "github.com/pingcap/kvproto/pkg/kvrpcpb" + "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/kvproto/pkg/pdpb" + "github.com/pingcap/kvproto/pkg/tikvpb" + pd "github.com/pingcap/pd/client" + "google.golang.org/grpc" +) + +// Client is a external client used by RegionSplitter. +type Client interface { + // GetStore gets a store by a store id. + GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) + // GetRegion gets a region which includes a specified key. + GetRegion(ctx context.Context, key []byte) (*RegionInfo, error) + // GetRegionByID gets a region by a region id. + GetRegionByID(ctx context.Context, regionID uint64) (*RegionInfo, error) + // SplitRegion splits a region from a key, if key is not included in the region, it will return nil. + // note: the key should not be encoded + SplitRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) + // ScatterRegion scatters a specified region. + ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error + // GetOperator gets the status of operator of the specified region. + GetOperator(ctx context.Context, regionID uint64) (*pdpb.GetOperatorResponse, error) + // ScanRegion gets a list of regions, starts from the region that contains key. + // Limit limits the maximum number of regions returned. + ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*RegionInfo, error) +} + +// pdClient is a wrapper of pd client, can be used by RegionSplitter. +type pdClient struct { + mu sync.Mutex + client pd.Client + storeCache map[uint64]*metapb.Store +} + +// NewClient returns a client used by RegionSplitter. +func NewClient(client pd.Client) Client { + return &pdClient{ + client: client, + storeCache: make(map[uint64]*metapb.Store), + } +} + +func (c *pdClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) { + c.mu.Lock() + defer c.mu.Unlock() + store, ok := c.storeCache[storeID] + if ok { + return store, nil + } + store, err := c.client.GetStore(ctx, storeID) + if err != nil { + return nil, err + } + c.storeCache[storeID] = store + return store, nil + +} + +func (c *pdClient) GetRegion(ctx context.Context, key []byte) (*RegionInfo, error) { + region, leader, err := c.client.GetRegion(ctx, key) + if err != nil { + return nil, err + } + if region == nil { + return nil, errors.Errorf("region not found: key=%x", key) + } + return &RegionInfo{ + Region: region, + Leader: leader, + }, nil +} + +func (c *pdClient) GetRegionByID(ctx context.Context, regionID uint64) (*RegionInfo, error) { + region, leader, err := c.client.GetRegionByID(ctx, regionID) + if err != nil { + return nil, err + } + if region == nil { + return nil, errors.Errorf("region not found: id=%d", regionID) + } + return &RegionInfo{ + Region: region, + Leader: leader, + }, nil +} + +func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) { + var peer *metapb.Peer + if regionInfo.Leader != nil { + peer = regionInfo.Leader + } else { + if len(regionInfo.Region.Peers) == 0 { + return nil, errors.New("region does not have peer") + } + peer = regionInfo.Region.Peers[0] + } + storeID := peer.GetStoreId() + store, err := c.GetStore(ctx, storeID) + if err != nil { + return nil, err + } + conn, err := grpc.Dial(store.GetAddress(), grpc.WithInsecure()) + if err != nil { + return nil, err + } + client := tikvpb.NewTikvClient(conn) + resp, err := client.SplitRegion(ctx, &kvrpcpb.SplitRegionRequest{ + Context: &kvrpcpb.Context{ + RegionId: regionInfo.Region.Id, + RegionEpoch: regionInfo.Region.RegionEpoch, + Peer: peer, + }, + SplitKeys: [][]byte{key}, + }) + if err != nil { + return nil, err + } + if resp.RegionError != nil { + return nil, errors.Errorf("split region failed: region=%v, key=%x, err=%v", regionInfo.Region, key, resp.RegionError) + } + + regions := resp.GetRegions() + var newRegion *metapb.Region + for _, r := range regions { + // Assume the new region is the left one. + if bytes.Equal(r.GetStartKey(), regionInfo.Region.GetStartKey()) { + newRegion = r + break + } + } + if newRegion == nil { + return nil, errors.New("split region failed") + } + var leader *metapb.Peer + // Assume the leaders will be at the same store. + if regionInfo.Leader != nil { + for _, p := range newRegion.GetPeers() { + if p.GetStoreId() == regionInfo.Leader.GetStoreId() { + leader = p + break + } + } + } + return &RegionInfo{ + Region: newRegion, + Leader: leader, + }, nil +} + +func (c *pdClient) ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error { + return c.client.ScatterRegion(ctx, regionInfo.Region.GetId()) +} + +func (c *pdClient) GetOperator(ctx context.Context, regionID uint64) (*pdpb.GetOperatorResponse, error) { + return c.client.GetOperator(ctx, regionID) +} + +func (c *pdClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*RegionInfo, error) { + regions, leaders, err := c.client.ScanRegions(ctx, key, endKey, limit) + if err != nil { + return nil, err + } + regionInfos := make([]*RegionInfo, 0, len(regions)) + for i := range regions { + regionInfos = append(regionInfos, &RegionInfo{ + Region: regions[i], + Leader: leaders[i], + }) + } + return regionInfos, nil +} diff --git a/range.go b/range.go new file mode 100644 index 000000000..4bb0804e5 --- /dev/null +++ b/range.go @@ -0,0 +1,92 @@ +package restore_util + +import ( + "bytes" + "fmt" + + "github.com/google/btree" + "github.com/pingcap/kvproto/pkg/import_sstpb" + "github.com/pingcap/kvproto/pkg/metapb" +) + +// Range represents a range of keys. +type Range struct { + StartKey []byte + EndKey []byte +} + +// String formats a range to a string +func (r *Range) String() string { + return fmt.Sprintf("[%x %x]", r.StartKey, r.EndKey) +} + +// Less compares a range with a btree.Item +func (r *Range) Less(than btree.Item) bool { + t := than.(*Range) + return len(r.EndKey) != 0 && bytes.Compare(r.EndKey, t.StartKey) <= 0 +} + +// contains returns if a key is included in the range. +func (r *Range) contains(key []byte) bool { + start, end := r.StartKey, r.EndKey + return bytes.Compare(key, start) >= 0 && + (len(end) == 0 || bytes.Compare(key, end) < 0) +} + +// RangeTree stores the ranges in an orderly manner. +// All the ranges it stored do not overlap. +type RangeTree struct { + tree *btree.BTree +} + +// NewRangeTree returns a new RangeTree. +func NewRangeTree() *RangeTree { + return &RangeTree{tree: btree.New(32)} +} + +// Find returns nil or a range in the range tree +func (rt *RangeTree) Find(key []byte) *Range { + var ret *Range + r := &Range{ + StartKey: key, + } + rt.tree.DescendLessOrEqual(r, func(i btree.Item) bool { + ret = i.(*Range) + return false + }) + if ret == nil || !ret.contains(key) { + return nil + } + return ret +} + +// InsertRanges inserts ranges into the range tree. +// it returns true if all ranges inserted successfully. +// it returns false if there are some overlapped ranges. +func (rt *RangeTree) InsertRange(rg Range) bool { + return rt.tree.ReplaceOrInsert(&rg) == nil +} + +// RangeIterator allows callers of Ascend to iterate in-order over portions of +// the tree. When this function returns false, iteration will stop and the +// associated Ascend function will immediately return. +type RangeIterator func(rg *Range) bool + +// Ascend calls the iterator for every value in the tree within [first, last], +// until the iterator returns false. +func (rt *RangeTree) Ascend(iterator RangeIterator) { + rt.tree.Ascend(func(i btree.Item) bool { + return iterator(i.(*Range)) + }) +} + +// RegionInfo includes a region and the leader of the region. +type RegionInfo struct { + Region *metapb.Region + Leader *metapb.Peer +} + +type RewriteRules struct { + Table []*import_sstpb.RewriteRule + Data []*import_sstpb.RewriteRule +} diff --git a/range_test.go b/range_test.go new file mode 100644 index 000000000..53ad5f8ce --- /dev/null +++ b/range_test.go @@ -0,0 +1,54 @@ +package restore_util + +import ( + "bytes" + "testing" +) + +func TestRangeTreeNormal(t *testing.T) { + ranges := initRanges() + rangeTree := NewRangeTree() + for _, rg := range ranges { + if !rangeTree.InsertRange(rg) { + t.Fatalf("insert range failed") + } + } + resRanges := make([]Range, 0) + rangeTree.Ascend(func(rg *Range) bool { + if rg == nil { + return false + } + resRanges = append(resRanges, Range{ + StartKey: rg.StartKey, + EndKey: rg.EndKey, + }) + return true + }) + if len(ranges) != len(resRanges) { + t.Fatalf("some inserted ranges missed: rg=%v res=%v", ranges, resRanges) + } + for i, rg := range ranges { + res := resRanges[i] + if !bytes.Equal(rg.StartKey, res.StartKey) || !bytes.Equal(rg.EndKey, res.EndKey) { + t.Fatalf("some inserted ranges missed: rg=%v res=%v", ranges, resRanges) + } + } +} + +func TestRangeOverlapped(t *testing.T) { + rg1 := Range{ + StartKey: []byte("aaa"), + EndKey: []byte("aaz"), + } + rg2 := Range{ + StartKey: []byte("aab"), + EndKey: []byte("aag"), + } + rangeTree := NewRangeTree() + if !rangeTree.InsertRange(rg1) { + t.Fatalf("insert range failed") + } + if rangeTree.InsertRange(rg2) { + t.Fatalf("overlapping not detected") + } +} diff --git a/split.go b/split.go new file mode 100644 index 000000000..c7d37d742 --- /dev/null +++ b/split.go @@ -0,0 +1,238 @@ +package restore_util + +import ( + "bytes" + "context" + "sync" + "time" + + "github.com/pingcap/errors" + "github.com/pingcap/kvproto/pkg/import_sstpb" + "github.com/pingcap/kvproto/pkg/pdpb" + "github.com/pingcap/log" + "github.com/pingcap/tidb/util/codec" + "go.uber.org/zap" +) + +const ( + SplitRetryTimes = 16 + + SplitWaitMaxRetryTimes = 64 + SplitWaitInterval = 8 * time.Millisecond + SplitMaxWaitInterval = time.Second + + ScatterWaitMaxRetryTimes = 128 + ScatterWaitInterval = 50 * time.Millisecond + ScatterMaxWaitInterval = 5 * time.Second +) + +// RegionSplitter is a executor of region split by rules. +type RegionSplitter struct { + client Client + rangeTree *RangeTree +} + +// NewRegionSplitter returns a new RegionSplitter. +func NewRegionSplitter(client Client) *RegionSplitter { + return &RegionSplitter{ + client: client, + } +} + +// Split executes a region split. It will split regions by the rewrite rules, +// then it will split regions by the end key of each range. +// tableRules includes the prefix of a table, since some ranges may have a prefix with record sequence or index sequence. +// note: all ranges and rewrite rules must have raw key. +func (rs *RegionSplitter) Split(ctx context.Context, ranges []Range, rewriteRules *RewriteRules) error { + var wg sync.WaitGroup + rangeTree, ok := newRangeTreeWithRewrite(ranges, rewriteRules) + if !ok { + return errors.Errorf("ranges overlapped: %v", ranges) + } + err := rs.splitByRewriteRules(ctx, &wg, rewriteRules.Data) + if err != nil { + return errors.Trace(err) + } + rangeTree.Ascend(func(rg *Range) bool { + if rg == nil { + return false + } + err = rs.maybeSplitRegion(ctx, rg, &wg) + return err == nil + }) + if err != nil { + return errors.Trace(err) + } + + // Wait for all regions scatter success + wg.Wait() + return nil +} + +// Split regions by the rewrite rules, to ensure all keys of one region only have one prefix. +func (rs *RegionSplitter) splitByRewriteRules(ctx context.Context, wg *sync.WaitGroup, rules []*import_sstpb.RewriteRule) error { + for _, rule := range rules { + err := rs.maybeSplitRegion(ctx, &Range{ + StartKey: rule.GetNewKeyPrefix(), + EndKey: rule.GetNewKeyPrefix(), + }, wg) + if err != nil { + return errors.Trace(err) + } + } + return nil +} + +func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, bool) { + rangeTree := NewRangeTree() + for _, rg := range ranges { + rg.StartKey = replacePrefix(rg.StartKey, rewriteRules) + rg.EndKey = replacePrefix(rg.EndKey, rewriteRules) + if !rangeTree.InsertRange(rg) { + return nil, false + } + } + return rangeTree, true +} + +func (rs *RegionSplitter) hasRegion(ctx context.Context, regionID uint64) (bool, error) { + regionInfo, err := rs.client.GetRegionByID(ctx, regionID) + if err != nil { + return false, err + } + return regionInfo != nil, nil +} + +func (rs *RegionSplitter) isScatterRegionFinished(ctx context.Context, regionID uint64) (bool, error) { + resp, err := rs.client.GetOperator(ctx, regionID) + if err != nil { + return false, err + } + // Heartbeat may not be sent to PD + if resp.GetHeader().GetError().GetType() == pdpb.ErrorType_REGION_NOT_FOUND { + return true, nil + } + // If the current operator of the region is not 'scatter-region', we could assume + // that 'scatter-operator' has finished or timeout + ok := string(resp.GetDesc()) != "scatter-region" || resp.GetStatus() != pdpb.OperatorStatus_RUNNING + return ok, nil +} + +func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) error { + interval := SplitWaitInterval + for i := 0; i < SplitWaitMaxRetryTimes; i++ { + ok, err := rs.hasRegion(ctx, regionID) + if err != nil { + return errors.Trace(err) + } + if ok { + break + } else { + interval = 2 * interval + if interval > SplitMaxWaitInterval { + interval = SplitMaxWaitInterval + } + time.Sleep(interval) + } + } + return nil +} + +func (rs *RegionSplitter) tryToScatterRegion(ctx context.Context, wg *sync.WaitGroup, regionInfo *RegionInfo) { + wg.Add(1) + go func() { + defer wg.Done() + err := rs.client.ScatterRegion(ctx, regionInfo) + if err != nil { + log.Warn("scatter region failed", zap.Error(err), zap.Reflect("region", regionInfo.Region)) + return + } + interval := ScatterWaitInterval + regionID := regionInfo.Region.GetId() + for i := 0; i < ScatterWaitMaxRetryTimes; i++ { + ok, err := rs.hasRegion(ctx, regionID) + if err != nil { + log.Warn("scatter region failed: do not have the region", zap.Reflect("region", regionInfo.Region)) + return + } + if ok { + break + } else { + interval = 2 * interval + if interval > ScatterMaxWaitInterval { + interval = ScatterMaxWaitInterval + } + time.Sleep(interval) + } + } + }() +} + +func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range, wg *sync.WaitGroup) error { + var i int + for { + regionInfo, err := rs.client.GetRegion(ctx, codec.EncodeBytes([]byte{}, r.StartKey)) + if err != nil { + return errors.Trace(err) + } + splitKey := r.EndKey + if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { + return nil + } + err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey, wg) + if err == nil { + return nil + } + i++ + if i >= SplitRetryTimes { + return err + } else { + log.Warn("split region failed, retry it", zap.Error(err), zap.Reflect("region", regionInfo.Region)) + } + } +} + +func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte, wg *sync.WaitGroup) error { + newRegion, err := rs.client.SplitRegion(ctx, regionInfo, key) + if err != nil { + return errors.Trace(err) + } + // Wait for a while until the region successfully splits. + err = rs.waitForSplit(ctx, newRegion.Region.GetId()) + if err != nil { + return errors.Trace(err) + } + rs.tryToScatterRegion(ctx, wg, newRegion) + return err +} + +func needSplit(splitKey []byte, regionInfo *RegionInfo) bool { + // If splitKey is the max key. + if len(splitKey) == 0 { + return false + } + // If splitKey is not in the region or is the boundary of the region. + if bytes.Compare(splitKey, regionInfo.Region.GetStartKey()) <= 0 { + return false + } + return beforeEnd(splitKey, regionInfo.Region.GetEndKey()) +} + +func beforeEnd(key []byte, end []byte) bool { + return bytes.Compare(key, end) < 0 || len(end) == 0 +} + +func replacePrefix(s []byte, rewriteRules *RewriteRules) []byte { + // We should search the dataRules firstly. + for _, rule := range rewriteRules.Data { + if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { + return append(rule.GetNewKeyPrefix(), s[len(rule.GetOldKeyPrefix()):]...) + } + } + for _, rule := range rewriteRules.Table { + if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { + return append(rule.GetNewKeyPrefix(), s[len(rule.GetOldKeyPrefix()):]...) + } + } + return s +} diff --git a/split_test.go b/split_test.go new file mode 100644 index 000000000..3d899b98f --- /dev/null +++ b/split_test.go @@ -0,0 +1,235 @@ +package restore_util + +import ( + "bytes" + "context" + "sync" + "testing" + + "github.com/pingcap/errors" + "github.com/pingcap/kvproto/pkg/import_sstpb" + "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/kvproto/pkg/pdpb" + "github.com/pingcap/tidb/util/codec" +) + +type testClient struct { + mu sync.RWMutex + stores map[uint64]*metapb.Store + regions map[uint64]*RegionInfo + nextRegionID uint64 +} + +func newTestClient(stores map[uint64]*metapb.Store, regions map[uint64]*RegionInfo, nextRegionID uint64) *testClient { + return &testClient{ + stores: stores, + regions: regions, + nextRegionID: nextRegionID, + } +} + +func (c *testClient) GetAllRegions() map[uint64]*RegionInfo { + c.mu.RLock() + defer c.mu.RUnlock() + return c.regions +} + +func (c *testClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) { + c.mu.RLock() + defer c.mu.RUnlock() + store, ok := c.stores[storeID] + if !ok { + return nil, errors.Errorf("store not found") + } + return store, nil +} + +func (c *testClient) GetRegion(ctx context.Context, key []byte) (*RegionInfo, error) { + c.mu.RLock() + defer c.mu.RUnlock() + for _, region := range c.regions { + if bytes.Compare(key, region.Region.StartKey) >= 0 && + (len(region.Region.EndKey) == 0 || bytes.Compare(key, region.Region.EndKey) < 0) { + return region, nil + } + } + return nil, errors.Errorf("region not found: key=%s", string(key)) +} + +func (c *testClient) GetRegionByID(ctx context.Context, regionID uint64) (*RegionInfo, error) { + c.mu.RLock() + defer c.mu.RUnlock() + region, ok := c.regions[regionID] + if !ok { + return nil, errors.Errorf("region not found: id=%d", regionID) + } + return region, nil +} + +func (c *testClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) { + c.mu.Lock() + defer c.mu.Unlock() + var target *RegionInfo + splitKey := codec.EncodeBytes([]byte{}, key) + for _, region := range c.regions { + if bytes.Compare(splitKey, region.Region.StartKey) >= 0 && + (len(region.Region.EndKey) == 0 || bytes.Compare(splitKey, region.Region.EndKey) < 0) { + target = region + } + } + if target == nil { + return nil, errors.Errorf("region not found: key=%s", string(key)) + } + newRegion := &RegionInfo{ + Region: &metapb.Region{ + Peers: target.Region.Peers, + Id: c.nextRegionID, + StartKey: target.Region.StartKey, + EndKey: splitKey, + }, + } + c.regions[c.nextRegionID] = newRegion + c.nextRegionID++ + target.Region.StartKey = splitKey + c.regions[target.Region.Id] = target + return newRegion, nil +} + +func (c *testClient) ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error { + return nil +} + +func (c *testClient) GetOperator(ctx context.Context, regionID uint64) (*pdpb.GetOperatorResponse, error) { + return &pdpb.GetOperatorResponse{ + Header: new(pdpb.ResponseHeader), + }, nil +} + +func (c *testClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*RegionInfo, error) { + return nil, nil +} + +// region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ) +// range: [aaa, aae), [aae, aaz), [ccd, ccf), [ccf, ccj) +// rewrite rules: aa -> xx, cc -> bb +// expected regions after split: +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +func TestSplit(t *testing.T) { + client := initTestClient() + ranges := initRanges() + rewriteRules := initRewriteRules() + regionSplitter := NewRegionSplitter(client) + + ctx := context.Background() + err := regionSplitter.Split(ctx, ranges, rewriteRules) + if err != nil { + t.Fatalf("split regions failed: %v", err) + } + regions := client.GetAllRegions() + if !validateRegions(regions) { + for _, region := range regions { + t.Errorf("region: %v", region.Region) + } + t.Fatalf("get wrong result") + } +} + +// region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ) +func initTestClient() *testClient { + peers := make([]*metapb.Peer, 1) + peers[0] = &metapb.Peer{ + Id: 1, + StoreId: 1, + } + keys := [6]string{"", "aay", "bba", "bbh", "cca", ""} + regions := make(map[uint64]*RegionInfo) + for i := uint64(1); i < 6; i++ { + startKey := []byte(keys[i-1]) + if len(startKey) != 0 { + startKey = codec.EncodeBytes([]byte{}, startKey) + } + endKey := []byte(keys[i]) + if len(endKey) != 0 { + endKey = codec.EncodeBytes([]byte{}, endKey) + } + regions[i] = &RegionInfo{ + Region: &metapb.Region{ + Id: i, + Peers: peers, + StartKey: startKey, + EndKey: endKey, + }, + } + } + stores := make(map[uint64]*metapb.Store) + stores[1] = &metapb.Store{ + Id: 1, + } + return newTestClient(stores, regions, 6) +} + +// range: [aaa, aae), [aae, aaz), [ccd, ccf), [ccf, ccj) +func initRanges() []Range { + var ranges [4]Range + ranges[0] = Range{ + StartKey: []byte("aaa"), + EndKey: []byte("aae"), + } + ranges[1] = Range{ + StartKey: []byte("aae"), + EndKey: []byte("aaz"), + } + ranges[2] = Range{ + StartKey: []byte("ccd"), + EndKey: []byte("ccf"), + } + ranges[3] = Range{ + StartKey: []byte("ccf"), + EndKey: []byte("ccj"), + } + return ranges[:] +} + +func initRewriteRules() *RewriteRules { + var rules [2]*import_sstpb.RewriteRule + rules[0] = &import_sstpb.RewriteRule{ + OldKeyPrefix: []byte("aa"), + NewKeyPrefix: []byte("xx"), + } + rules[1] = &import_sstpb.RewriteRule{ + OldKeyPrefix: []byte("cc"), + NewKeyPrefix: []byte("bb"), + } + return &RewriteRules{ + Table: rules[:], + Data: rules[:], + } +} + +// expected regions after split: +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +func validateRegions(regions map[uint64]*RegionInfo) bool { + keys := [11]string{"", "aay", "bb", "bba", "bbf", "bbh", "cca", "xx", "xxe", "xxz", ""} + if len(regions) != 10 { + return false + } +FindRegion: + for i := 1; i < 11; i++ { + for _, region := range regions { + startKey := []byte(keys[i-1]) + if len(startKey) != 0 { + startKey = codec.EncodeBytes([]byte{}, startKey) + } + endKey := []byte(keys[i]) + if len(endKey) != 0 { + endKey = codec.EncodeBytes([]byte{}, endKey) + } + if bytes.Equal(region.Region.GetStartKey(), startKey) && + bytes.Equal(region.Region.GetEndKey(), endKey) { + continue FindRegion + } + } + return false + } + return true +} From ee6e2189d4a7f0c280e4f17c18f562cd93b6cb39 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Wed, 30 Oct 2019 16:00:21 +0800 Subject: [PATCH 02/17] restore-util: add split retry interval (#277) * reset dependencies to release-3.1 * add split retry interval Signed-off-by: 5kbpers * fix go.sum Signed-off-by: 5kbpers --- client.go | 2 +- split.go | 54 ++++++++++++++++++++++++++++++------------------------ 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/client.go b/client.go index fc4ab47d6..ac4061e36 100644 --- a/client.go +++ b/client.go @@ -119,7 +119,7 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key RegionEpoch: regionInfo.Region.RegionEpoch, Peer: peer, }, - SplitKeys: [][]byte{key}, + SplitKey: key, }) if err != nil { return nil, err diff --git a/split.go b/split.go index c7d37d742..10df7b70a 100644 --- a/split.go +++ b/split.go @@ -15,11 +15,13 @@ import ( ) const ( - SplitRetryTimes = 16 + SplitRetryTimes = 32 + SplitRetryInterval = 50 * time.Millisecond + SplitMaxRetryInterval = time.Second - SplitWaitMaxRetryTimes = 64 - SplitWaitInterval = 8 * time.Millisecond - SplitMaxWaitInterval = time.Second + SplitCheckMaxRetryTimes = 64 + SplitCheckInterval = 8 * time.Millisecond + SplitMaxCheckInterval = time.Second ScatterWaitMaxRetryTimes = 128 ScatterWaitInterval = 50 * time.Millisecond @@ -119,8 +121,8 @@ func (rs *RegionSplitter) isScatterRegionFinished(ctx context.Context, regionID } func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) error { - interval := SplitWaitInterval - for i := 0; i < SplitWaitMaxRetryTimes; i++ { + interval := SplitCheckInterval + for i := 0; i < SplitCheckMaxRetryTimes; i++ { ok, err := rs.hasRegion(ctx, regionID) if err != nil { return errors.Trace(err) @@ -129,8 +131,8 @@ func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) err break } else { interval = 2 * interval - if interval > SplitMaxWaitInterval { - interval = SplitMaxWaitInterval + if interval > SplitMaxCheckInterval { + interval = SplitMaxCheckInterval } time.Sleep(interval) } @@ -169,27 +171,31 @@ func (rs *RegionSplitter) tryToScatterRegion(ctx context.Context, wg *sync.WaitG } func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range, wg *sync.WaitGroup) error { - var i int - for { - regionInfo, err := rs.client.GetRegion(ctx, codec.EncodeBytes([]byte{}, r.StartKey)) - if err != nil { - return errors.Trace(err) + interval := SplitRetryInterval + var err error + for i := 0; i < SplitRetryTimes; i++ { + if i > 0 { + log.Warn("split region failed, retry it", zap.Error(err), zap.Reflect("key", r.StartKey)) } - splitKey := r.EndKey - if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { - return nil - } - err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey, wg) + var regionInfo *RegionInfo + regionInfo, err = rs.client.GetRegion(ctx, codec.EncodeBytes([]byte{}, r.StartKey)) if err == nil { - return nil + splitKey := r.EndKey + if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { + return nil + } + err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey, wg) + if err == nil { + return nil + } } - i++ - if i >= SplitRetryTimes { - return err - } else { - log.Warn("split region failed, retry it", zap.Error(err), zap.Reflect("region", regionInfo.Region)) + interval = 2 * interval + if interval > SplitMaxRetryInterval { + interval = SplitMaxRetryInterval } + time.Sleep(interval) } + return err } func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte, wg *sync.WaitGroup) error { From 97ba30e389f53bcfea20593ef43e8f08db31ec13 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Sat, 2 Nov 2019 14:49:51 +0800 Subject: [PATCH 03/17] restore-util: wait for scatter region sequentially (#279) * wait for scatter region sequentially Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers --- client.go | 3 +- split.go | 90 +++++++++++++++++++++++++++---------------------------- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/client.go b/client.go index ac4061e36..a7158f271 100644 --- a/client.go +++ b/client.go @@ -109,6 +109,7 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key return nil, err } conn, err := grpc.Dial(store.GetAddress(), grpc.WithInsecure()) + defer conn.Close() if err != nil { return nil, err } @@ -138,7 +139,7 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key } } if newRegion == nil { - return nil, errors.New("split region failed") + return nil, errors.New("split region failed: new region is nil") } var leader *metapb.Peer // Assume the leaders will be at the same store. diff --git a/split.go b/split.go index 10df7b70a..868d7b612 100644 --- a/split.go +++ b/split.go @@ -3,7 +3,6 @@ package restore_util import ( "bytes" "context" - "sync" "time" "github.com/pingcap/errors" @@ -46,12 +45,11 @@ func NewRegionSplitter(client Client) *RegionSplitter { // tableRules includes the prefix of a table, since some ranges may have a prefix with record sequence or index sequence. // note: all ranges and rewrite rules must have raw key. func (rs *RegionSplitter) Split(ctx context.Context, ranges []Range, rewriteRules *RewriteRules) error { - var wg sync.WaitGroup rangeTree, ok := newRangeTreeWithRewrite(ranges, rewriteRules) if !ok { return errors.Errorf("ranges overlapped: %v", ranges) } - err := rs.splitByRewriteRules(ctx, &wg, rewriteRules.Data) + scatterRegions, err := rs.splitByRewriteRules(ctx, rewriteRules.Data) if err != nil { return errors.Trace(err) } @@ -59,30 +57,42 @@ func (rs *RegionSplitter) Split(ctx context.Context, ranges []Range, rewriteRule if rg == nil { return false } - err = rs.maybeSplitRegion(ctx, rg, &wg) - return err == nil + var newRegion *RegionInfo + newRegion, err = rs.maybeSplitRegion(ctx, rg) + if err != nil { + return false + } + if newRegion != nil { + scatterRegions = append(scatterRegions, newRegion) + } + return true }) if err != nil { return errors.Trace(err) } - // Wait for all regions scatter success - wg.Wait() + for _, region := range scatterRegions { + rs.waitForScatterRegion(ctx, region) + } return nil } // Split regions by the rewrite rules, to ensure all keys of one region only have one prefix. -func (rs *RegionSplitter) splitByRewriteRules(ctx context.Context, wg *sync.WaitGroup, rules []*import_sstpb.RewriteRule) error { +func (rs *RegionSplitter) splitByRewriteRules(ctx context.Context, rules []*import_sstpb.RewriteRule) ([]*RegionInfo, error) { + scatterRegions := make([]*RegionInfo, 0) for _, rule := range rules { - err := rs.maybeSplitRegion(ctx, &Range{ + newRegion, err := rs.maybeSplitRegion(ctx, &Range{ StartKey: rule.GetNewKeyPrefix(), EndKey: rule.GetNewKeyPrefix(), - }, wg) + }) if err != nil { - return errors.Trace(err) + return nil, errors.Trace(err) + } + if newRegion != nil { + scatterRegions = append(scatterRegions, newRegion) } } - return nil + return scatterRegions, nil } func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, bool) { @@ -140,37 +150,28 @@ func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) err return nil } -func (rs *RegionSplitter) tryToScatterRegion(ctx context.Context, wg *sync.WaitGroup, regionInfo *RegionInfo) { - wg.Add(1) - go func() { - defer wg.Done() - err := rs.client.ScatterRegion(ctx, regionInfo) +func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo *RegionInfo) { + interval := ScatterWaitInterval + regionID := regionInfo.Region.GetId() + for i := 0; i < ScatterWaitMaxRetryTimes; i++ { + ok, err := rs.isScatterRegionFinished(ctx, regionID) if err != nil { - log.Warn("scatter region failed", zap.Error(err), zap.Reflect("region", regionInfo.Region)) + log.Warn("scatter region failed: do not have the region", zap.Reflect("region", regionInfo.Region)) return } - interval := ScatterWaitInterval - regionID := regionInfo.Region.GetId() - for i := 0; i < ScatterWaitMaxRetryTimes; i++ { - ok, err := rs.hasRegion(ctx, regionID) - if err != nil { - log.Warn("scatter region failed: do not have the region", zap.Reflect("region", regionInfo.Region)) - return - } - if ok { - break - } else { - interval = 2 * interval - if interval > ScatterMaxWaitInterval { - interval = ScatterMaxWaitInterval - } - time.Sleep(interval) + if ok { + break + } else { + interval = 2 * interval + if interval > ScatterMaxWaitInterval { + interval = ScatterMaxWaitInterval } + time.Sleep(interval) } - }() + } } -func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range, wg *sync.WaitGroup) error { +func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range) (*RegionInfo, error) { interval := SplitRetryInterval var err error for i := 0; i < SplitRetryTimes; i++ { @@ -182,11 +183,11 @@ func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range, wg *sy if err == nil { splitKey := r.EndKey if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { - return nil + return nil, nil } - err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey, wg) + newRegion, err := rs.splitAndScatterRegion(ctx, regionInfo, splitKey) if err == nil { - return nil + return newRegion, nil } } interval = 2 * interval @@ -195,21 +196,20 @@ func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range, wg *sy } time.Sleep(interval) } - return err + return nil, err } -func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte, wg *sync.WaitGroup) error { +func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) { newRegion, err := rs.client.SplitRegion(ctx, regionInfo, key) if err != nil { - return errors.Trace(err) + return nil, errors.Trace(err) } // Wait for a while until the region successfully splits. err = rs.waitForSplit(ctx, newRegion.Region.GetId()) if err != nil { - return errors.Trace(err) + return nil, errors.Trace(err) } - rs.tryToScatterRegion(ctx, wg, newRegion) - return err + return newRegion, rs.client.ScatterRegion(ctx, regionInfo) } func needSplit(splitKey []byte, regionInfo *RegionInfo) bool { From d5b0016a013a83ce9658223cd150b474f12330f3 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Wed, 6 Nov 2019 11:27:56 +0800 Subject: [PATCH 04/17] restore-util: add on split hook (#281) * restore-util: add on split hook Signed-off-by: Neil Shen * Nil check onSplit Co-Authored-By: kennytm --- split.go | 17 +++++++++++++++-- split_test.go | 12 +++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/split.go b/split.go index 868d7b612..86416c809 100644 --- a/split.go +++ b/split.go @@ -40,11 +40,20 @@ func NewRegionSplitter(client Client) *RegionSplitter { } } +// OnSplitFunc is called before split a range. +type OnSplitFunc func(*Range) + // Split executes a region split. It will split regions by the rewrite rules, // then it will split regions by the end key of each range. -// tableRules includes the prefix of a table, since some ranges may have a prefix with record sequence or index sequence. +// tableRules includes the prefix of a table, since some ranges may have +// a prefix with record sequence or index sequence. // note: all ranges and rewrite rules must have raw key. -func (rs *RegionSplitter) Split(ctx context.Context, ranges []Range, rewriteRules *RewriteRules) error { +func (rs *RegionSplitter) Split( + ctx context.Context, + ranges []Range, + rewriteRules *RewriteRules, + onSplit OnSplitFunc, +) error { rangeTree, ok := newRangeTreeWithRewrite(ranges, rewriteRules) if !ok { return errors.Errorf("ranges overlapped: %v", ranges) @@ -57,6 +66,10 @@ func (rs *RegionSplitter) Split(ctx context.Context, ranges []Range, rewriteRule if rg == nil { return false } + if onSplit != nil { + onSplit(rg) + } + var newRegion *RegionInfo newRegion, err = rs.maybeSplitRegion(ctx, rg) if err != nil { diff --git a/split_test.go b/split_test.go index 3d899b98f..6b8939e39 100644 --- a/split_test.go +++ b/split_test.go @@ -121,7 +121,17 @@ func TestSplit(t *testing.T) { regionSplitter := NewRegionSplitter(client) ctx := context.Background() - err := regionSplitter.Split(ctx, ranges, rewriteRules) + length := 0 + err := regionSplitter.Split(ctx, ranges, rewriteRules, + func(rg *Range) { + length++ + if rg == nil { + t.Fatal("nil Range") + } + }) + if length != 4 { + t.Fatal("wrong length", length) + } if err != nil { t.Fatalf("split regions failed: %v", err) } From 64e57b3d957aa85d9b321467a4be425e01539c51 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Fri, 8 Nov 2019 11:28:53 +0800 Subject: [PATCH 05/17] restore-util: fix returned new region is nil (#283) * restore-util: fix returned new region is nil Signed-off-by: 5kbpers * more logs Signed-off-by: 5kbpers * *: gofmt Signed-off-by: 5kbpers * Apply suggestions from code review Co-Authored-By: kennytm * fix log Signed-off-by: 5kbpers --- client.go | 16 +++++++++------- split.go | 10 ++++++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/client.go b/client.go index a7158f271..3ba93e1c6 100644 --- a/client.go +++ b/client.go @@ -129,13 +129,15 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key return nil, errors.Errorf("split region failed: region=%v, key=%x, err=%v", regionInfo.Region, key, resp.RegionError) } - regions := resp.GetRegions() - var newRegion *metapb.Region - for _, r := range regions { - // Assume the new region is the left one. - if bytes.Equal(r.GetStartKey(), regionInfo.Region.GetStartKey()) { - newRegion = r - break + // Assume the new region is the left one. + newRegion := resp.GetLeft() + if newRegion == nil { + regions := resp.GetRegions() + for _, r := range regions { + if bytes.Equal(r.GetStartKey(), regionInfo.Region.GetStartKey()) { + newRegion = r + break + } } } if newRegion == nil { diff --git a/split.go b/split.go index 86416c809..aeaed68ea 100644 --- a/split.go +++ b/split.go @@ -54,6 +54,7 @@ func (rs *RegionSplitter) Split( rewriteRules *RewriteRules, onSplit OnSplitFunc, ) error { + startTime := time.Now() rangeTree, ok := newRangeTreeWithRewrite(ranges, rewriteRules) if !ok { return errors.Errorf("ranges overlapped: %v", ranges) @@ -83,10 +84,14 @@ func (rs *RegionSplitter) Split( if err != nil { return errors.Trace(err) } - + log.Info("splitting regions done, wait for scattering regions", + zap.Int("regions", len(scatterRegions)), zap.Duration("cost", time.Since(startTime))) + startTime = time.Now() for _, region := range scatterRegions { rs.waitForScatterRegion(ctx, region) } + log.Info("waiting for scattering regions done", + zap.Int("regions", len(scatterRegions)), zap.Duration("cost", time.Since(startTime))) return nil } @@ -198,7 +203,8 @@ func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range) (*Regi if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { return nil, nil } - newRegion, err := rs.splitAndScatterRegion(ctx, regionInfo, splitKey) + var newRegion *RegionInfo + newRegion, err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey) if err == nil { return newRegion, nil } From a32debb8b44160c177805a83d4706723290fc9a5 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Tue, 12 Nov 2019 23:25:16 +0800 Subject: [PATCH 06/17] restore-util: call onSplit on splitByRewriteRules (#285) Signed-off-by: Neil Shen --- split.go | 18 ++++++++++++++---- split_test.go | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/split.go b/split.go index aeaed68ea..6756c7ada 100644 --- a/split.go +++ b/split.go @@ -59,7 +59,8 @@ func (rs *RegionSplitter) Split( if !ok { return errors.Errorf("ranges overlapped: %v", ranges) } - scatterRegions, err := rs.splitByRewriteRules(ctx, rewriteRules.Data) + scatterRegions, err := rs.splitByRewriteRules( + ctx, rewriteRules.Data, onSplit) if err != nil { return errors.Trace(err) } @@ -96,13 +97,22 @@ func (rs *RegionSplitter) Split( } // Split regions by the rewrite rules, to ensure all keys of one region only have one prefix. -func (rs *RegionSplitter) splitByRewriteRules(ctx context.Context, rules []*import_sstpb.RewriteRule) ([]*RegionInfo, error) { +func (rs *RegionSplitter) splitByRewriteRules( + ctx context.Context, + rules []*import_sstpb.RewriteRule, + onSplit OnSplitFunc, +) ([]*RegionInfo, error) { scatterRegions := make([]*RegionInfo, 0) for _, rule := range rules { - newRegion, err := rs.maybeSplitRegion(ctx, &Range{ + rg := Range{ StartKey: rule.GetNewKeyPrefix(), EndKey: rule.GetNewKeyPrefix(), - }) + } + if onSplit != nil { + onSplit(&rg) + } + + newRegion, err := rs.maybeSplitRegion(ctx, &rg) if err != nil { return nil, errors.Trace(err) } diff --git a/split_test.go b/split_test.go index 6b8939e39..89cc52b2e 100644 --- a/split_test.go +++ b/split_test.go @@ -129,7 +129,7 @@ func TestSplit(t *testing.T) { t.Fatal("nil Range") } }) - if length != 4 { + if length != 4+2 { // Ranges + Rules t.Fatal("wrong length", length) } if err != nil { From 72ddd22a8163554ad195937244d3a07e71ecf16f Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Tue, 26 Nov 2019 22:09:29 +0800 Subject: [PATCH 07/17] restore-util: fix overlapped error message (#293) * restore-util: fix overlapped error message Signed-off-by: 5kbpers * fix log message Signed-off-by: 5kbpers * reduce error trace Signed-off-by: 5kbpers * fix test Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers --- range.go | 4 ++-- range_test.go | 12 ++++++------ split.go | 31 ++++++++++++++++--------------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/range.go b/range.go index 4bb0804e5..dfeb2d3fc 100644 --- a/range.go +++ b/range.go @@ -63,8 +63,8 @@ func (rt *RangeTree) Find(key []byte) *Range { // InsertRanges inserts ranges into the range tree. // it returns true if all ranges inserted successfully. // it returns false if there are some overlapped ranges. -func (rt *RangeTree) InsertRange(rg Range) bool { - return rt.tree.ReplaceOrInsert(&rg) == nil +func (rt *RangeTree) InsertRange(rg Range) btree.Item { + return rt.tree.ReplaceOrInsert(&rg) } // RangeIterator allows callers of Ascend to iterate in-order over portions of diff --git a/range_test.go b/range_test.go index 53ad5f8ce..56805d59d 100644 --- a/range_test.go +++ b/range_test.go @@ -9,8 +9,8 @@ func TestRangeTreeNormal(t *testing.T) { ranges := initRanges() rangeTree := NewRangeTree() for _, rg := range ranges { - if !rangeTree.InsertRange(rg) { - t.Fatalf("insert range failed") + if out := rangeTree.InsertRange(rg); out != nil { + t.Fatalf("insert range failed: %s %s", out.(*Range).String(), rg.String()) } } resRanges := make([]Range, 0) @@ -45,10 +45,10 @@ func TestRangeOverlapped(t *testing.T) { EndKey: []byte("aag"), } rangeTree := NewRangeTree() - if !rangeTree.InsertRange(rg1) { - t.Fatalf("insert range failed") + if out := rangeTree.InsertRange(rg1); out != nil { + t.Fatalf("insert range failed: %s %s", out.(*Range).String(), rg1.String()) } - if rangeTree.InsertRange(rg2) { - t.Fatalf("overlapping not detected") + if out := rangeTree.InsertRange(rg2); out == nil { + t.Fatalf("overlapping not detected: %s %s", rg1.String(), rg2.String()) } } diff --git a/split.go b/split.go index 6756c7ada..9912cc784 100644 --- a/split.go +++ b/split.go @@ -55,9 +55,9 @@ func (rs *RegionSplitter) Split( onSplit OnSplitFunc, ) error { startTime := time.Now() - rangeTree, ok := newRangeTreeWithRewrite(ranges, rewriteRules) - if !ok { - return errors.Errorf("ranges overlapped: %v", ranges) + rangeTree, err := newRangeTreeWithRewrite(ranges, rewriteRules) + if err != nil { + return errors.Trace(err) } scatterRegions, err := rs.splitByRewriteRules( ctx, rewriteRules.Data, onSplit) @@ -86,13 +86,13 @@ func (rs *RegionSplitter) Split( return errors.Trace(err) } log.Info("splitting regions done, wait for scattering regions", - zap.Int("regions", len(scatterRegions)), zap.Duration("cost", time.Since(startTime))) + zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) startTime = time.Now() for _, region := range scatterRegions { rs.waitForScatterRegion(ctx, region) } log.Info("waiting for scattering regions done", - zap.Int("regions", len(scatterRegions)), zap.Duration("cost", time.Since(startTime))) + zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) return nil } @@ -114,7 +114,7 @@ func (rs *RegionSplitter) splitByRewriteRules( newRegion, err := rs.maybeSplitRegion(ctx, &rg) if err != nil { - return nil, errors.Trace(err) + return nil, err } if newRegion != nil { scatterRegions = append(scatterRegions, newRegion) @@ -123,16 +123,16 @@ func (rs *RegionSplitter) splitByRewriteRules( return scatterRegions, nil } -func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, bool) { +func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, error) { rangeTree := NewRangeTree() for _, rg := range ranges { rg.StartKey = replacePrefix(rg.StartKey, rewriteRules) rg.EndKey = replacePrefix(rg.EndKey, rewriteRules) - if !rangeTree.InsertRange(rg) { - return nil, false + if out := rangeTree.InsertRange(rg); out != nil { + return nil, errors.Errorf("ranges overlapped: %v, %v", out.(*Range).String(), rg.String()) } } - return rangeTree, true + return rangeTree, nil } func (rs *RegionSplitter) hasRegion(ctx context.Context, regionID uint64) (bool, error) { @@ -203,7 +203,7 @@ func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range) (*Regi interval := SplitRetryInterval var err error for i := 0; i < SplitRetryTimes; i++ { - if i > 0 { + if i > SplitRetryTimes/2 { log.Warn("split region failed, retry it", zap.Error(err), zap.Reflect("key", r.StartKey)) } var regionInfo *RegionInfo @@ -231,12 +231,12 @@ func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range) (*Regi func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) { newRegion, err := rs.client.SplitRegion(ctx, regionInfo, key) if err != nil { - return nil, errors.Trace(err) + return nil, err } // Wait for a while until the region successfully splits. err = rs.waitForSplit(ctx, newRegion.Region.GetId()) if err != nil { - return nil, errors.Trace(err) + return nil, err } return newRegion, rs.client.ScatterRegion(ctx, regionInfo) } @@ -261,13 +261,14 @@ func replacePrefix(s []byte, rewriteRules *RewriteRules) []byte { // We should search the dataRules firstly. for _, rule := range rewriteRules.Data { if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { - return append(rule.GetNewKeyPrefix(), s[len(rule.GetOldKeyPrefix()):]...) + return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...) } } for _, rule := range rewriteRules.Table { if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { - return append(rule.GetNewKeyPrefix(), s[len(rule.GetOldKeyPrefix()):]...) + return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...) } } + return s } From 17315e70d4d1c6193f3b14224be6ce94de33f87e Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Mon, 9 Dec 2019 14:24:50 +0800 Subject: [PATCH 08/17] restore-util: log warning when cannot find matched rewrite rule (#299) --- split.go | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/split.go b/split.go index 9912cc784..b3578c73f 100644 --- a/split.go +++ b/split.go @@ -126,8 +126,28 @@ func (rs *RegionSplitter) splitByRewriteRules( func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, error) { rangeTree := NewRangeTree() for _, rg := range ranges { - rg.StartKey = replacePrefix(rg.StartKey, rewriteRules) - rg.EndKey = replacePrefix(rg.EndKey, rewriteRules) + if rewriteRules != nil { + var startRule *import_sstpb.RewriteRule + rg.StartKey, startRule = replacePrefix(rg.StartKey, rewriteRules) + if startRule != nil { + log.Warn("cannot find rewrite rule", zap.ByteString("key", rg.StartKey)) + } else { + log.Debug( + "rewrite start key", + zap.ByteString("key", rg.StartKey), + zap.Stringer("rule", startRule)) + } + var endRule *import_sstpb.RewriteRule + rg.EndKey, endRule = replacePrefix(rg.EndKey, rewriteRules) + if endRule != nil { + log.Warn("cannot find rewrite rule", zap.ByteString("key", rg.EndKey)) + } else { + log.Debug( + "rewrite end key", + zap.ByteString("key", rg.EndKey), + zap.Stringer("rule", endRule)) + } + } if out := rangeTree.InsertRange(rg); out != nil { return nil, errors.Errorf("ranges overlapped: %v, %v", out.(*Range).String(), rg.String()) } @@ -257,18 +277,18 @@ func beforeEnd(key []byte, end []byte) bool { return bytes.Compare(key, end) < 0 || len(end) == 0 } -func replacePrefix(s []byte, rewriteRules *RewriteRules) []byte { +func replacePrefix(s []byte, rewriteRules *RewriteRules) ([]byte, *import_sstpb.RewriteRule) { // We should search the dataRules firstly. for _, rule := range rewriteRules.Data { if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { - return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...) + return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...), rule } } for _, rule := range rewriteRules.Table { if bytes.HasPrefix(s, rule.GetOldKeyPrefix()) { - return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...) + return append(append([]byte{}, rule.GetNewKeyPrefix()...), s[len(rule.GetOldKeyPrefix()):]...), rule } } - return s + return s, nil } From 9f9465b43cc3f28b572af4ed753f8d9e3f094013 Mon Sep 17 00:00:00 2001 From: disksing Date: Tue, 17 Dec 2019 12:44:32 +0800 Subject: [PATCH 09/17] restore-util: add method to set placement rules and store labels (#301) * restore-util: add method to set placement rules and store labels Signed-off-by: disksing * minor fix Signed-off-by: disksing * address comment Signed-off-by: disksing * add GetPlacementRules Signed-off-by: disksing * fix test Signed-off-by: disksing --- client.go | 100 +++++++++++++++++++++++++++++++++++++++++++++++++- split_test.go | 17 +++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index 3ba93e1c6..106d6a26b 100644 --- a/client.go +++ b/client.go @@ -3,6 +3,13 @@ package restore_util import ( "bytes" "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "path" + "strconv" + "strings" "sync" "github.com/pingcap/errors" @@ -11,10 +18,11 @@ import ( "github.com/pingcap/kvproto/pkg/pdpb" "github.com/pingcap/kvproto/pkg/tikvpb" pd "github.com/pingcap/pd/client" + "github.com/pingcap/pd/server/schedule/placement" "google.golang.org/grpc" ) -// Client is a external client used by RegionSplitter. +// Client is an external client used by RegionSplitter. type Client interface { // GetStore gets a store by a store id. GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) @@ -32,6 +40,15 @@ type Client interface { // ScanRegion gets a list of regions, starts from the region that contains key. // Limit limits the maximum number of regions returned. ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*RegionInfo, error) + // GetPlacementRule loads a placement rule from PD. + GetPlacementRule(ctx context.Context, groupID, ruleID string) (placement.Rule, error) + // SetPlacementRule insert or update a placement rule to PD. + SetPlacementRule(ctx context.Context, rule placement.Rule) error + // DeletePlacementRule removes a placement rule from PD. + DeletePlacementRule(ctx context.Context, groupID, ruleID string) error + // SetStoreLabel add or update specified label of stores. If labelValue + // is empty, it clears the label. + SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error } // pdClient is a wrapper of pd client, can be used by RegionSplitter. @@ -181,3 +198,84 @@ func (c *pdClient) ScanRegions(ctx context.Context, key, endKey []byte, limit in } return regionInfos, nil } + +func (c *pdClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) (placement.Rule, error) { + var rule placement.Rule + addr := c.getPDAPIAddr() + if addr == "" { + return rule, errors.New("failed to add stores labels: no leader") + } + req, _ := http.NewRequestWithContext(ctx, "GET", path.Join(addr, "pd/api/v1/config/rule", groupID, ruleID), nil) + res, err := http.DefaultClient.Do(req) + if err != nil { + return rule, errors.WithStack(err) + } + b, err := ioutil.ReadAll(res.Body) + if err != nil { + return rule, errors.WithStack(err) + } + res.Body.Close() + err = json.Unmarshal(b, &rule) + if err != nil { + return rule, errors.WithStack(err) + } + return rule, nil +} + +func (c *pdClient) SetPlacementRule(ctx context.Context, rule placement.Rule) error { + addr := c.getPDAPIAddr() + if addr == "" { + return errors.New("failed to add stores labels: no leader") + } + m, _ := json.Marshal(rule) + req, _ := http.NewRequestWithContext(ctx, "POST", path.Join(addr, "pd/api/v1/config/rule"), bytes.NewReader(m)) + res, err := http.DefaultClient.Do(req) + if err != nil { + return errors.WithStack(err) + } + ioutil.ReadAll(res.Body) + res.Body.Close() + return nil +} + +func (c *pdClient) DeletePlacementRule(ctx context.Context, groupID, ruleID string) error { + addr := c.getPDAPIAddr() + if addr == "" { + return errors.New("failed to add stores labels: no leader") + } + req, _ := http.NewRequestWithContext(ctx, "DELETE", path.Join(addr, "pd/api/v1/config/rule", groupID, ruleID), nil) + res, err := http.DefaultClient.Do(req) + if err != nil { + return errors.WithStack(err) + } + ioutil.ReadAll(res.Body) + res.Body.Close() + return nil +} + +func (c *pdClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error { + b := []byte(fmt.Sprintf(`{"%s": "%s"}`, labelKey, labelValue)) + addr := c.getPDAPIAddr() + if addr == "" { + return errors.New("failed to add stores labels: no leader") + } + for _, id := range stores { + req, _ := http.NewRequestWithContext(ctx, "POST", path.Join(addr, "pd/api/v1/store", strconv.FormatUint(id, 10), "label"), bytes.NewReader(b)) + res, err := http.DefaultClient.Do(req) + if err != nil { + return errors.WithStack(err) + } + ioutil.ReadAll(res.Body) + res.Body.Close() + + } + return nil +} + +func (c *pdClient) getPDAPIAddr() string { + addr := c.client.GetLeaderAddr() + if addr != "" && !strings.HasPrefix(addr, "http") { + addr = "http://" + addr + } + return addr +} diff --git a/split_test.go b/split_test.go index 89cc52b2e..08d580d2b 100644 --- a/split_test.go +++ b/split_test.go @@ -10,6 +10,7 @@ import ( "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/kvproto/pkg/pdpb" + "github.com/pingcap/pd/server/schedule/placement" "github.com/pingcap/tidb/util/codec" ) @@ -109,6 +110,22 @@ func (c *testClient) ScanRegions(ctx context.Context, key, endKey []byte, limit return nil, nil } +func (c *testClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) (r placement.Rule, err error) { + return +} + +func (c *testClient) SetPlacementRule(ctx context.Context, rule placement.Rule) error { + return nil +} + +func (c *testClient) DeletePlacementRule(ctx context.Context, groupID, ruleID string) error { + return nil +} + +func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error { + return nil +} + // region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ) // range: [aaa, aae), [aae, aaz), [ccd, ccf), [ccf, ccj) // rewrite rules: aa -> xx, cc -> bb From b9ac4580c96096f12ae66d4c803487d3aa3cbb54 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Wed, 18 Dec 2019 20:47:33 +0800 Subject: [PATCH 10/17] restore-util: support batch split (#300) * restore-util: support batch split Signed-off-by: 5kbpers * go fmt Signed-off-by: 5kbpers * Apply suggestions from code review Co-Authored-By: kennytm * address commits Signed-off-by: 5kbpers * Update pkg/restore-util/split.go Co-Authored-By: kennytm * add onSplit callback Signed-off-by: 5kbpers * fix test Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers --- client.go | 65 +++++++++++++++ range.go | 44 ++++++++++ split.go | 222 +++++++++++++++++++++++--------------------------- split_test.go | 68 ++++++++++++---- 4 files changed, 263 insertions(+), 136 deletions(-) diff --git a/client.go b/client.go index 106d6a26b..f70cb4060 100644 --- a/client.go +++ b/client.go @@ -33,6 +33,9 @@ type Client interface { // SplitRegion splits a region from a key, if key is not included in the region, it will return nil. // note: the key should not be encoded SplitRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) + // BatchSplitRegions splits a region from a batch of keys. + // note: the keys should not be encoded + BatchSplitRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) // ScatterRegion scatters a specified region. ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error // GetOperator gets the status of operator of the specified region. @@ -176,6 +179,68 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key }, nil } +func (c *pdClient) BatchSplitRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { + var peer *metapb.Peer + if regionInfo.Leader != nil { + peer = regionInfo.Leader + } else { + if len(regionInfo.Region.Peers) == 0 { + return nil, errors.New("region does not have peer") + } + peer = regionInfo.Region.Peers[0] + } + + storeID := peer.GetStoreId() + store, err := c.GetStore(ctx, storeID) + if err != nil { + return nil, err + } + conn, err := grpc.Dial(store.GetAddress(), grpc.WithInsecure()) + if err != nil { + return nil, err + } + defer conn.Close() + client := tikvpb.NewTikvClient(conn) + resp, err := client.SplitRegion(ctx, &kvrpcpb.SplitRegionRequest{ + Context: &kvrpcpb.Context{ + RegionId: regionInfo.Region.Id, + RegionEpoch: regionInfo.Region.RegionEpoch, + Peer: peer, + }, + SplitKeys: keys, + }) + if err != nil { + return nil, err + } + if resp.RegionError != nil { + return nil, errors.Errorf("split region failed: region=%v, err=%v", regionInfo.Region, resp.RegionError) + } + + regions := resp.GetRegions() + newRegionInfos := make([]*RegionInfo, 0, len(regions)) + for _, region := range regions { + // Skip the original region + if region.GetId() == regionInfo.Region.GetId() { + continue + } + var leader *metapb.Peer + // Assume the leaders will be at the same store. + if regionInfo.Leader != nil { + for _, p := range region.GetPeers() { + if p.GetStoreId() == regionInfo.Leader.GetStoreId() { + leader = p + break + } + } + } + newRegionInfos = append(newRegionInfos, &RegionInfo{ + Region: region, + Leader: leader, + }) + } + return newRegionInfos, nil +} + func (c *pdClient) ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error { return c.client.ScatterRegion(ctx, regionInfo.Region.GetId()) } diff --git a/range.go b/range.go index dfeb2d3fc..9eba63dc6 100644 --- a/range.go +++ b/range.go @@ -5,8 +5,11 @@ import ( "fmt" "github.com/google/btree" + "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/log" + "go.uber.org/zap" ) // Range represents a range of keys. @@ -33,6 +36,47 @@ func (r *Range) contains(key []byte) bool { (len(end) == 0 || bytes.Compare(key, end) < 0) } +// sortRanges checks if the range overlapped and sort them +func sortRanges(ranges []Range, rewriteRules *RewriteRules) ([]Range, error) { + rangeTree := NewRangeTree() + for _, rg := range ranges { + if rewriteRules != nil { + var startRule *import_sstpb.RewriteRule + rg.StartKey, startRule = replacePrefix(rg.StartKey, rewriteRules) + if startRule == nil { + log.Warn("cannot find rewrite rule", zap.Binary("key", rg.StartKey)) + } else { + log.Debug( + "rewrite start key", + zap.Binary("key", rg.StartKey), + zap.Stringer("rule", startRule)) + } + var endRule *import_sstpb.RewriteRule + rg.EndKey, endRule = replacePrefix(rg.EndKey, rewriteRules) + if endRule == nil { + log.Warn("cannot find rewrite rule", zap.Binary("key", rg.EndKey)) + } else { + log.Debug( + "rewrite end key", + zap.Binary("key", rg.EndKey), + zap.Stringer("rule", endRule)) + } + } + if out := rangeTree.InsertRange(rg); out != nil { + return nil, errors.Errorf("ranges overlapped: %s, %s", out, rg) + } + } + sortedRanges := make([]Range, 0, len(ranges)) + rangeTree.Ascend(func(rg *Range) bool { + if rg == nil { + return false + } + sortedRanges = append(sortedRanges, *rg) + return true + }) + return sortedRanges, nil +} + // RangeTree stores the ranges in an orderly manner. // All the ranges it stored do not overlap. type RangeTree struct { diff --git a/split.go b/split.go index b3578c73f..373c31b35 100644 --- a/split.go +++ b/split.go @@ -41,7 +41,7 @@ func NewRegionSplitter(client Client) *RegionSplitter { } // OnSplitFunc is called before split a range. -type OnSplitFunc func(*Range) +type OnSplitFunc func(key [][]byte) // Split executes a region split. It will split regions by the rewrite rules, // then it will split regions by the end key of each range. @@ -54,36 +54,69 @@ func (rs *RegionSplitter) Split( rewriteRules *RewriteRules, onSplit OnSplitFunc, ) error { + if len(ranges) == 0 { + return nil + } startTime := time.Now() - rangeTree, err := newRangeTreeWithRewrite(ranges, rewriteRules) + // Sort the range for getting the min and max key of the ranges + sortedRanges, err := sortRanges(ranges, rewriteRules) if err != nil { return errors.Trace(err) } - scatterRegions, err := rs.splitByRewriteRules( - ctx, rewriteRules.Data, onSplit) - if err != nil { - return errors.Trace(err) + minKey := codec.EncodeBytes([]byte{}, sortedRanges[0].StartKey) + maxKey := codec.EncodeBytes([]byte{}, sortedRanges[len(sortedRanges)-1].EndKey) + for _, rule := range rewriteRules.Table { + if bytes.Compare(minKey, rule.GetNewKeyPrefix()) > 0 { + minKey = rule.GetNewKeyPrefix() + } + if bytes.Compare(maxKey, rule.GetNewKeyPrefix()) < 0 { + maxKey = rule.GetNewKeyPrefix() + } } - rangeTree.Ascend(func(rg *Range) bool { - if rg == nil { - return false + for _, rule := range rewriteRules.Data { + if bytes.Compare(minKey, rule.GetNewKeyPrefix()) > 0 { + minKey = rule.GetNewKeyPrefix() } - if onSplit != nil { - onSplit(rg) + if bytes.Compare(maxKey, rule.GetNewKeyPrefix()) < 0 { + maxKey = rule.GetNewKeyPrefix() } - - var newRegion *RegionInfo - newRegion, err = rs.maybeSplitRegion(ctx, rg) + } + interval := SplitRetryInterval + var scatterRegions []*RegionInfo +SplitRegions: + for i := 0; i < SplitRetryTimes; i++ { + regions, err := rs.client.ScanRegions(ctx, minKey, maxKey, 0) if err != nil { - return false + return errors.Trace(err) } - if newRegion != nil { - scatterRegions = append(scatterRegions, newRegion) + if len(regions) == 0 { + log.Warn("cannot scan any region") + return nil } - return true - }) - if err != nil { - return errors.Trace(err) + splitKeyMap := getSplitKeys(rewriteRules, sortedRanges, regions) + regionMap := make(map[uint64]*RegionInfo) + for _, region := range regions { + regionMap[region.Region.GetId()] = region + } + scatterRegions = make([]*RegionInfo, 0) + for regionID, keys := range splitKeyMap { + var newRegions []*RegionInfo + newRegions, err = rs.splitAndScatterRegions(ctx, regionMap[regionID], keys) + if err != nil { + interval = 2 * interval + if interval > SplitMaxRetryInterval { + interval = SplitMaxRetryInterval + } + time.Sleep(interval) + if i > 3 { + log.Warn("splitting regions failed, retry it", zap.Error(err)) + } + continue SplitRegions + } + onSplit(keys) + scatterRegions = append(scatterRegions, newRegions...) + } + break } log.Info("splitting regions done, wait for scattering regions", zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) @@ -96,65 +129,6 @@ func (rs *RegionSplitter) Split( return nil } -// Split regions by the rewrite rules, to ensure all keys of one region only have one prefix. -func (rs *RegionSplitter) splitByRewriteRules( - ctx context.Context, - rules []*import_sstpb.RewriteRule, - onSplit OnSplitFunc, -) ([]*RegionInfo, error) { - scatterRegions := make([]*RegionInfo, 0) - for _, rule := range rules { - rg := Range{ - StartKey: rule.GetNewKeyPrefix(), - EndKey: rule.GetNewKeyPrefix(), - } - if onSplit != nil { - onSplit(&rg) - } - - newRegion, err := rs.maybeSplitRegion(ctx, &rg) - if err != nil { - return nil, err - } - if newRegion != nil { - scatterRegions = append(scatterRegions, newRegion) - } - } - return scatterRegions, nil -} - -func newRangeTreeWithRewrite(ranges []Range, rewriteRules *RewriteRules) (*RangeTree, error) { - rangeTree := NewRangeTree() - for _, rg := range ranges { - if rewriteRules != nil { - var startRule *import_sstpb.RewriteRule - rg.StartKey, startRule = replacePrefix(rg.StartKey, rewriteRules) - if startRule != nil { - log.Warn("cannot find rewrite rule", zap.ByteString("key", rg.StartKey)) - } else { - log.Debug( - "rewrite start key", - zap.ByteString("key", rg.StartKey), - zap.Stringer("rule", startRule)) - } - var endRule *import_sstpb.RewriteRule - rg.EndKey, endRule = replacePrefix(rg.EndKey, rewriteRules) - if endRule != nil { - log.Warn("cannot find rewrite rule", zap.ByteString("key", rg.EndKey)) - } else { - log.Debug( - "rewrite end key", - zap.ByteString("key", rg.EndKey), - zap.Stringer("rule", endRule)) - } - } - if out := rangeTree.InsertRange(rg); out != nil { - return nil, errors.Errorf("ranges overlapped: %v, %v", out.(*Range).String(), rg.String()) - } - } - return rangeTree, nil -} - func (rs *RegionSplitter) hasRegion(ctx context.Context, regionID uint64) (bool, error) { regionInfo, err := rs.client.GetRegionByID(ctx, regionID) if err != nil { @@ -219,58 +193,68 @@ func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo * } } -func (rs *RegionSplitter) maybeSplitRegion(ctx context.Context, r *Range) (*RegionInfo, error) { - interval := SplitRetryInterval - var err error - for i := 0; i < SplitRetryTimes; i++ { - if i > SplitRetryTimes/2 { - log.Warn("split region failed, retry it", zap.Error(err), zap.Reflect("key", r.StartKey)) - } - var regionInfo *RegionInfo - regionInfo, err = rs.client.GetRegion(ctx, codec.EncodeBytes([]byte{}, r.StartKey)) - if err == nil { - splitKey := r.EndKey - if !needSplit(codec.EncodeBytes([]byte{}, splitKey), regionInfo) { - return nil, nil - } - var newRegion *RegionInfo - newRegion, err = rs.splitAndScatterRegion(ctx, regionInfo, splitKey) - if err == nil { - return newRegion, nil - } +func (rs *RegionSplitter) splitAndScatterRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { + newRegions, err := rs.client.BatchSplitRegions(ctx, regionInfo, keys) + if err != nil { + return nil, err + } + for _, region := range newRegions { + // Wait for a while until the regions successfully splits. + err = rs.waitForSplit(ctx, region.Region.Id) + if err != nil { + return nil, err } - interval = 2 * interval - if interval > SplitMaxRetryInterval { - interval = SplitMaxRetryInterval + if err = rs.client.ScatterRegion(ctx, region); err != nil { + return nil, err } - time.Sleep(interval) } - return nil, err + return newRegions, nil } -func (rs *RegionSplitter) splitAndScatterRegion(ctx context.Context, regionInfo *RegionInfo, key []byte) (*RegionInfo, error) { - newRegion, err := rs.client.SplitRegion(ctx, regionInfo, key) - if err != nil { - return nil, err +// getSplitKeys checks if the regions should be split by the new prefix of the rewrites rule and the end key of +// the ranges, groups the split keys by region id +func getSplitKeys(rewriteRules *RewriteRules, ranges []Range, regions []*RegionInfo) map[uint64][][]byte { + splitKeyMap := make(map[uint64][][]byte) + checkKeys := make([][]byte, 0) + for _, rule := range rewriteRules.Table { + checkKeys = append(checkKeys, rule.GetNewKeyPrefix()) } - // Wait for a while until the region successfully splits. - err = rs.waitForSplit(ctx, newRegion.Region.GetId()) - if err != nil { - return nil, err + for _, rule := range rewriteRules.Data { + checkKeys = append(checkKeys, rule.GetNewKeyPrefix()) } - return newRegion, rs.client.ScatterRegion(ctx, regionInfo) + for _, rg := range ranges { + checkKeys = append(checkKeys, rg.EndKey) + } + for _, key := range checkKeys { + if region := needSplit(key, regions); region != nil { + splitKeys, ok := splitKeyMap[region.Region.GetId()] + if !ok { + splitKeys = make([][]byte, 0, 1) + } + splitKeyMap[region.Region.GetId()] = append(splitKeys, key) + } + } + return splitKeyMap } -func needSplit(splitKey []byte, regionInfo *RegionInfo) bool { +// needSplit checks whether a key is necessary to split, if true returns the split region +func needSplit(splitKey []byte, regions []*RegionInfo) *RegionInfo { // If splitKey is the max key. if len(splitKey) == 0 { - return false + return nil } - // If splitKey is not in the region or is the boundary of the region. - if bytes.Compare(splitKey, regionInfo.Region.GetStartKey()) <= 0 { - return false + splitKey = codec.EncodeBytes([]byte{}, splitKey) + for _, region := range regions { + // If splitKey is the boundary of the region + if bytes.Compare(splitKey, region.Region.GetStartKey()) == 0 { + return nil + } + // If splitKey is in a region + if bytes.Compare(splitKey, region.Region.GetStartKey()) > 0 && beforeEnd(splitKey, region.Region.GetEndKey()) { + return region + } } - return beforeEnd(splitKey, regionInfo.Region.GetEndKey()) + return nil } func beforeEnd(key []byte, end []byte) bool { diff --git a/split_test.go b/split_test.go index 08d580d2b..e5a5b8e19 100644 --- a/split_test.go +++ b/split_test.go @@ -96,6 +96,39 @@ func (c *testClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, ke return newRegion, nil } +func (c *testClient) BatchSplitRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { + c.mu.Lock() + defer c.mu.Unlock() + newRegions := make([]*RegionInfo, 0) + for _, key := range keys { + var target *RegionInfo + splitKey := codec.EncodeBytes([]byte{}, key) + for _, region := range c.regions { + if bytes.Compare(splitKey, region.Region.GetStartKey()) > 0 && + beforeEnd(splitKey, region.Region.GetEndKey()) { + target = region + } + } + if target == nil { + continue + } + newRegion := &RegionInfo{ + Region: &metapb.Region{ + Peers: target.Region.Peers, + Id: c.nextRegionID, + StartKey: target.Region.StartKey, + EndKey: splitKey, + }, + } + c.regions[c.nextRegionID] = newRegion + c.nextRegionID++ + target.Region.StartKey = splitKey + c.regions[target.Region.Id] = target + newRegions = append(newRegions, newRegion) + } + return newRegions, nil +} + func (c *testClient) ScatterRegion(ctx context.Context, regionInfo *RegionInfo) error { return nil } @@ -107,7 +140,18 @@ func (c *testClient) GetOperator(ctx context.Context, regionID uint64) (*pdpb.Ge } func (c *testClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*RegionInfo, error) { - return nil, nil + regions := make([]*RegionInfo, 0) + for _, region := range c.regions { + if limit > 0 && len(regions) >= limit { + break + } + if (len(region.Region.GetEndKey()) != 0 && bytes.Compare(region.Region.GetEndKey(), key) <= 0) || + bytes.Compare(region.Region.GetStartKey(), endKey) > 0 { + continue + } + regions = append(regions, region) + } + return regions, nil } func (c *testClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) (r placement.Rule, err error) { @@ -130,7 +174,7 @@ func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelK // range: [aaa, aae), [aae, aaz), [ccd, ccf), [ccf, ccj) // rewrite rules: aa -> xx, cc -> bb // expected regions after split: -// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) func TestSplit(t *testing.T) { client := initTestClient() ranges := initRanges() @@ -138,17 +182,7 @@ func TestSplit(t *testing.T) { regionSplitter := NewRegionSplitter(client) ctx := context.Background() - length := 0 - err := regionSplitter.Split(ctx, ranges, rewriteRules, - func(rg *Range) { - length++ - if rg == nil { - t.Fatal("nil Range") - } - }) - if length != 4+2 { // Ranges + Rules - t.Fatal("wrong length", length) - } + err := regionSplitter.Split(ctx, ranges, rewriteRules, func(key [][]byte) {}) if err != nil { t.Fatalf("split regions failed: %v", err) } @@ -234,14 +268,14 @@ func initRewriteRules() *RewriteRules { } // expected regions after split: -// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) func validateRegions(regions map[uint64]*RegionInfo) bool { - keys := [11]string{"", "aay", "bb", "bba", "bbf", "bbh", "cca", "xx", "xxe", "xxz", ""} - if len(regions) != 10 { + keys := [12]string{"", "aay", "bb", "bba", "bbf", "bbh", "bbj", "cca", "xx", "xxe", "xxz", ""} + if len(regions) != 11 { return false } FindRegion: - for i := 1; i < 11; i++ { + for i := 1; i < 12; i++ { for _, region := range regions { startKey := []byte(keys[i-1]) if len(startKey) != 0 { From d1462acc5f30a3870c2a287650c9770bd4a34661 Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Mon, 23 Dec 2019 14:43:26 +0800 Subject: [PATCH 11/17] restore-util: add upper bound time for waiting for scatter (#305) * restore: fix scatter regions failed Signed-off-by: 5kbpers * add log Signed-off-by: 5kbpers * stop waiting for scatter after 3min Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers --- client.go | 4 ++-- split.go | 59 ++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/client.go b/client.go index f70cb4060..ab03ad84f 100644 --- a/client.go +++ b/client.go @@ -91,7 +91,7 @@ func (c *pdClient) GetRegion(ctx context.Context, key []byte) (*RegionInfo, erro return nil, err } if region == nil { - return nil, errors.Errorf("region not found: key=%x", key) + return nil, nil } return &RegionInfo{ Region: region, @@ -105,7 +105,7 @@ func (c *pdClient) GetRegionByID(ctx context.Context, regionID uint64) (*RegionI return nil, err } if region == nil { - return nil, errors.Errorf("region not found: id=%d", regionID) + return nil, nil } return &RegionInfo{ Region: region, diff --git a/split.go b/split.go index 373c31b35..6b0415aa1 100644 --- a/split.go +++ b/split.go @@ -22,9 +22,11 @@ const ( SplitCheckInterval = 8 * time.Millisecond SplitMaxCheckInterval = time.Second - ScatterWaitMaxRetryTimes = 128 + ScatterWaitMaxRetryTimes = 64 ScatterWaitInterval = 50 * time.Millisecond - ScatterMaxWaitInterval = 5 * time.Second + ScatterMaxWaitInterval = time.Second + + ScatterWaitUpperInterval = 180 * time.Second ) // RegionSplitter is a executor of region split by rules. @@ -82,10 +84,11 @@ func (rs *RegionSplitter) Split( } } interval := SplitRetryInterval - var scatterRegions []*RegionInfo + scatterRegions := make([]*RegionInfo, 0) SplitRegions: for i := 0; i < SplitRetryTimes; i++ { - regions, err := rs.client.ScanRegions(ctx, minKey, maxKey, 0) + var regions []*RegionInfo + regions, err = rs.client.ScanRegions(ctx, minKey, maxKey, 0) if err != nil { return errors.Trace(err) } @@ -98,7 +101,6 @@ SplitRegions: for _, region := range regions { regionMap[region.Region.GetId()] = region } - scatterRegions = make([]*RegionInfo, 0) for regionID, keys := range splitKeyMap { var newRegions []*RegionInfo newRegions, err = rs.splitAndScatterRegions(ctx, regionMap[regionID], keys) @@ -113,19 +115,34 @@ SplitRegions: } continue SplitRegions } - onSplit(keys) scatterRegions = append(scatterRegions, newRegions...) + onSplit(keys) } break } + if err != nil { + return errors.Trace(err) + } log.Info("splitting regions done, wait for scattering regions", zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) startTime = time.Now() + scatterCount := 0 for _, region := range scatterRegions { rs.waitForScatterRegion(ctx, region) + if time.Since(startTime) > ScatterWaitUpperInterval { + break + } + scatterCount++ + } + if scatterCount == len(scatterRegions) { + log.Info("waiting for scattering regions done", + zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) + } else { + log.Warn("waiting for scattering regions timeout", + zap.Int("scatterCount", scatterCount), + zap.Int("regions", len(scatterRegions)), + zap.Duration("take", time.Since(startTime))) } - log.Info("waiting for scattering regions done", - zap.Int("regions", len(scatterRegions)), zap.Duration("take", time.Since(startTime))) return nil } @@ -143,8 +160,15 @@ func (rs *RegionSplitter) isScatterRegionFinished(ctx context.Context, regionID return false, err } // Heartbeat may not be sent to PD - if resp.GetHeader().GetError().GetType() == pdpb.ErrorType_REGION_NOT_FOUND { - return true, nil + if respErr := resp.GetHeader().GetError(); respErr != nil { + if respErr.GetType() == pdpb.ErrorType_REGION_NOT_FOUND { + return true, nil + } + return false, errors.Errorf("get operator error: %s", respErr.GetType()) + } + retryTimes := ctx.Value("retryTimes").(int) + if retryTimes > 3 { + log.Warn("get operator", zap.Uint64("regionID", regionID), zap.Stringer("resp", resp)) } // If the current operator of the region is not 'scatter-region', we could assume // that 'scatter-operator' has finished or timeout @@ -152,12 +176,13 @@ func (rs *RegionSplitter) isScatterRegionFinished(ctx context.Context, regionID return ok, nil } -func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) error { +func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) { interval := SplitCheckInterval for i := 0; i < SplitCheckMaxRetryTimes; i++ { ok, err := rs.hasRegion(ctx, regionID) if err != nil { - return errors.Trace(err) + log.Warn("wait for split failed", zap.Error(err)) + return } if ok { break @@ -169,13 +194,14 @@ func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) err time.Sleep(interval) } } - return nil + return } func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo *RegionInfo) { interval := ScatterWaitInterval regionID := regionInfo.Region.GetId() for i := 0; i < ScatterWaitMaxRetryTimes; i++ { + ctx = context.WithValue(ctx, "retryTimes", i) ok, err := rs.isScatterRegionFinished(ctx, regionID) if err != nil { log.Warn("scatter region failed: do not have the region", zap.Reflect("region", regionInfo.Region)) @@ -200,12 +226,9 @@ func (rs *RegionSplitter) splitAndScatterRegions(ctx context.Context, regionInfo } for _, region := range newRegions { // Wait for a while until the regions successfully splits. - err = rs.waitForSplit(ctx, region.Region.Id) - if err != nil { - return nil, err - } + rs.waitForSplit(ctx, region.Region.Id) if err = rs.client.ScatterRegion(ctx, region); err != nil { - return nil, err + log.Warn("scatter region failed", zap.Stringer("region", region.Region), zap.Error(err)) } } return newRegions, nil From bd126c4dd23ea17230ec1d2645a464271cd4f9f1 Mon Sep 17 00:00:00 2001 From: disksing Date: Wed, 25 Dec 2019 12:17:41 +0800 Subject: [PATCH 12/17] restore-util: fix wrong url (#306) Signed-off-by: disksing --- client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client.go b/client.go index ab03ad84f..b126f68a6 100644 --- a/client.go +++ b/client.go @@ -270,7 +270,7 @@ func (c *pdClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) if addr == "" { return rule, errors.New("failed to add stores labels: no leader") } - req, _ := http.NewRequestWithContext(ctx, "GET", path.Join(addr, "pd/api/v1/config/rule", groupID, ruleID), nil) + req, _ := http.NewRequestWithContext(ctx, "GET", addr+path.Join("/pd/api/v1/config/rule", groupID, ruleID), nil) res, err := http.DefaultClient.Do(req) if err != nil { return rule, errors.WithStack(err) @@ -293,7 +293,7 @@ func (c *pdClient) SetPlacementRule(ctx context.Context, rule placement.Rule) er return errors.New("failed to add stores labels: no leader") } m, _ := json.Marshal(rule) - req, _ := http.NewRequestWithContext(ctx, "POST", path.Join(addr, "pd/api/v1/config/rule"), bytes.NewReader(m)) + req, _ := http.NewRequestWithContext(ctx, "POST", addr+path.Join("/pd/api/v1/config/rule"), bytes.NewReader(m)) res, err := http.DefaultClient.Do(req) if err != nil { return errors.WithStack(err) @@ -308,7 +308,7 @@ func (c *pdClient) DeletePlacementRule(ctx context.Context, groupID, ruleID stri if addr == "" { return errors.New("failed to add stores labels: no leader") } - req, _ := http.NewRequestWithContext(ctx, "DELETE", path.Join(addr, "pd/api/v1/config/rule", groupID, ruleID), nil) + req, _ := http.NewRequestWithContext(ctx, "DELETE", addr+path.Join("/pd/api/v1/config/rule", groupID, ruleID), nil) res, err := http.DefaultClient.Do(req) if err != nil { return errors.WithStack(err) @@ -325,7 +325,7 @@ func (c *pdClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey return errors.New("failed to add stores labels: no leader") } for _, id := range stores { - req, _ := http.NewRequestWithContext(ctx, "POST", path.Join(addr, "pd/api/v1/store", strconv.FormatUint(id, 10), "label"), bytes.NewReader(b)) + req, _ := http.NewRequestWithContext(ctx, "POST", addr+path.Join("/pd/api/v1/store", strconv.FormatUint(id, 10), "label"), bytes.NewReader(b)) res, err := http.DefaultClient.Do(req) if err != nil { return errors.WithStack(err) @@ -342,5 +342,5 @@ func (c *pdClient) getPDAPIAddr() string { if addr != "" && !strings.HasPrefix(addr, "http") { addr = "http://" + addr } - return addr + return strings.TrimRight(addr, "/") } From e8ab896effc0960a789c5e45eb425b4351dabc6f Mon Sep 17 00:00:00 2001 From: 5kbpers <20279863+5kbpers@users.noreply.github.com> Date: Wed, 8 Jan 2020 14:11:54 +0800 Subject: [PATCH 13/17] restore-util: add warning about unmatched table id (#313) * restore-util: support table partition Signed-off-by: 5kbpers * fix log Signed-off-by: 5kbpers * warn table id does not match Signed-off-by: 5kbpers * add unit tests Signed-off-by: 5kbpers * Apply suggestions from code review Co-Authored-By: Neil Shen * fix compile error Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * address comments Signed-off-by: 5kbpers * fix test Signed-off-by: 5kbpers Co-authored-by: Ian Co-authored-by: Neil Shen --- range.go | 47 +++++++++++++++---------- range_test.go | 97 +++++++++++++++++++++++++++++++-------------------- split.go | 3 +- split_test.go | 11 +++--- 4 files changed, 97 insertions(+), 61 deletions(-) diff --git a/range.go b/range.go index 9eba63dc6..7f10f9f09 100644 --- a/range.go +++ b/range.go @@ -9,6 +9,7 @@ import ( "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/log" + "github.com/pingcap/tidb/tablecodec" "go.uber.org/zap" ) @@ -41,25 +42,35 @@ func sortRanges(ranges []Range, rewriteRules *RewriteRules) ([]Range, error) { rangeTree := NewRangeTree() for _, rg := range ranges { if rewriteRules != nil { - var startRule *import_sstpb.RewriteRule - rg.StartKey, startRule = replacePrefix(rg.StartKey, rewriteRules) - if startRule == nil { - log.Warn("cannot find rewrite rule", zap.Binary("key", rg.StartKey)) + startID := tablecodec.DecodeTableID(rg.StartKey) + endID := tablecodec.DecodeTableID(rg.EndKey) + var rule *import_sstpb.RewriteRule + if startID == endID { + rg.StartKey, rule = replacePrefix(rg.StartKey, rewriteRules) + if rule == nil { + log.Warn("cannot find rewrite rule", zap.Binary("key", rg.StartKey)) + } else { + log.Debug( + "rewrite start key", + zap.Binary("key", rg.StartKey), + zap.Stringer("rule", rule)) + } + rg.EndKey, rule = replacePrefix(rg.EndKey, rewriteRules) + if rule == nil { + log.Warn("cannot find rewrite rule", zap.Binary("key", rg.EndKey)) + } else { + log.Debug( + "rewrite end key", + zap.Binary("key", rg.EndKey), + zap.Stringer("rule", rule)) + } } else { - log.Debug( - "rewrite start key", - zap.Binary("key", rg.StartKey), - zap.Stringer("rule", startRule)) - } - var endRule *import_sstpb.RewriteRule - rg.EndKey, endRule = replacePrefix(rg.EndKey, rewriteRules) - if endRule == nil { - log.Warn("cannot find rewrite rule", zap.Binary("key", rg.EndKey)) - } else { - log.Debug( - "rewrite end key", - zap.Binary("key", rg.EndKey), - zap.Stringer("rule", endRule)) + log.Warn("table id does not match", + zap.Binary("startKey", rg.StartKey), + zap.Binary("endKey", rg.EndKey), + zap.Int64("startID", startID), + zap.Int64("endID", endID)) + return nil, errors.New("table id does not match") } } if out := rangeTree.InsertRange(rg); out != nil { diff --git a/range_test.go b/range_test.go index 56805d59d..df29e8933 100644 --- a/range_test.go +++ b/range_test.go @@ -3,52 +3,75 @@ package restore_util import ( "bytes" "testing" + + . "github.com/pingcap/check" + "github.com/pingcap/kvproto/pkg/import_sstpb" + "github.com/pingcap/tidb/tablecodec" ) -func TestRangeTreeNormal(t *testing.T) { - ranges := initRanges() - rangeTree := NewRangeTree() - for _, rg := range ranges { - if out := rangeTree.InsertRange(rg); out != nil { - t.Fatalf("insert range failed: %s %s", out.(*Range).String(), rg.String()) - } - } - resRanges := make([]Range, 0) - rangeTree.Ascend(func(rg *Range) bool { - if rg == nil { - return false - } - resRanges = append(resRanges, Range{ - StartKey: rg.StartKey, - EndKey: rg.EndKey, - }) - return true - }) - if len(ranges) != len(resRanges) { - t.Fatalf("some inserted ranges missed: rg=%v res=%v", ranges, resRanges) +func TestRestoreUtil(t *testing.T) { + TestingT(t) +} + +type testRestoreUtilSuite struct{} + +var _ = Suite(&testRestoreUtilSuite{}) + +type rangeEquals struct { + *CheckerInfo +} + +var RangeEquals Checker = &rangeEquals{ + &CheckerInfo{Name: "RangeEquals", Params: []string{"obtained", "expected"}}, +} + +func (checker *rangeEquals) Check(params []interface{}, names []string) (result bool, error string) { + obtained := params[0].([]Range) + expected := params[1].([]Range) + if len(obtained) != len(expected) { + return false, "" } - for i, rg := range ranges { - res := resRanges[i] - if !bytes.Equal(rg.StartKey, res.StartKey) || !bytes.Equal(rg.EndKey, res.EndKey) { - t.Fatalf("some inserted ranges missed: rg=%v res=%v", ranges, resRanges) + for i := range obtained { + if !bytes.Equal(obtained[i].StartKey, expected[i].StartKey) || + !bytes.Equal(obtained[i].EndKey, expected[i].EndKey) { + return false, "" } } + return true, "" } -func TestRangeOverlapped(t *testing.T) { - rg1 := Range{ - StartKey: []byte("aaa"), - EndKey: []byte("aaz"), +func (s *testRestoreUtilSuite) TestSortRange(c *C) { + dataRules := []*import_sstpb.RewriteRule{ + {OldKeyPrefix: tablecodec.GenTableRecordPrefix(1), NewKeyPrefix: tablecodec.GenTableRecordPrefix(4)}, + {OldKeyPrefix: tablecodec.GenTableRecordPrefix(2), NewKeyPrefix: tablecodec.GenTableRecordPrefix(5)}, } - rg2 := Range{ - StartKey: []byte("aab"), - EndKey: []byte("aag"), + rewriteRules := &RewriteRules{ + Table: make([]*import_sstpb.RewriteRule, 0), + Data: dataRules, } - rangeTree := NewRangeTree() - if out := rangeTree.InsertRange(rg1); out != nil { - t.Fatalf("insert range failed: %s %s", out.(*Range).String(), rg1.String()) + ranges1 := []Range{ + {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(1), []byte("bbb")...)}, } - if out := rangeTree.InsertRange(rg2); out == nil { - t.Fatalf("overlapping not detected: %s %s", rg1.String(), rg2.String()) + rs1, err := sortRanges(ranges1, rewriteRules) + c.Assert(err, IsNil, Commentf("sort range1 failed: %v", err)) + c.Assert(rs1, RangeEquals, []Range{ + {append(tablecodec.GenTableRecordPrefix(4), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(4), []byte("bbb")...)}, + }) + + ranges2 := []Range{ + {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(2), []byte("bbb")...)}, } + _, err = sortRanges(ranges2, rewriteRules) + c.Assert(err, ErrorMatches, ".*table id does not match.*") + + ranges3 := initRanges() + rewriteRules1 := initRewriteRules() + rs3, err := sortRanges(ranges3, rewriteRules1) + c.Assert(err, IsNil, Commentf("sort range1 failed: %v", err)) + c.Assert(rs3, RangeEquals, []Range{ + {[]byte("bbd"), []byte("bbf")}, + {[]byte("bbf"), []byte("bbj")}, + {[]byte("xxa"), []byte("xxe")}, + {[]byte("xxe"), []byte("xxz")}, + }) } diff --git a/split.go b/split.go index 6b0415aa1..6ec47bbca 100644 --- a/split.go +++ b/split.go @@ -204,7 +204,8 @@ func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo * ctx = context.WithValue(ctx, "retryTimes", i) ok, err := rs.isScatterRegionFinished(ctx, regionID) if err != nil { - log.Warn("scatter region failed: do not have the region", zap.Reflect("region", regionInfo.Region)) + log.Warn("scatter region failed: do not have the region", + zap.Stringer("region", regionInfo.Region)) return } if ok { diff --git a/split_test.go b/split_test.go index e5a5b8e19..801985e79 100644 --- a/split_test.go +++ b/split_test.go @@ -4,8 +4,8 @@ import ( "bytes" "context" "sync" - "testing" + . "github.com/pingcap/check" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" @@ -175,7 +175,7 @@ func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelK // rewrite rules: aa -> xx, cc -> bb // expected regions after split: // [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) -func TestSplit(t *testing.T) { +func (s *testRestoreUtilSuite) TestSplit(c *C) { client := initTestClient() ranges := initRanges() rewriteRules := initRewriteRules() @@ -184,14 +184,15 @@ func TestSplit(t *testing.T) { ctx := context.Background() err := regionSplitter.Split(ctx, ranges, rewriteRules, func(key [][]byte) {}) if err != nil { - t.Fatalf("split regions failed: %v", err) + c.Assert(err, IsNil, Commentf("split regions failed: %v", err)) } regions := client.GetAllRegions() if !validateRegions(regions) { for _, region := range regions { - t.Errorf("region: %v", region.Region) + c.Logf("region: %v\n", region.Region) } - t.Fatalf("get wrong result") + c.Log("get wrong result") + c.Fail() } } From 6087860794e4605984a795c0663f95e796a2c423 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Thu, 16 Jan 2020 19:50:37 +0800 Subject: [PATCH 14/17] *: prune tidb-tools Signed-off-by: Neil Shen --- cmd/validate.go | 2 +- go.mod | 1 - go.sum | 3 +-- pkg/restore/client.go | 2 +- pkg/restore/import.go | 2 +- pkg/restore/util.go | 2 +- pkg/restore/util_test.go | 3 ++- 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/validate.go b/cmd/validate.go index dd1e11fb0..feab732aa 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -15,11 +15,11 @@ import ( "github.com/pingcap/log" "github.com/pingcap/parser/model" "github.com/pingcap/pd/pkg/mock/mockid" - restore_util "github.com/pingcap/tidb-tools/pkg/restore-util" "github.com/spf13/cobra" "go.uber.org/zap" "github.com/pingcap/br/pkg/restore" + restore_util "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/storage" "github.com/pingcap/br/pkg/utils" ) diff --git a/go.mod b/go.mod index 8e50bbf35..9951c2922 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,6 @@ require ( github.com/pingcap/parser v0.0.0-20191210060830-bdf23a7ade01 github.com/pingcap/pd v1.1.0-beta.0.20191212045800-234784c7a9c5 github.com/pingcap/tidb v1.1.0-beta.0.20191213040028-9009da737834 - github.com/pingcap/tidb-tools v3.1.0-beta.0.20191223064326-e9c7a23a8dcb+incompatible github.com/pingcap/tipb v0.0.0-20191209145133-44f75c9bef33 github.com/prometheus/client_golang v1.0.0 github.com/sirupsen/logrus v1.4.2 diff --git a/go.sum b/go.sum index 696ccee81..085e00355 100644 --- a/go.sum +++ b/go.sum @@ -283,9 +283,8 @@ github.com/pingcap/sysutil v0.0.0-20191126040022-986c5b3ed9a3 h1:HCNif3lukL83gNC github.com/pingcap/sysutil v0.0.0-20191126040022-986c5b3ed9a3/go.mod h1:Futrrmuw98pEsbEmoPsjw8aKLCmixwHEmT2rF+AsXGw= github.com/pingcap/tidb v1.1.0-beta.0.20191213040028-9009da737834 h1:eNf7bDY39moIzzcs5+PhLLW0BM2D2yrzFbjW/X42y0s= github.com/pingcap/tidb v1.1.0-beta.0.20191213040028-9009da737834/go.mod h1:VWx47QOXISBHHtZeWrDQlBOdbvth9TE9gei6QpoqJ4g= +github.com/pingcap/tidb-tools v3.0.6-0.20191106033616-90632dda3863+incompatible h1:H1jg0aDWz2SLRh3hNBo2HFtnuHtudIUvBumU7syRkic= github.com/pingcap/tidb-tools v3.0.6-0.20191106033616-90632dda3863+incompatible/go.mod h1:XGdcy9+yqlDSEMTpOXnwf3hiTeqrV6MN/u1se9N8yIM= -github.com/pingcap/tidb-tools v3.1.0-beta.0.20191223064326-e9c7a23a8dcb+incompatible h1:GxWxXVqA2aAZIgS+bEpasJkkspu9Jom1/oB2NmP7t/o= -github.com/pingcap/tidb-tools v3.1.0-beta.0.20191223064326-e9c7a23a8dcb+incompatible/go.mod h1:XGdcy9+yqlDSEMTpOXnwf3hiTeqrV6MN/u1se9N8yIM= github.com/pingcap/tipb v0.0.0-20191209145133-44f75c9bef33 h1:cTSaVv1hue17BCPqt+sURADTFSMpSD26ZuvKRyYIjJs= github.com/pingcap/tipb v0.0.0-20191209145133-44f75c9bef33/go.mod h1:RtkHW8WbcNxj8lsbzjaILci01CtYnYbIkQhjyZWrWVI= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/pkg/restore/client.go b/pkg/restore/client.go index 9714edc2a..c112b5403 100644 --- a/pkg/restore/client.go +++ b/pkg/restore/client.go @@ -13,7 +13,6 @@ import ( "github.com/pingcap/log" "github.com/pingcap/parser/model" pd "github.com/pingcap/pd/client" - restore_util "github.com/pingcap/tidb-tools/pkg/restore-util" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/store/tikv/oracle" @@ -23,6 +22,7 @@ import ( "google.golang.org/grpc/keepalive" "github.com/pingcap/br/pkg/checksum" + restore_util "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" "github.com/pingcap/br/pkg/utils" ) diff --git a/pkg/restore/import.go b/pkg/restore/import.go index fc09b7b16..3a35315f2 100644 --- a/pkg/restore/import.go +++ b/pkg/restore/import.go @@ -12,10 +12,10 @@ import ( "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/log" "github.com/pingcap/pd/pkg/codec" - restore_util "github.com/pingcap/tidb-tools/pkg/restore-util" "go.uber.org/zap" "google.golang.org/grpc" + restore_util "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) diff --git a/pkg/restore/util.go b/pkg/restore/util.go index 126e864fd..bfbf85900 100644 --- a/pkg/restore/util.go +++ b/pkg/restore/util.go @@ -13,12 +13,12 @@ import ( "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/log" "github.com/pingcap/parser/model" - restore_util "github.com/pingcap/tidb-tools/pkg/restore-util" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/util/codec" "go.uber.org/zap" "go.uber.org/zap/zapcore" + restore_util "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) diff --git a/pkg/restore/util_test.go b/pkg/restore/util_test.go index 5da5c9ab7..a581f413c 100644 --- a/pkg/restore/util_test.go +++ b/pkg/restore/util_test.go @@ -5,8 +5,9 @@ import ( "github.com/pingcap/kvproto/pkg/backup" "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" - restore_util "github.com/pingcap/tidb-tools/pkg/restore-util" "github.com/pingcap/tidb/tablecodec" + + restore_util "github.com/pingcap/br/pkg/restoreutil" ) var _ = Suite(&testRestoreUtilSuite{}) From 23aa6f72b8f3c5a0f47c090b38a961dd8d764383 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Thu, 16 Jan 2020 20:27:14 +0800 Subject: [PATCH 15/17] restore: address linters suggestions Signed-off-by: Neil Shen --- .golangci.yml | 5 ++++ cmd/validate.go | 10 ++++---- pkg/restore/client.go | 14 +++++------ pkg/restore/import.go | 34 ++++++++++++------------- pkg/restore/util.go | 30 +++++++++++----------- pkg/restore/util_test.go | 4 +-- pkg/restoreutil/client.go | 43 ++++++++++++++++++-------------- pkg/restoreutil/range.go | 5 ++-- pkg/restoreutil/range_test.go | 11 +++++--- pkg/restoreutil/split.go | 47 +++++++++++++++++++---------------- pkg/restoreutil/split_test.go | 12 ++++++--- 11 files changed, 119 insertions(+), 96 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 969cac759..07e8afcdb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,3 +13,8 @@ issues: text: "Use of weak random number generator" linters: - gosec + # TODO Remove it. + - path: client.go + text: "SA1019:" + linters: + - staticcheck diff --git a/cmd/validate.go b/cmd/validate.go index feab732aa..58ceffa43 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -19,7 +19,7 @@ import ( "go.uber.org/zap" "github.com/pingcap/br/pkg/restore" - restore_util "github.com/pingcap/br/pkg/restoreutil" + restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/storage" "github.com/pingcap/br/pkg/utils" ) @@ -187,15 +187,15 @@ func newBackupMetaCommand() *cobra.Command { tables = append(tables, db.Tables...) } // Check if the ranges of files overlapped - rangeTree := restore_util.NewRangeTree() + rangeTree := restoreutil.NewRangeTree() for _, file := range files { - if out := rangeTree.InsertRange(restore_util.Range{ + if out := rangeTree.InsertRange(restoreutil.Range{ StartKey: file.GetStartKey(), EndKey: file.GetEndKey(), }); out != nil { log.Error( "file ranges overlapped", - zap.Stringer("out", out.(*restore_util.Range)), + zap.Stringer("out", out.(*restoreutil.Range)), zap.Stringer("file", file), ) } @@ -206,7 +206,7 @@ func newBackupMetaCommand() *cobra.Command { for offset := uint64(0); offset < tableIDOffset; offset++ { _, _ = tableIDAllocator.Alloc() // Ignore error } - rewriteRules := &restore_util.RewriteRules{ + rewriteRules := &restoreutil.RewriteRules{ Table: make([]*import_sstpb.RewriteRule, 0), Data: make([]*import_sstpb.RewriteRule, 0), } diff --git a/pkg/restore/client.go b/pkg/restore/client.go index c112b5403..9ee61a723 100644 --- a/pkg/restore/client.go +++ b/pkg/restore/client.go @@ -22,7 +22,7 @@ import ( "google.golang.org/grpc/keepalive" "github.com/pingcap/br/pkg/checksum" - restore_util "github.com/pingcap/br/pkg/restoreutil" + restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" "github.com/pingcap/br/pkg/utils" ) @@ -108,7 +108,7 @@ func (rc *Client) InitBackupMeta(backupMeta *backup.BackupMeta, backend *backup. rc.databases = databases rc.backupMeta = backupMeta - metaClient := restore_util.NewClient(rc.pdClient) + metaClient := restoreutil.NewSplitClient(rc.pdClient) importClient := NewImportClient(metaClient) rc.fileImporter = NewFileImporter(rc.ctx, metaClient, importClient, backend, rc.rateLimit) return nil @@ -189,8 +189,8 @@ func (rc *Client) CreateTables( dom *domain.Domain, tables []*utils.Table, newTS uint64, -) (*restore_util.RewriteRules, []*model.TableInfo, error) { - rewriteRules := &restore_util.RewriteRules{ +) (*restoreutil.RewriteRules, []*model.TableInfo, error) { + rewriteRules := &restoreutil.RewriteRules{ Table: make([]*import_sstpb.RewriteRule, 0), Data: make([]*import_sstpb.RewriteRule, 0), } @@ -232,7 +232,7 @@ func (rc *Client) setSpeedLimit() error { // RestoreTable tries to restore the data of a table. func (rc *Client) RestoreTable( table *utils.Table, - rewriteRules *restore_util.RewriteRules, + rewriteRules *restoreutil.RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() @@ -300,7 +300,7 @@ func (rc *Client) RestoreTable( // RestoreDatabase tries to restore the data of a database func (rc *Client) RestoreDatabase( db *utils.Database, - rewriteRules *restore_util.RewriteRules, + rewriteRules *restoreutil.RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() @@ -336,7 +336,7 @@ func (rc *Client) RestoreDatabase( // RestoreAll tries to restore all the data of backup files. func (rc *Client) RestoreAll( - rewriteRules *restore_util.RewriteRules, + rewriteRules *restoreutil.RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() diff --git a/pkg/restore/import.go b/pkg/restore/import.go index 3a35315f2..b772bbdc4 100644 --- a/pkg/restore/import.go +++ b/pkg/restore/import.go @@ -15,7 +15,7 @@ import ( "go.uber.org/zap" "google.golang.org/grpc" - restore_util "github.com/pingcap/br/pkg/restoreutil" + restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) @@ -60,12 +60,12 @@ type ImporterClient interface { type importClient struct { mu sync.Mutex - metaClient restore_util.Client + metaClient restoreutil.SplitClient clients map[uint64]import_sstpb.ImportSSTClient } // NewImportClient returns a new ImporterClient -func NewImportClient(metaClient restore_util.Client) ImporterClient { +func NewImportClient(metaClient restoreutil.SplitClient) ImporterClient { return &importClient{ metaClient: metaClient, clients: make(map[uint64]import_sstpb.ImportSSTClient), @@ -133,7 +133,7 @@ func (ic *importClient) getImportClient( // FileImporter used to import a file to TiKV. type FileImporter struct { - metaClient restore_util.Client + metaClient restoreutil.SplitClient importClient ImporterClient backend *backup.StorageBackend rateLimit uint64 @@ -145,7 +145,7 @@ type FileImporter struct { // NewFileImporter returns a new file importClient. func NewFileImporter( ctx context.Context, - metaClient restore_util.Client, + metaClient restoreutil.SplitClient, importClient ImporterClient, backend *backup.StorageBackend, rateLimit uint64, @@ -163,7 +163,7 @@ func NewFileImporter( // Import tries to import a file. // All rules must contain encoded keys. -func (importer *FileImporter) Import(file *backup.File, rewriteRules *restore_util.RewriteRules) error { +func (importer *FileImporter) Import(file *backup.File, rewriteRules *restoreutil.RewriteRules) error { log.Debug("import file", zap.Stringer("file", file)) // Rewrite the start key and end key of file to scan regions startKey, endKey, err := rewriteFileKeys(file, rewriteRules) @@ -179,9 +179,9 @@ func (importer *FileImporter) Import(file *backup.File, rewriteRules *restore_ut ctx, cancel := context.WithTimeout(importer.ctx, importScanResgionTime) defer cancel() // Scan regions covered by the file range - regionInfos, err := importer.metaClient.ScanRegions(ctx, startKey, endKey, 0) - if err != nil { - return errors.Trace(err) + regionInfos, err1 := importer.metaClient.ScanRegions(ctx, startKey, endKey, 0) + if err1 != nil { + return errors.Trace(err1) } log.Debug("scan regions", zap.Stringer("file", file), zap.Int("count", len(regionInfos))) // Try to download and ingest the file in every region @@ -190,20 +190,20 @@ func (importer *FileImporter) Import(file *backup.File, rewriteRules *restore_ut info := regionInfo // Try to download file. err = withRetry(func() error { - var err error + var err2 error var isEmpty bool - downloadMeta, isEmpty, err = importer.downloadSST(info, file, rewriteRules) - if err != nil { + downloadMeta, isEmpty, err2 = importer.downloadSST(info, file, rewriteRules) + if err2 != nil { if err != errRewriteRuleNotFound { log.Warn("download file failed", zap.Stringer("file", file), zap.Stringer("region", info.Region), zap.Binary("startKey", startKey), zap.Binary("endKey", endKey), - zap.Error(err), + zap.Error(err2), ) } - return err + return err2 } if isEmpty { log.Info( @@ -255,9 +255,9 @@ func (importer *FileImporter) setDownloadSpeedLimit(storeID uint64) error { } func (importer *FileImporter) downloadSST( - regionInfo *restore_util.RegionInfo, + regionInfo *restoreutil.RegionInfo, file *backup.File, - rewriteRules *restore_util.RewriteRules, + rewriteRules *restoreutil.RewriteRules, ) (*import_sstpb.SSTMeta, bool, error) { id, err := uuid.New().MarshalBinary() if err != nil { @@ -312,7 +312,7 @@ func (importer *FileImporter) downloadSST( func (importer *FileImporter) ingestSST( sstMeta *import_sstpb.SSTMeta, - regionInfo *restore_util.RegionInfo, + regionInfo *restoreutil.RegionInfo, ) error { leader := regionInfo.Leader if leader == nil { diff --git a/pkg/restore/util.go b/pkg/restore/util.go index bfbf85900..8e801abdf 100644 --- a/pkg/restore/util.go +++ b/pkg/restore/util.go @@ -18,7 +18,7 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" - restore_util "github.com/pingcap/br/pkg/restoreutil" + restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) @@ -76,7 +76,7 @@ func GetRewriteRules( newTable *model.TableInfo, oldTable *model.TableInfo, newTimeStamp uint64, -) *restore_util.RewriteRules { +) *restoreutil.RewriteRules { tableIDs := make(map[int64]int64) tableIDs[oldTable.ID] = newTable.ID if oldTable.Partition != nil { @@ -119,7 +119,7 @@ func GetRewriteRules( } } - return &restore_util.RewriteRules{ + return &restoreutil.RewriteRules{ Table: tableRules, Data: dataRules, } @@ -196,9 +196,9 @@ func withRetry( // ValidateFileRanges checks and returns the ranges of the files. func ValidateFileRanges( files []*backup.File, - rewriteRules *restore_util.RewriteRules, -) ([]restore_util.Range, error) { - ranges := make([]restore_util.Range, 0, len(files)) + rewriteRules *restoreutil.RewriteRules, +) ([]restoreutil.Range, error) { + ranges := make([]restoreutil.Range, 0, len(files)) fileAppended := make(map[string]bool) for _, file := range files { @@ -217,7 +217,7 @@ func ValidateFileRanges( zap.Stringer("file", file)) return nil, errors.New("table ids dont match") } - ranges = append(ranges, restore_util.Range{ + ranges = append(ranges, restoreutil.Range{ StartKey: file.GetStartKey(), EndKey: file.GetEndKey(), }) @@ -228,7 +228,7 @@ func ValidateFileRanges( } // ValidateFileRewriteRule uses rewrite rules to validate the ranges of a file -func ValidateFileRewriteRule(file *backup.File, rewriteRules *restore_util.RewriteRules) error { +func ValidateFileRewriteRule(file *backup.File, rewriteRules *restoreutil.RewriteRules) error { // Check if the start key has a matched rewrite key _, startRule := rewriteRawKey(file.GetStartKey(), rewriteRules) if rewriteRules != nil && startRule == nil { @@ -269,7 +269,7 @@ func ValidateFileRewriteRule(file *backup.File, rewriteRules *restore_util.Rewri } // Rewrites a raw key and returns a encoded key -func rewriteRawKey(key []byte, rewriteRules *restore_util.RewriteRules) ([]byte, *import_sstpb.RewriteRule) { +func rewriteRawKey(key []byte, rewriteRules *restoreutil.RewriteRules) ([]byte, *import_sstpb.RewriteRule) { if rewriteRules == nil { return codec.EncodeBytes([]byte{}, key), nil } @@ -281,7 +281,7 @@ func rewriteRawKey(key []byte, rewriteRules *restore_util.RewriteRules) ([]byte, return nil, nil } -func matchOldPrefix(key []byte, rewriteRules *restore_util.RewriteRules) *import_sstpb.RewriteRule { +func matchOldPrefix(key []byte, rewriteRules *restoreutil.RewriteRules) *import_sstpb.RewriteRule { for _, rule := range rewriteRules.Data { if bytes.HasPrefix(key, rule.GetOldKeyPrefix()) { return rule @@ -295,7 +295,7 @@ func matchOldPrefix(key []byte, rewriteRules *restore_util.RewriteRules) *import return nil } -func matchNewPrefix(key []byte, rewriteRules *restore_util.RewriteRules) *import_sstpb.RewriteRule { +func matchNewPrefix(key []byte, rewriteRules *restoreutil.RewriteRules) *import_sstpb.RewriteRule { for _, rule := range rewriteRules.Data { if bytes.HasPrefix(key, rule.GetNewKeyPrefix()) { return rule @@ -319,8 +319,8 @@ func truncateTS(key []byte) []byte { func SplitRanges( ctx context.Context, client *Client, - ranges []restore_util.Range, - rewriteRules *restore_util.RewriteRules, + ranges []restoreutil.Range, + rewriteRules *restoreutil.RewriteRules, updateCh chan<- struct{}, ) error { start := time.Now() @@ -328,7 +328,7 @@ func SplitRanges( elapsed := time.Since(start) summary.CollectDuration("split region", elapsed) }() - splitter := restore_util.NewRegionSplitter(restore_util.NewClient(client.GetPDClient())) + splitter := restoreutil.NewRegionSplitter(restoreutil.NewSplitClient(client.GetPDClient())) return splitter.Split(ctx, ranges, rewriteRules, func(keys [][]byte) { for range keys { updateCh <- struct{}{} @@ -336,7 +336,7 @@ func SplitRanges( }) } -func rewriteFileKeys(file *backup.File, rewriteRules *restore_util.RewriteRules) (startKey, endKey []byte, err error) { +func rewriteFileKeys(file *backup.File, rewriteRules *restoreutil.RewriteRules) (startKey, endKey []byte, err error) { startID := tablecodec.DecodeTableID(file.GetStartKey()) endID := tablecodec.DecodeTableID(file.GetEndKey()) var rule *import_sstpb.RewriteRule diff --git a/pkg/restore/util_test.go b/pkg/restore/util_test.go index a581f413c..d0c1704d7 100644 --- a/pkg/restore/util_test.go +++ b/pkg/restore/util_test.go @@ -7,7 +7,7 @@ import ( "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/tablecodec" - restore_util "github.com/pingcap/br/pkg/restoreutil" + restoreutil "github.com/pingcap/br/pkg/restoreutil" ) var _ = Suite(&testRestoreUtilSuite{}) @@ -35,7 +35,7 @@ func (s *testRestoreUtilSuite) TestGetSSTMetaFromFile(c *C) { } func (s *testRestoreUtilSuite) TestValidateFileRanges(c *C) { - rules := &restore_util.RewriteRules{ + rules := &restoreutil.RewriteRules{ Table: []*import_sstpb.RewriteRule{&import_sstpb.RewriteRule{ OldKeyPrefix: []byte(tablecodec.EncodeTablePrefix(1)), NewKeyPrefix: []byte(tablecodec.EncodeTablePrefix(2)), diff --git a/pkg/restoreutil/client.go b/pkg/restoreutil/client.go index b126f68a6..2d88e25b3 100644 --- a/pkg/restoreutil/client.go +++ b/pkg/restoreutil/client.go @@ -1,4 +1,4 @@ -package restore_util +package restoreutil import ( "bytes" @@ -22,8 +22,8 @@ import ( "google.golang.org/grpc" ) -// Client is an external client used by RegionSplitter. -type Client interface { +// SplitClient is an external client used by RegionSplitter. +type SplitClient interface { // GetStore gets a store by a store id. GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) // GetRegion gets a region which includes a specified key. @@ -61,8 +61,8 @@ type pdClient struct { storeCache map[uint64]*metapb.Store } -// NewClient returns a client used by RegionSplitter. -func NewClient(client pd.Client) Client { +// NewSplitClient returns a client used by RegionSplitter. +func NewSplitClient(client pd.Client) SplitClient { return &pdClient{ client: client, storeCache: make(map[uint64]*metapb.Store), @@ -129,10 +129,11 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key return nil, err } conn, err := grpc.Dial(store.GetAddress(), grpc.WithInsecure()) - defer conn.Close() if err != nil { return nil, err } + defer conn.Close() + client := tikvpb.NewTikvClient(conn) resp, err := client.SplitRegion(ctx, &kvrpcpb.SplitRegionRequest{ Context: &kvrpcpb.Context{ @@ -149,6 +150,7 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key return nil, errors.Errorf("split region failed: region=%v, key=%x, err=%v", regionInfo.Region, key, resp.RegionError) } + // BUG: Left is deprecated, it may be nil even if split is succeed! // Assume the new region is the left one. newRegion := resp.GetLeft() if newRegion == nil { @@ -179,7 +181,9 @@ func (c *pdClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, key }, nil } -func (c *pdClient) BatchSplitRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { +func (c *pdClient) BatchSplitRegions( + ctx context.Context, regionInfo *RegionInfo, keys [][]byte, +) ([]*RegionInfo, error) { var peer *metapb.Peer if regionInfo.Leader != nil { peer = regionInfo.Leader @@ -298,9 +302,7 @@ func (c *pdClient) SetPlacementRule(ctx context.Context, rule placement.Rule) er if err != nil { return errors.WithStack(err) } - ioutil.ReadAll(res.Body) - res.Body.Close() - return nil + return errors.Trace(res.Body.Close()) } func (c *pdClient) DeletePlacementRule(ctx context.Context, groupID, ruleID string) error { @@ -313,26 +315,31 @@ func (c *pdClient) DeletePlacementRule(ctx context.Context, groupID, ruleID stri if err != nil { return errors.WithStack(err) } - ioutil.ReadAll(res.Body) - res.Body.Close() - return nil + return errors.Trace(res.Body.Close()) } -func (c *pdClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error { +func (c *pdClient) SetStoresLabel( + ctx context.Context, stores []uint64, labelKey, labelValue string, +) error { b := []byte(fmt.Sprintf(`{"%s": "%s"}`, labelKey, labelValue)) addr := c.getPDAPIAddr() if addr == "" { return errors.New("failed to add stores labels: no leader") } for _, id := range stores { - req, _ := http.NewRequestWithContext(ctx, "POST", addr+path.Join("/pd/api/v1/store", strconv.FormatUint(id, 10), "label"), bytes.NewReader(b)) + req, _ := http.NewRequestWithContext( + ctx, "POST", + addr+path.Join("/pd/api/v1/store", strconv.FormatUint(id, 10), "label"), + bytes.NewReader(b), + ) res, err := http.DefaultClient.Do(req) if err != nil { return errors.WithStack(err) } - ioutil.ReadAll(res.Body) - res.Body.Close() - + err = res.Body.Close() + if err != nil { + return errors.Trace(err) + } } return nil } diff --git a/pkg/restoreutil/range.go b/pkg/restoreutil/range.go index 7f10f9f09..6024f0ae9 100644 --- a/pkg/restoreutil/range.go +++ b/pkg/restoreutil/range.go @@ -1,4 +1,4 @@ -package restore_util +package restoreutil import ( "bytes" @@ -115,7 +115,7 @@ func (rt *RangeTree) Find(key []byte) *Range { return ret } -// InsertRanges inserts ranges into the range tree. +// InsertRange inserts ranges into the range tree. // it returns true if all ranges inserted successfully. // it returns false if there are some overlapped ranges. func (rt *RangeTree) InsertRange(rg Range) btree.Item { @@ -141,6 +141,7 @@ type RegionInfo struct { Leader *metapb.Peer } +// RewriteRules contains rules for rewriting keys of tables. type RewriteRules struct { Table []*import_sstpb.RewriteRule Data []*import_sstpb.RewriteRule diff --git a/pkg/restoreutil/range_test.go b/pkg/restoreutil/range_test.go index df29e8933..13fb0138d 100644 --- a/pkg/restoreutil/range_test.go +++ b/pkg/restoreutil/range_test.go @@ -1,4 +1,4 @@ -package restore_util +package restoreutil import ( "bytes" @@ -50,16 +50,19 @@ func (s *testRestoreUtilSuite) TestSortRange(c *C) { Data: dataRules, } ranges1 := []Range{ - {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(1), []byte("bbb")...)}, + {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), + append(tablecodec.GenTableRecordPrefix(1), []byte("bbb")...)}, } rs1, err := sortRanges(ranges1, rewriteRules) c.Assert(err, IsNil, Commentf("sort range1 failed: %v", err)) c.Assert(rs1, RangeEquals, []Range{ - {append(tablecodec.GenTableRecordPrefix(4), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(4), []byte("bbb")...)}, + {append(tablecodec.GenTableRecordPrefix(4), []byte("aaa")...), + append(tablecodec.GenTableRecordPrefix(4), []byte("bbb")...)}, }) ranges2 := []Range{ - {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), append(tablecodec.GenTableRecordPrefix(2), []byte("bbb")...)}, + {append(tablecodec.GenTableRecordPrefix(1), []byte("aaa")...), + append(tablecodec.GenTableRecordPrefix(2), []byte("bbb")...)}, } _, err = sortRanges(ranges2, rewriteRules) c.Assert(err, ErrorMatches, ".*table id does not match.*") diff --git a/pkg/restoreutil/split.go b/pkg/restoreutil/split.go index 6ec47bbca..2f52bc218 100644 --- a/pkg/restoreutil/split.go +++ b/pkg/restoreutil/split.go @@ -1,4 +1,4 @@ -package restore_util +package restoreutil import ( "bytes" @@ -13,6 +13,7 @@ import ( "go.uber.org/zap" ) +// Constants for split retry machinery. const ( SplitRetryTimes = 32 SplitRetryInterval = 50 * time.Millisecond @@ -31,12 +32,11 @@ const ( // RegionSplitter is a executor of region split by rules. type RegionSplitter struct { - client Client - rangeTree *RangeTree + client SplitClient } // NewRegionSplitter returns a new RegionSplitter. -func NewRegionSplitter(client Client) *RegionSplitter { +func NewRegionSplitter(client SplitClient) *RegionSplitter { return &RegionSplitter{ client: client, } @@ -166,7 +166,7 @@ func (rs *RegionSplitter) isScatterRegionFinished(ctx context.Context, regionID } return false, errors.Errorf("get operator error: %s", respErr.GetType()) } - retryTimes := ctx.Value("retryTimes").(int) + retryTimes := ctx.Value(retryTimes).(int) if retryTimes > 3 { log.Warn("get operator", zap.Uint64("regionID", regionID), zap.Stringer("resp", resp)) } @@ -186,23 +186,25 @@ func (rs *RegionSplitter) waitForSplit(ctx context.Context, regionID uint64) { } if ok { break - } else { - interval = 2 * interval - if interval > SplitMaxCheckInterval { - interval = SplitMaxCheckInterval - } - time.Sleep(interval) } + interval = 2 * interval + if interval > SplitMaxCheckInterval { + interval = SplitMaxCheckInterval + } + time.Sleep(interval) } - return } +type retryTimeKey struct{} + +var retryTimes = new(retryTimeKey) + func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo *RegionInfo) { interval := ScatterWaitInterval regionID := regionInfo.Region.GetId() for i := 0; i < ScatterWaitMaxRetryTimes; i++ { - ctx = context.WithValue(ctx, "retryTimes", i) - ok, err := rs.isScatterRegionFinished(ctx, regionID) + ctx1 := context.WithValue(ctx, retryTimes, i) + ok, err := rs.isScatterRegionFinished(ctx1, regionID) if err != nil { log.Warn("scatter region failed: do not have the region", zap.Stringer("region", regionInfo.Region)) @@ -210,17 +212,18 @@ func (rs *RegionSplitter) waitForScatterRegion(ctx context.Context, regionInfo * } if ok { break - } else { - interval = 2 * interval - if interval > ScatterMaxWaitInterval { - interval = ScatterMaxWaitInterval - } - time.Sleep(interval) } + interval = 2 * interval + if interval > ScatterMaxWaitInterval { + interval = ScatterMaxWaitInterval + } + time.Sleep(interval) } } -func (rs *RegionSplitter) splitAndScatterRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { +func (rs *RegionSplitter) splitAndScatterRegions( + ctx context.Context, regionInfo *RegionInfo, keys [][]byte, +) ([]*RegionInfo, error) { newRegions, err := rs.client.BatchSplitRegions(ctx, regionInfo, keys) if err != nil { return nil, err @@ -270,7 +273,7 @@ func needSplit(splitKey []byte, regions []*RegionInfo) *RegionInfo { splitKey = codec.EncodeBytes([]byte{}, splitKey) for _, region := range regions { // If splitKey is the boundary of the region - if bytes.Compare(splitKey, region.Region.GetStartKey()) == 0 { + if bytes.Equal(splitKey, region.Region.GetStartKey()) { return nil } // If splitKey is in a region diff --git a/pkg/restoreutil/split_test.go b/pkg/restoreutil/split_test.go index 801985e79..85878bda3 100644 --- a/pkg/restoreutil/split_test.go +++ b/pkg/restoreutil/split_test.go @@ -1,4 +1,4 @@ -package restore_util +package restoreutil import ( "bytes" @@ -96,7 +96,9 @@ func (c *testClient) SplitRegion(ctx context.Context, regionInfo *RegionInfo, ke return newRegion, nil } -func (c *testClient) BatchSplitRegions(ctx context.Context, regionInfo *RegionInfo, keys [][]byte) ([]*RegionInfo, error) { +func (c *testClient) BatchSplitRegions( + ctx context.Context, regionInfo *RegionInfo, keys [][]byte, +) ([]*RegionInfo, error) { c.mu.Lock() defer c.mu.Unlock() newRegions := make([]*RegionInfo, 0) @@ -174,7 +176,8 @@ func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelK // range: [aaa, aae), [aae, aaz), [ccd, ccf), [ccf, ccj) // rewrite rules: aa -> xx, cc -> bb // expected regions after split: -// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), +// [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) func (s *testRestoreUtilSuite) TestSplit(c *C) { client := initTestClient() ranges := initRanges() @@ -269,7 +272,8 @@ func initRewriteRules() *RewriteRules { } // expected regions after split: -// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) +// [, aay), [aay, bb), [bb, bba), [bba, bbf), [bbf, bbh), [bbh, bbj), +// [bbj, cca), [cca, xx), [xx, xxe), [xxe, xxz), [xxz, ) func validateRegions(regions map[uint64]*RegionInfo) bool { keys := [12]string{"", "aay", "bb", "bba", "bbf", "bbh", "bbj", "cca", "xx", "xxe", "xxz", ""} if len(regions) != 11 { From 5f5a239ffdd6a0a7a59ee23192ce06a1b5a0a592 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Thu, 16 Jan 2020 20:56:43 +0800 Subject: [PATCH 16/17] restore: merge restoreutil into restore Signed-off-by: Neil Shen --- .golangci.yml | 4 --- cmd/validate.go | 9 +++--- pkg/restore/client.go | 13 ++++----- pkg/restore/import.go | 17 +++++------ pkg/{restoreutil => restore}/range.go | 2 +- pkg/{restoreutil => restore}/range_test.go | 13 +++------ pkg/{restoreutil => restore}/split.go | 2 +- .../client.go => restore/split_client.go} | 2 +- pkg/{restoreutil => restore}/split_test.go | 2 +- pkg/restore/util.go | 29 +++++++++---------- pkg/restore/util_test.go | 4 +-- 11 files changed, 41 insertions(+), 56 deletions(-) rename pkg/{restoreutil => restore}/range.go (99%) rename pkg/{restoreutil => restore}/range_test.go (91%) rename pkg/{restoreutil => restore}/split.go (99%) rename pkg/{restoreutil/client.go => restore/split_client.go} (99%) rename pkg/{restoreutil => restore}/split_test.go (99%) diff --git a/.golangci.yml b/.golangci.yml index 07e8afcdb..3a7cfa063 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,10 +9,6 @@ issues: text: "Potential HTTP request made with variable url" linters: - gosec - - path: .go - text: "Use of weak random number generator" - linters: - - gosec # TODO Remove it. - path: client.go text: "SA1019:" diff --git a/cmd/validate.go b/cmd/validate.go index 58ceffa43..8ba72b372 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -19,7 +19,6 @@ import ( "go.uber.org/zap" "github.com/pingcap/br/pkg/restore" - restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/storage" "github.com/pingcap/br/pkg/utils" ) @@ -187,15 +186,15 @@ func newBackupMetaCommand() *cobra.Command { tables = append(tables, db.Tables...) } // Check if the ranges of files overlapped - rangeTree := restoreutil.NewRangeTree() + rangeTree := restore.NewRangeTree() for _, file := range files { - if out := rangeTree.InsertRange(restoreutil.Range{ + if out := rangeTree.InsertRange(restore.Range{ StartKey: file.GetStartKey(), EndKey: file.GetEndKey(), }); out != nil { log.Error( "file ranges overlapped", - zap.Stringer("out", out.(*restoreutil.Range)), + zap.Stringer("out", out.(*restore.Range)), zap.Stringer("file", file), ) } @@ -206,7 +205,7 @@ func newBackupMetaCommand() *cobra.Command { for offset := uint64(0); offset < tableIDOffset; offset++ { _, _ = tableIDAllocator.Alloc() // Ignore error } - rewriteRules := &restoreutil.RewriteRules{ + rewriteRules := &restore.RewriteRules{ Table: make([]*import_sstpb.RewriteRule, 0), Data: make([]*import_sstpb.RewriteRule, 0), } diff --git a/pkg/restore/client.go b/pkg/restore/client.go index 9ee61a723..3030ba857 100644 --- a/pkg/restore/client.go +++ b/pkg/restore/client.go @@ -22,7 +22,6 @@ import ( "google.golang.org/grpc/keepalive" "github.com/pingcap/br/pkg/checksum" - restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" "github.com/pingcap/br/pkg/utils" ) @@ -108,7 +107,7 @@ func (rc *Client) InitBackupMeta(backupMeta *backup.BackupMeta, backend *backup. rc.databases = databases rc.backupMeta = backupMeta - metaClient := restoreutil.NewSplitClient(rc.pdClient) + metaClient := NewSplitClient(rc.pdClient) importClient := NewImportClient(metaClient) rc.fileImporter = NewFileImporter(rc.ctx, metaClient, importClient, backend, rc.rateLimit) return nil @@ -189,8 +188,8 @@ func (rc *Client) CreateTables( dom *domain.Domain, tables []*utils.Table, newTS uint64, -) (*restoreutil.RewriteRules, []*model.TableInfo, error) { - rewriteRules := &restoreutil.RewriteRules{ +) (*RewriteRules, []*model.TableInfo, error) { + rewriteRules := &RewriteRules{ Table: make([]*import_sstpb.RewriteRule, 0), Data: make([]*import_sstpb.RewriteRule, 0), } @@ -232,7 +231,7 @@ func (rc *Client) setSpeedLimit() error { // RestoreTable tries to restore the data of a table. func (rc *Client) RestoreTable( table *utils.Table, - rewriteRules *restoreutil.RewriteRules, + rewriteRules *RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() @@ -300,7 +299,7 @@ func (rc *Client) RestoreTable( // RestoreDatabase tries to restore the data of a database func (rc *Client) RestoreDatabase( db *utils.Database, - rewriteRules *restoreutil.RewriteRules, + rewriteRules *RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() @@ -336,7 +335,7 @@ func (rc *Client) RestoreDatabase( // RestoreAll tries to restore all the data of backup files. func (rc *Client) RestoreAll( - rewriteRules *restoreutil.RewriteRules, + rewriteRules *RewriteRules, updateCh chan<- struct{}, ) (err error) { start := time.Now() diff --git a/pkg/restore/import.go b/pkg/restore/import.go index b772bbdc4..77273ebab 100644 --- a/pkg/restore/import.go +++ b/pkg/restore/import.go @@ -15,7 +15,6 @@ import ( "go.uber.org/zap" "google.golang.org/grpc" - restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) @@ -60,12 +59,12 @@ type ImporterClient interface { type importClient struct { mu sync.Mutex - metaClient restoreutil.SplitClient + metaClient SplitClient clients map[uint64]import_sstpb.ImportSSTClient } // NewImportClient returns a new ImporterClient -func NewImportClient(metaClient restoreutil.SplitClient) ImporterClient { +func NewImportClient(metaClient SplitClient) ImporterClient { return &importClient{ metaClient: metaClient, clients: make(map[uint64]import_sstpb.ImportSSTClient), @@ -133,7 +132,7 @@ func (ic *importClient) getImportClient( // FileImporter used to import a file to TiKV. type FileImporter struct { - metaClient restoreutil.SplitClient + metaClient SplitClient importClient ImporterClient backend *backup.StorageBackend rateLimit uint64 @@ -145,7 +144,7 @@ type FileImporter struct { // NewFileImporter returns a new file importClient. func NewFileImporter( ctx context.Context, - metaClient restoreutil.SplitClient, + metaClient SplitClient, importClient ImporterClient, backend *backup.StorageBackend, rateLimit uint64, @@ -163,7 +162,7 @@ func NewFileImporter( // Import tries to import a file. // All rules must contain encoded keys. -func (importer *FileImporter) Import(file *backup.File, rewriteRules *restoreutil.RewriteRules) error { +func (importer *FileImporter) Import(file *backup.File, rewriteRules *RewriteRules) error { log.Debug("import file", zap.Stringer("file", file)) // Rewrite the start key and end key of file to scan regions startKey, endKey, err := rewriteFileKeys(file, rewriteRules) @@ -255,9 +254,9 @@ func (importer *FileImporter) setDownloadSpeedLimit(storeID uint64) error { } func (importer *FileImporter) downloadSST( - regionInfo *restoreutil.RegionInfo, + regionInfo *RegionInfo, file *backup.File, - rewriteRules *restoreutil.RewriteRules, + rewriteRules *RewriteRules, ) (*import_sstpb.SSTMeta, bool, error) { id, err := uuid.New().MarshalBinary() if err != nil { @@ -312,7 +311,7 @@ func (importer *FileImporter) downloadSST( func (importer *FileImporter) ingestSST( sstMeta *import_sstpb.SSTMeta, - regionInfo *restoreutil.RegionInfo, + regionInfo *RegionInfo, ) error { leader := regionInfo.Leader if leader == nil { diff --git a/pkg/restoreutil/range.go b/pkg/restore/range.go similarity index 99% rename from pkg/restoreutil/range.go rename to pkg/restore/range.go index 6024f0ae9..f3914539e 100644 --- a/pkg/restoreutil/range.go +++ b/pkg/restore/range.go @@ -1,4 +1,4 @@ -package restoreutil +package restore import ( "bytes" diff --git a/pkg/restoreutil/range_test.go b/pkg/restore/range_test.go similarity index 91% rename from pkg/restoreutil/range_test.go rename to pkg/restore/range_test.go index 13fb0138d..a9edc5b82 100644 --- a/pkg/restoreutil/range_test.go +++ b/pkg/restore/range_test.go @@ -1,21 +1,16 @@ -package restoreutil +package restore import ( "bytes" - "testing" . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/tidb/tablecodec" ) -func TestRestoreUtil(t *testing.T) { - TestingT(t) -} - -type testRestoreUtilSuite struct{} +type testRangeSuite struct{} -var _ = Suite(&testRestoreUtilSuite{}) +var _ = Suite(&testRangeSuite{}) type rangeEquals struct { *CheckerInfo @@ -40,7 +35,7 @@ func (checker *rangeEquals) Check(params []interface{}, names []string) (result return true, "" } -func (s *testRestoreUtilSuite) TestSortRange(c *C) { +func (s *testRangeSuite) TestSortRange(c *C) { dataRules := []*import_sstpb.RewriteRule{ {OldKeyPrefix: tablecodec.GenTableRecordPrefix(1), NewKeyPrefix: tablecodec.GenTableRecordPrefix(4)}, {OldKeyPrefix: tablecodec.GenTableRecordPrefix(2), NewKeyPrefix: tablecodec.GenTableRecordPrefix(5)}, diff --git a/pkg/restoreutil/split.go b/pkg/restore/split.go similarity index 99% rename from pkg/restoreutil/split.go rename to pkg/restore/split.go index 2f52bc218..31b23a60f 100644 --- a/pkg/restoreutil/split.go +++ b/pkg/restore/split.go @@ -1,4 +1,4 @@ -package restoreutil +package restore import ( "bytes" diff --git a/pkg/restoreutil/client.go b/pkg/restore/split_client.go similarity index 99% rename from pkg/restoreutil/client.go rename to pkg/restore/split_client.go index 2d88e25b3..8a618a191 100644 --- a/pkg/restoreutil/client.go +++ b/pkg/restore/split_client.go @@ -1,4 +1,4 @@ -package restoreutil +package restore import ( "bytes" diff --git a/pkg/restoreutil/split_test.go b/pkg/restore/split_test.go similarity index 99% rename from pkg/restoreutil/split_test.go rename to pkg/restore/split_test.go index 85878bda3..509c4cfa0 100644 --- a/pkg/restoreutil/split_test.go +++ b/pkg/restore/split_test.go @@ -1,4 +1,4 @@ -package restoreutil +package restore import ( "bytes" diff --git a/pkg/restore/util.go b/pkg/restore/util.go index 8e801abdf..a2e9e3e38 100644 --- a/pkg/restore/util.go +++ b/pkg/restore/util.go @@ -18,7 +18,6 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" - restoreutil "github.com/pingcap/br/pkg/restoreutil" "github.com/pingcap/br/pkg/summary" ) @@ -76,7 +75,7 @@ func GetRewriteRules( newTable *model.TableInfo, oldTable *model.TableInfo, newTimeStamp uint64, -) *restoreutil.RewriteRules { +) *RewriteRules { tableIDs := make(map[int64]int64) tableIDs[oldTable.ID] = newTable.ID if oldTable.Partition != nil { @@ -119,7 +118,7 @@ func GetRewriteRules( } } - return &restoreutil.RewriteRules{ + return &RewriteRules{ Table: tableRules, Data: dataRules, } @@ -196,9 +195,9 @@ func withRetry( // ValidateFileRanges checks and returns the ranges of the files. func ValidateFileRanges( files []*backup.File, - rewriteRules *restoreutil.RewriteRules, -) ([]restoreutil.Range, error) { - ranges := make([]restoreutil.Range, 0, len(files)) + rewriteRules *RewriteRules, +) ([]Range, error) { + ranges := make([]Range, 0, len(files)) fileAppended := make(map[string]bool) for _, file := range files { @@ -217,7 +216,7 @@ func ValidateFileRanges( zap.Stringer("file", file)) return nil, errors.New("table ids dont match") } - ranges = append(ranges, restoreutil.Range{ + ranges = append(ranges, Range{ StartKey: file.GetStartKey(), EndKey: file.GetEndKey(), }) @@ -228,7 +227,7 @@ func ValidateFileRanges( } // ValidateFileRewriteRule uses rewrite rules to validate the ranges of a file -func ValidateFileRewriteRule(file *backup.File, rewriteRules *restoreutil.RewriteRules) error { +func ValidateFileRewriteRule(file *backup.File, rewriteRules *RewriteRules) error { // Check if the start key has a matched rewrite key _, startRule := rewriteRawKey(file.GetStartKey(), rewriteRules) if rewriteRules != nil && startRule == nil { @@ -269,7 +268,7 @@ func ValidateFileRewriteRule(file *backup.File, rewriteRules *restoreutil.Rewrit } // Rewrites a raw key and returns a encoded key -func rewriteRawKey(key []byte, rewriteRules *restoreutil.RewriteRules) ([]byte, *import_sstpb.RewriteRule) { +func rewriteRawKey(key []byte, rewriteRules *RewriteRules) ([]byte, *import_sstpb.RewriteRule) { if rewriteRules == nil { return codec.EncodeBytes([]byte{}, key), nil } @@ -281,7 +280,7 @@ func rewriteRawKey(key []byte, rewriteRules *restoreutil.RewriteRules) ([]byte, return nil, nil } -func matchOldPrefix(key []byte, rewriteRules *restoreutil.RewriteRules) *import_sstpb.RewriteRule { +func matchOldPrefix(key []byte, rewriteRules *RewriteRules) *import_sstpb.RewriteRule { for _, rule := range rewriteRules.Data { if bytes.HasPrefix(key, rule.GetOldKeyPrefix()) { return rule @@ -295,7 +294,7 @@ func matchOldPrefix(key []byte, rewriteRules *restoreutil.RewriteRules) *import_ return nil } -func matchNewPrefix(key []byte, rewriteRules *restoreutil.RewriteRules) *import_sstpb.RewriteRule { +func matchNewPrefix(key []byte, rewriteRules *RewriteRules) *import_sstpb.RewriteRule { for _, rule := range rewriteRules.Data { if bytes.HasPrefix(key, rule.GetNewKeyPrefix()) { return rule @@ -319,8 +318,8 @@ func truncateTS(key []byte) []byte { func SplitRanges( ctx context.Context, client *Client, - ranges []restoreutil.Range, - rewriteRules *restoreutil.RewriteRules, + ranges []Range, + rewriteRules *RewriteRules, updateCh chan<- struct{}, ) error { start := time.Now() @@ -328,7 +327,7 @@ func SplitRanges( elapsed := time.Since(start) summary.CollectDuration("split region", elapsed) }() - splitter := restoreutil.NewRegionSplitter(restoreutil.NewSplitClient(client.GetPDClient())) + splitter := NewRegionSplitter(NewSplitClient(client.GetPDClient())) return splitter.Split(ctx, ranges, rewriteRules, func(keys [][]byte) { for range keys { updateCh <- struct{}{} @@ -336,7 +335,7 @@ func SplitRanges( }) } -func rewriteFileKeys(file *backup.File, rewriteRules *restoreutil.RewriteRules) (startKey, endKey []byte, err error) { +func rewriteFileKeys(file *backup.File, rewriteRules *RewriteRules) (startKey, endKey []byte, err error) { startID := tablecodec.DecodeTableID(file.GetStartKey()) endID := tablecodec.DecodeTableID(file.GetEndKey()) var rule *import_sstpb.RewriteRule diff --git a/pkg/restore/util_test.go b/pkg/restore/util_test.go index d0c1704d7..bc4da9168 100644 --- a/pkg/restore/util_test.go +++ b/pkg/restore/util_test.go @@ -6,8 +6,6 @@ import ( "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/tablecodec" - - restoreutil "github.com/pingcap/br/pkg/restoreutil" ) var _ = Suite(&testRestoreUtilSuite{}) @@ -35,7 +33,7 @@ func (s *testRestoreUtilSuite) TestGetSSTMetaFromFile(c *C) { } func (s *testRestoreUtilSuite) TestValidateFileRanges(c *C) { - rules := &restoreutil.RewriteRules{ + rules := &RewriteRules{ Table: []*import_sstpb.RewriteRule{&import_sstpb.RewriteRule{ OldKeyPrefix: []byte(tablecodec.EncodeTablePrefix(1)), NewKeyPrefix: []byte(tablecodec.EncodeTablePrefix(2)), From bafcbc130c6db126022384e81338dab5a64470e7 Mon Sep 17 00:00:00 2001 From: Neil Shen Date: Sun, 19 Jan 2020 16:12:37 +0800 Subject: [PATCH 17/17] address comment Signed-off-by: Neil Shen --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 3a7cfa063..1b025678e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,7 +10,7 @@ issues: linters: - gosec # TODO Remove it. - - path: client.go + - path: split_client.go text: "SA1019:" linters: - staticcheck