Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

restore: add error field to DownloadResponse #195

Merged
merged 7 commits into from
Mar 25, 2020
Merged

restore: add error field to DownloadResponse #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c0b72e
restore: add error field to `DownloadResponse`
5kbpers Mar 16, 2020
da897b2
add test
5kbpers Mar 17, 2020
b47fe77
Merge branch 'master' into download-error
5kbpers Mar 20, 2020
fdeadbf
address comments
5kbpers Mar 20, 2020
8bb16f7
Merge branch 'master' into download-error
overvenus Mar 21, 2020
6407c96
Merge branch 'master' into download-error
overvenus Mar 24, 2020
a21d1ec
Merge branch 'master' into download-error
overvenus Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989/go.mod h1:O17Xtb
github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w=
github.com/pingcap/kvproto v0.0.0-20200214064158-62d31900d88e/go.mod h1:IOdRDPLyda8GX2hE/jO7gqaCV/PNFh8BZQCQZXfIOqI=
github.com/pingcap/kvproto v0.0.0-20200221034943-a2aa1d1e20a8/go.mod h1:IOdRDPLyda8GX2hE/jO7gqaCV/PNFh8BZQCQZXfIOqI=
github.com/pingcap/kvproto v0.0.0-20200228095611-2cf9a243b8d5 h1:knEvP4R5v5b2T107/Q6VzB0C8/6T7NXB/V7Vl1FtQsg=
github.com/pingcap/kvproto v0.0.0-20200228095611-2cf9a243b8d5/go.mod h1:IOdRDPLyda8GX2hE/jO7gqaCV/PNFh8BZQCQZXfIOqI=
github.com/pingcap/kvproto v0.0.0-20200317112120-78042b285b75 h1:DB3NTM0ilba/6sW+vccdEnP10bVvrVunDwWvRa0hSKc=
github.com/pingcap/kvproto v0.0.0-20200317112120-78042b285b75/go.mod h1:IOdRDPLyda8GX2hE/jO7gqaCV/PNFh8BZQCQZXfIOqI=
Expand Down
13 changes: 3 additions & 10 deletions pkg/restore/backoff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,11 @@ import (
var (
errEpochNotMatch = errors.NewNoStackError("epoch not match")
errKeyNotInRegion = errors.NewNoStackError("key not in region")
errRegionNotFound = errors.NewNoStackError("region not found")
errResp = errors.NewNoStackError("response error")
errRewriteRuleNotFound = errors.NewNoStackError("rewrite rule not found")
errRangeIsEmpty = errors.NewNoStackError("range is empty")
errGrpc = errors.NewNoStackError("gRPC error")

// TODO: add `error` field to `DownloadResponse` for distinguish the errors of gRPC
// and the errors of request
errBadFormat = errors.NewNoStackError("bad format")
errWrongKeyPrefix = errors.NewNoStackError("wrong key prefix")
errFileCorrupted = errors.NewNoStackError("file corrupted")
errCannotRead = errors.NewNoStackError("cannot read externel storage")
errDownloadFailed = errors.NewNoStackError("download sst failed")
errIngestFailed = errors.NewNoStackError("ingest sst failed")
)

const (
Expand Down Expand Up @@ -67,7 +60,7 @@ func newDownloadSSTBackoffer() utils.Backoffer {

func (bo *importerBackoffer) NextBackoff(err error) time.Duration {
switch errors.Cause(err) {
case errResp, errGrpc, errEpochNotMatch:
case errGrpc, errEpochNotMatch, errIngestFailed:
Copy link
Member

Choose a reason for hiding this comment

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

Should we retry onerrRewriteRuleNotFound?

TiKV can return region not found after tikv/tikv#7135

Copy link
Contributor Author

Choose a reason for hiding this comment

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

errRewriteRuleNotFound is for handling with the region which exists in the range [t{tableid}, t{tableid+1}), but don't match to any rewrite rule, such as the region [t{tableid}, t{tableid}_i1). We should skip them.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, I mean errRegionNotFound (errRewriteRuleNotFound is a typo 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now errRegionNotFound is included in errIngestFailed

bo.delayTime = 2 * bo.delayTime
bo.attempt--
case errRangeIsEmpty, errRewriteRuleNotFound:
Expand Down
6 changes: 3 additions & 3 deletions pkg/restore/backoff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (s *testBackofferSuite) TestImporterBackoffer(c *C) {
case 0:
return errGrpc
case 1:
return errResp
return errEpochNotMatch
case 2:
return errRangeIsEmpty
}
Expand All @@ -54,8 +54,8 @@ func (s *testBackofferSuite) TestImporterBackoffer(c *C) {
}
err = utils.WithRetry(context.Background(), func() error {
defer func() { counter++ }()
return errResp
return errEpochNotMatch
}, &backoffer)
c.Assert(counter, Equals, 10)
c.Assert(err, Equals, errResp)
c.Assert(err, Equals, errEpochNotMatch)
}
32 changes: 10 additions & 22 deletions pkg/restore/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"bytes"
"context"
"crypto/tls"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -272,14 +271,12 @@ func (importer *FileImporter) Import(file *backup.File, rewriteRules *RewriteRul
// 2. retry ingest
errIngest = errors.AddStack(errEpochNotMatch)
break ingestRetry
case errPb.RegionNotFound != nil:
errIngest = errors.AddStack(errRegionNotFound)
break ingestRetry
case errPb.KeyNotInRegion != nil:
errIngest = errors.AddStack(errKeyNotInRegion)
break ingestRetry
default:
errIngest = errors.Errorf("ingest error %s", errPb)
// Other errors like `ServerIsBusy`, `RegionNotFound`, etc. should be retryable
errIngest = errors.Annotatef(errIngestFailed, "ingest error %s", errPb)
break ingestRetry
}
}
Expand Down Expand Up @@ -346,7 +343,10 @@ func (importer *FileImporter) downloadSST(
for _, peer := range regionInfo.Region.GetPeers() {
resp, err = importer.importClient.DownloadSST(importer.ctx, peer.GetStoreId(), req)
if err != nil {
return nil, extractDownloadSSTError(err)
return nil, errors.Annotatef(errGrpc, "%s", err)
}
if resp.GetError() != nil {
return nil, errors.Annotate(errDownloadFailed, resp.GetError().GetMessage())
}
if resp.GetIsEmpty() {
return nil, errors.Trace(errRangeIsEmpty)
Expand Down Expand Up @@ -395,7 +395,10 @@ func (importer *FileImporter) downloadRawKVSST(
for _, peer := range regionInfo.Region.GetPeers() {
resp, err = importer.importClient.DownloadSST(importer.ctx, peer.GetStoreId(), req)
if err != nil {
return nil, extractDownloadSSTError(err)
return nil, errors.Annotatef(errGrpc, "%s", err)
}
if resp.GetError() != nil {
return nil, errors.Annotate(errDownloadFailed, resp.GetError().GetMessage())
}
if resp.GetIsEmpty() {
return nil, errors.Trace(errRangeIsEmpty)
Expand Down Expand Up @@ -439,18 +442,3 @@ func checkRegionEpoch(new, old *RegionInfo) bool {
}
return false
}

func extractDownloadSSTError(e error) error {
err := errGrpc
switch {
case strings.Contains(e.Error(), "bad format"):
err = errBadFormat
case strings.Contains(e.Error(), "wrong prefix"):
err = errWrongKeyPrefix
case strings.Contains(e.Error(), "corrupted"):
err = errFileCorrupted
case strings.Contains(e.Error(), "Cannot read"):
err = errCannotRead
}
return errors.Annotatef(err, "%s", e)
}
9 changes: 9 additions & 0 deletions tests/br_move_backup/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ run_sql "DROP TABLE $DB.$TABLE;"
# change backup path
mv $TEST_DIR/$DB $TEST_DIR/another$DB

# restore table with old path
echo "restore with old path start..."
run_br restore table --db $DB --table $TABLE -s "local://$TEST_DIR/$DB" --pd $PD_ADDR || restore_old_fail=1

if [ "$restore_old_fail" -ne "1" ];then
echo "TEST: [$TEST_NAME] test restore with old path failed!"
exit 1
fi

# restore table
echo "restore start..."
run_br restore table --db $DB --table $TABLE -s "local://$TEST_DIR/another$DB" --pd $PD_ADDR
Expand Down