From f25aabb4b367123b90cc939a66a383afb865b92f Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Thu, 20 Jul 2023 18:19:04 +0800 Subject: [PATCH 1/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/table_import.go | 13 +++++++++++++ br/tests/lightning_routes/config.toml | 3 +++ br/tests/lightning_routes/run.sh | 5 +++++ 3 files changed, 21 insertions(+) diff --git a/br/pkg/lightning/importer/table_import.go b/br/pkg/lightning/importer/table_import.go index 79a6ed296048c..f23c57211aa9b 100644 --- a/br/pkg/lightning/importer/table_import.go +++ b/br/pkg/lightning/importer/table_import.go @@ -19,6 +19,9 @@ import ( "database/sql" "encoding/hex" "fmt" + "github.com/pingcap/tidb/br/pkg/logutil" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "path/filepath" "strings" "sync" @@ -1034,6 +1037,16 @@ func (tr *TableImporter) postProcess( var remoteChecksum *local.RemoteChecksum remoteChecksum, err = DoChecksum(ctx, tr.tableInfo) + failpoint.Inject("checksum-error", func() { + logutil.CL(ctx).Debug("failpoint checksum-error injected.") + err = status.Error(codes.Unknown, "Checksum meets error.") + }) + if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { + if err != nil { + tr.logger.Warn("do checksum failed, will skip this error and go on", log.ShortError(err)) + err = nil + } + } if err != nil { return false, err } diff --git a/br/tests/lightning_routes/config.toml b/br/tests/lightning_routes/config.toml index bb54609dd03b1..74913091c5916 100644 --- a/br/tests/lightning_routes/config.toml +++ b/br/tests/lightning_routes/config.toml @@ -8,3 +8,6 @@ schema-pattern = "routes_a*" table-pattern = "t*" target-schema = "routes_b" target-table = "u" + +[post-restore] +checksum = "optional" diff --git a/br/tests/lightning_routes/run.sh b/br/tests/lightning_routes/run.sh index 1db0ce2035021..ba6ad98b8cf88 100755 --- a/br/tests/lightning_routes/run.sh +++ b/br/tests/lightning_routes/run.sh @@ -4,12 +4,17 @@ set -eux +echo "testing checksum-error..." +export GO_FAILPOINTS="github.com/pingcap/tidb/br/pkg/lightning/importer/checksum-error=1*return(true)" + run_sql 'DROP DATABASE IF EXISTS routes_a0;' run_sql 'DROP DATABASE IF EXISTS routes_a1;' run_sql 'DROP DATABASE IF EXISTS routes_b;' run_lightning +echo "test checksum-error success!" + run_sql 'SELECT count(1), sum(x) FROM routes_b.u;' check_contains 'count(1): 4' check_contains 'sum(x): 259' From 7caa461c3a17f3fefeface77c05ec6b5548832b1 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Thu, 20 Jul 2023 18:40:50 +0800 Subject: [PATCH 2/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/BUILD.bazel | 3 +++ 1 file changed, 3 insertions(+) diff --git a/br/pkg/lightning/importer/BUILD.bazel b/br/pkg/lightning/importer/BUILD.bazel index 0ed8a538dc2f8..774a392ce5e10 100644 --- a/br/pkg/lightning/importer/BUILD.bazel +++ b/br/pkg/lightning/importer/BUILD.bazel @@ -39,6 +39,7 @@ go_library( "//br/pkg/lightning/verification", "//br/pkg/lightning/web", "//br/pkg/lightning/worker", + "//br/pkg/logutil", "//br/pkg/pdutil", "//br/pkg/redact", "//br/pkg/storage", @@ -86,6 +87,8 @@ go_library( "@com_github_tikv_pd_client//:client", "@io_etcd_go_etcd_client_v3//:client", "@org_golang_google_grpc//:grpc", + "@org_golang_google_grpc//codes", + "@org_golang_google_grpc//status", "@org_golang_x_exp//maps", "@org_golang_x_exp//slices", "@org_golang_x_sync//errgroup", From 0f060b610dd9b62afeb13b7f01f60abbed306a73 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Thu, 20 Jul 2023 20:47:18 +0800 Subject: [PATCH 3/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/table_import.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/br/pkg/lightning/importer/table_import.go b/br/pkg/lightning/importer/table_import.go index f23c57211aa9b..f8b4d5024a01d 100644 --- a/br/pkg/lightning/importer/table_import.go +++ b/br/pkg/lightning/importer/table_import.go @@ -19,9 +19,6 @@ import ( "database/sql" "encoding/hex" "fmt" - "github.com/pingcap/tidb/br/pkg/logutil" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "path/filepath" "strings" "sync" @@ -43,6 +40,7 @@ import ( verify "github.com/pingcap/tidb/br/pkg/lightning/verification" "github.com/pingcap/tidb/br/pkg/lightning/web" "github.com/pingcap/tidb/br/pkg/lightning/worker" + "github.com/pingcap/tidb/br/pkg/logutil" "github.com/pingcap/tidb/br/pkg/version" "github.com/pingcap/tidb/errno" tidbkv "github.com/pingcap/tidb/kv" @@ -57,6 +55,8 @@ import ( "go.uber.org/multierr" "go.uber.org/zap" "golang.org/x/exp/slices" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // TableImporter is a helper struct to import a table. From 4f24e14d2b894ce5baf0480b4975f01d663f24a8 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Mon, 24 Jul 2023 12:56:37 +0800 Subject: [PATCH 4/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/table_import.go | 18 ++++++++++-------- br/tests/lightning_routes/run.sh | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/br/pkg/lightning/importer/table_import.go b/br/pkg/lightning/importer/table_import.go index f8b4d5024a01d..0377e1e4e445e 100644 --- a/br/pkg/lightning/importer/table_import.go +++ b/br/pkg/lightning/importer/table_import.go @@ -40,7 +40,6 @@ import ( verify "github.com/pingcap/tidb/br/pkg/lightning/verification" "github.com/pingcap/tidb/br/pkg/lightning/web" "github.com/pingcap/tidb/br/pkg/lightning/worker" - "github.com/pingcap/tidb/br/pkg/logutil" "github.com/pingcap/tidb/br/pkg/version" "github.com/pingcap/tidb/errno" tidbkv "github.com/pingcap/tidb/kv" @@ -1038,7 +1037,8 @@ func (tr *TableImporter) postProcess( var remoteChecksum *local.RemoteChecksum remoteChecksum, err = DoChecksum(ctx, tr.tableInfo) failpoint.Inject("checksum-error", func() { - logutil.CL(ctx).Debug("failpoint checksum-error injected.") + tr.logger.Info("failpoint checksum-error injected.") + remoteChecksum = nil err = status.Error(codes.Unknown, "Checksum meets error.") }) if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { @@ -1050,12 +1050,14 @@ func (tr *TableImporter) postProcess( if err != nil { return false, err } - err = tr.compareChecksum(remoteChecksum, localChecksum) - // with post restore level 'optional', we will skip checksum error - if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { - if err != nil { - tr.logger.Warn("compare checksum failed, will skip this error and go on", log.ShortError(err)) - err = nil + if remoteChecksum != nil { + err = tr.compareChecksum(remoteChecksum, localChecksum) + // with post restore level 'optional', we will skip checksum error + if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { + if err != nil { + tr.logger.Warn("compare checksum failed, will skip this error and go on", log.ShortError(err)) + err = nil + } } } } else { diff --git a/br/tests/lightning_routes/run.sh b/br/tests/lightning_routes/run.sh index ba6ad98b8cf88..5ae757eb0bd43 100755 --- a/br/tests/lightning_routes/run.sh +++ b/br/tests/lightning_routes/run.sh @@ -5,7 +5,7 @@ set -eux echo "testing checksum-error..." -export GO_FAILPOINTS="github.com/pingcap/tidb/br/pkg/lightning/importer/checksum-error=1*return(true)" +export GO_FAILPOINTS="github.com/pingcap/tidb/br/pkg/lightning/importer/checksum-error=1*return()" run_sql 'DROP DATABASE IF EXISTS routes_a0;' run_sql 'DROP DATABASE IF EXISTS routes_a1;' From 45bb0136c3ec0fbac575795389ecff1b0641ed9b Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Mon, 24 Jul 2023 14:54:35 +0800 Subject: [PATCH 5/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/br/pkg/lightning/importer/BUILD.bazel b/br/pkg/lightning/importer/BUILD.bazel index 774a392ce5e10..0c43b6394b8fc 100644 --- a/br/pkg/lightning/importer/BUILD.bazel +++ b/br/pkg/lightning/importer/BUILD.bazel @@ -39,7 +39,6 @@ go_library( "//br/pkg/lightning/verification", "//br/pkg/lightning/web", "//br/pkg/lightning/worker", - "//br/pkg/logutil", "//br/pkg/pdutil", "//br/pkg/redact", "//br/pkg/storage", From bfc940a0bd383bf5abd7054a6c25434245fd41d7 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Mon, 24 Jul 2023 17:02:23 +0800 Subject: [PATCH 6/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/table_import.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/br/pkg/lightning/importer/table_import.go b/br/pkg/lightning/importer/table_import.go index 0377e1e4e445e..8dbe47d0e2a9d 100644 --- a/br/pkg/lightning/importer/table_import.go +++ b/br/pkg/lightning/importer/table_import.go @@ -1041,13 +1041,12 @@ func (tr *TableImporter) postProcess( remoteChecksum = nil err = status.Error(codes.Unknown, "Checksum meets error.") }) - if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { - if err != nil { + if err != nil { + if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { tr.logger.Warn("do checksum failed, will skip this error and go on", log.ShortError(err)) err = nil } - } - if err != nil { + } else { return false, err } if remoteChecksum != nil { From 39d8c160f97cefbc1098ff0d768aebb486230cd1 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Mon, 24 Jul 2023 18:04:16 +0800 Subject: [PATCH 7/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- br/pkg/lightning/importer/table_import.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/br/pkg/lightning/importer/table_import.go b/br/pkg/lightning/importer/table_import.go index 8dbe47d0e2a9d..9ca3f9e930425 100644 --- a/br/pkg/lightning/importer/table_import.go +++ b/br/pkg/lightning/importer/table_import.go @@ -1042,12 +1042,11 @@ func (tr *TableImporter) postProcess( err = status.Error(codes.Unknown, "Checksum meets error.") }) if err != nil { - if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { - tr.logger.Warn("do checksum failed, will skip this error and go on", log.ShortError(err)) - err = nil + if rc.cfg.PostRestore.Checksum != config.OpLevelOptional { + return false, err } - } else { - return false, err + tr.logger.Warn("do checksum failed, will skip this error and go on", log.ShortError(err)) + err = nil } if remoteChecksum != nil { err = tr.compareChecksum(remoteChecksum, localChecksum) From 3b801d94182bdf9f9f34825064c70c4a5b412ca8 Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Wed, 2 Aug 2023 14:51:09 +0800 Subject: [PATCH 8/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- disttask/importinto/subtask_executor.go | 30 +++++++++++++++---------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/disttask/importinto/subtask_executor.go b/disttask/importinto/subtask_executor.go index be6de9a75d0c0..5f69902a03d36 100644 --- a/disttask/importinto/subtask_executor.go +++ b/disttask/importinto/subtask_executor.go @@ -120,21 +120,27 @@ func verifyChecksum(ctx context.Context, taskMeta *TaskMeta, subtaskMeta *PostPr } remoteChecksum, err := checksumTable(ctx, globalTaskManager, taskMeta, logger) if err != nil { - return err + if taskMeta.Plan.Checksum != config.OpLevelOptional { + return err + } + logger.Warn("checksumTable failed, will skip this error and go on", zap.Error(err)) + err = nil } - if !remoteChecksum.IsEqual(&localChecksum) { - err2 := common.ErrChecksumMismatch.GenWithStackByArgs( - remoteChecksum.Checksum, localChecksum.Sum(), - remoteChecksum.TotalKVs, localChecksum.SumKVS(), - remoteChecksum.TotalBytes, localChecksum.SumSize(), - ) - if taskMeta.Plan.Checksum == config.OpLevelOptional { - logger.Warn("verify checksum failed, but checksum is optional, will skip it", zap.Error(err2)) - err2 = nil + if remoteChecksum != nil { + if !remoteChecksum.IsEqual(&localChecksum) { + err2 := common.ErrChecksumMismatch.GenWithStackByArgs( + remoteChecksum.Checksum, localChecksum.Sum(), + remoteChecksum.TotalKVs, localChecksum.SumKVS(), + remoteChecksum.TotalBytes, localChecksum.SumSize(), + ) + if taskMeta.Plan.Checksum == config.OpLevelOptional { + logger.Warn("verify checksum failed, but checksum is optional, will skip it", zap.Error(err2)) + err2 = nil + } + return err2 } - return err2 + logger.Info("checksum pass", zap.Object("local", &localChecksum)) } - logger.Info("checksum pass", zap.Object("local", &localChecksum)) return nil } From fec2e13d0765884625e29cccd53a0f1713a63ddc Mon Sep 17 00:00:00 2001 From: lyzx2001 Date: Wed, 2 Aug 2023 15:00:23 +0800 Subject: [PATCH 9/9] lightning: make OpLevelOptional suppress the error of DoChecksum --- disttask/importinto/subtask_executor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/disttask/importinto/subtask_executor.go b/disttask/importinto/subtask_executor.go index 5f69902a03d36..ebf3c37ab26ac 100644 --- a/disttask/importinto/subtask_executor.go +++ b/disttask/importinto/subtask_executor.go @@ -124,7 +124,6 @@ func verifyChecksum(ctx context.Context, taskMeta *TaskMeta, subtaskMeta *PostPr return err } logger.Warn("checksumTable failed, will skip this error and go on", zap.Error(err)) - err = nil } if remoteChecksum != nil { if !remoteChecksum.IsEqual(&localChecksum) {