From 3dd01f329ebe303f224ded889fd12ef12cf1bfb5 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 6 Dec 2021 16:36:53 +0800 Subject: [PATCH 1/4] make pre-check output message clearer --- br/pkg/lightning/common/storage_unix.go | 7 ++- br/pkg/lightning/common/storage_windows.go | 4 ++ br/pkg/lightning/restore/check_info.go | 36 +++++++-------- br/pkg/lightning/restore/check_info_test.go | 50 +++++++++++++++++++++ br/pkg/lightning/restore/check_template.go | 18 ++++---- br/pkg/lightning/restore/restore.go | 3 +- 6 files changed, 86 insertions(+), 32 deletions(-) diff --git a/br/pkg/lightning/common/storage_unix.go b/br/pkg/lightning/common/storage_unix.go index 465bc912a373b..2cf9b9f2c4b14 100644 --- a/br/pkg/lightning/common/storage_unix.go +++ b/br/pkg/lightning/common/storage_unix.go @@ -20,6 +20,7 @@ package common import ( + "github.com/pingcap/failpoint" "reflect" "syscall" @@ -29,8 +30,12 @@ import ( // GetStorageSize gets storage's capacity and available size func GetStorageSize(dir string) (size StorageSize, err error) { - var stat unix.Statfs_t + failpoint.Inject("GetStorageSize", func(val failpoint.Value) { + injectedSize := val.(int) + failpoint.Return(StorageSize{Capacity: uint64(injectedSize), Available: uint64(injectedSize)}, nil) + }) + var stat unix.Statfs_t err = unix.Statfs(dir, &stat) if err != nil { return size, errors.Annotatef(err, "cannot get disk capacity at %s", dir) diff --git a/br/pkg/lightning/common/storage_windows.go b/br/pkg/lightning/common/storage_windows.go index 737f21acf8c44..c72c0271704cd 100644 --- a/br/pkg/lightning/common/storage_windows.go +++ b/br/pkg/lightning/common/storage_windows.go @@ -33,6 +33,10 @@ var ( // GetStorageSize gets storage's capacity and available size func GetStorageSize(dir string) (size StorageSize, err error) { + failpoint.Inject("GetStorageSize", func(val failpoint.Value) { + injectedSize := val.(int) + failpoint.Return(StorageSize{Capacity: uint64(injectedSize), Available: uint64(injectedSize)}, nil) + }) r, _, e := getDiskFreeSpaceExW.Call( uintptr(unsafe.Pointer(syscall.StringToUTF16Ptr(dir))), uintptr(unsafe.Pointer(&size.Available)), diff --git a/br/pkg/lightning/restore/check_info.go b/br/pkg/lightning/restore/check_info.go index 49a8a8a9427d1..f97afc33b7cd0 100644 --- a/br/pkg/lightning/restore/check_info.go +++ b/br/pkg/lightning/restore/check_info.go @@ -464,33 +464,31 @@ func (rc *Controller) localResource(sourceSize int64) error { if err != nil { return errors.Trace(err) } - localAvailable := storageSize.Available + localAvailable := int64(storageSize.Available) var message string var passed bool switch { - case localAvailable > uint64(sourceSize): + case localAvailable > sourceSize: message = fmt.Sprintf("local disk resources are rich, estimate sorted data size %s, local available is %s", units.BytesSize(float64(sourceSize)), units.BytesSize(float64(localAvailable))) passed = true + case int64(rc.cfg.TikvImporter.DiskQuota) > localAvailable: + message = fmt.Sprintf("local disk space may not enough to finish import, estimate sorted data size is %s,"+ + " but local available is %s, please set `tikv-importer.disk-quota` to a smaller value than %s"+ + " or change `mydumper.sorted-kv-dir` to another disk with enough space to finish imports", + units.BytesSize(float64(sourceSize)), + units.BytesSize(float64(localAvailable)), units.BytesSize(float64(localAvailable))) + passed = false + log.L().Error(message) default: - if int64(rc.cfg.TikvImporter.DiskQuota) > int64(localAvailable) { - message = fmt.Sprintf("local disk space may not enough to finish import"+ - "estimate sorted data size is %s, but local available is %s,"+ - "you need a smaller number for tikv-importer.disk-quota (%s) to finish imports", - units.BytesSize(float64(sourceSize)), - units.BytesSize(float64(localAvailable)), units.BytesSize(float64(rc.cfg.TikvImporter.DiskQuota))) - passed = false - log.L().Error(message) - } else { - message = fmt.Sprintf("local disk space may not enough to finish import, "+ - "estimate sorted data size is %s, but local available is %s,"+ - "we will use disk-quota (size: %s) to finish imports, which may slow down import", - units.BytesSize(float64(sourceSize)), - units.BytesSize(float64(localAvailable)), units.BytesSize(float64(rc.cfg.TikvImporter.DiskQuota))) - passed = true - log.L().Warn(message) - } + message = fmt.Sprintf("local disk space may not enough to finish import, "+ + "estimate sorted data size is %s, but local available is %s,"+ + "we will use disk-quota (size: %s) to finish imports, which may slow down import", + units.BytesSize(float64(sourceSize)), + units.BytesSize(float64(localAvailable)), units.BytesSize(float64(rc.cfg.TikvImporter.DiskQuota))) + passed = true + log.L().Warn(message) } rc.checkTemplate.Collect(Critical, passed, message) return nil diff --git a/br/pkg/lightning/restore/check_info_test.go b/br/pkg/lightning/restore/check_info_test.go index e1dd939d9c2b1..6e99ea2fb3baa 100644 --- a/br/pkg/lightning/restore/check_info_test.go +++ b/br/pkg/lightning/restore/check_info_test.go @@ -17,6 +17,7 @@ package restore import ( "context" "fmt" + "github.com/pingcap/failpoint" "os" "path/filepath" @@ -401,5 +402,54 @@ func (s *checkInfoSuite) TestCheckCSVHeader(c *C) { c.Assert(rc.checkTemplate.FailedCount(ca.level), Equals, 1) } } +} + +func (s *checkInfoSuite) TestLocalResource(c *C) { + dir := c.MkDir() + mockStore, err := storage.NewLocalStorage(dir) + c.Assert(err, IsNil) + + err = failpoint.Enable("github.com/pingcap/tidb/br/pkg/lightning/common/GetStorageSize", "return(2048)") + c.Assert(err, IsNil) + defer func() { + _ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/lightning/common/GetStorageSize") + }() + + cfg := config.NewConfig() + cfg.Mydumper.SourceDir = dir + cfg.TikvImporter.SortedKVDir = dir + cfg.TikvImporter.Backend = "local" + rc := &Controller{ + cfg: cfg, + store: mockStore, + ioWorkers: worker.NewPool(context.Background(), 1, "io"), + } + // 1. source-size is smaller than disk-size, won't trigger error information + rc.checkTemplate = NewSimpleTemplate() + err = rc.localResource(1000) + c.Assert(err, IsNil) + tmpl := rc.checkTemplate.(*SimpleTemplate) + c.Assert(tmpl.warnFailedCount, Equals, 1) + c.Assert(tmpl.criticalFailedCount, Equals, 0) + c.Assert(tmpl.failedMsg[1], Matches, "local disk resources are rich, estimate sorted data size 1000B, local available is 2KiB") + + // 2. source-size is bigger than disk-size, with default disk-quota will trigger a critical error + rc.checkTemplate = NewSimpleTemplate() + err = rc.localResource(4096) + c.Assert(err, IsNil) + tmpl = rc.checkTemplate.(*SimpleTemplate) + c.Assert(tmpl.warnFailedCount, Equals, 1) + c.Assert(tmpl.criticalFailedCount, Equals, 1) + c.Assert(tmpl.failedMsg[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB, please set `tikv-importer.disk-quota` to a smaller value than 2KiB or change `mydumper.sorted-kv-dir` to another disk with enough space to finish imports") + + // 3. source-size is bigger than disk-size, with a vaild disk-quota will trigger a warning + rc.checkTemplate = NewSimpleTemplate() + rc.cfg.TikvImporter.DiskQuota = config.ByteSize(1024) + err = rc.localResource(4096) + c.Assert(err, IsNil) + tmpl = rc.checkTemplate.(*SimpleTemplate) + c.Assert(tmpl.warnFailedCount, Equals, 1) + c.Assert(tmpl.criticalFailedCount, Equals, 0) + c.Assert(tmpl.failedMsg[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB,we will use disk-quota \\(size: 1KiB\\) to finish imports, which may slow down import") } diff --git a/br/pkg/lightning/restore/check_template.go b/br/pkg/lightning/restore/check_template.go index 3fb8c22904caa..24b56da4f6074 100644 --- a/br/pkg/lightning/restore/check_template.go +++ b/br/pkg/lightning/restore/check_template.go @@ -51,7 +51,8 @@ type SimpleTemplate struct { count int warnFailedCount int criticalFailedCount int - failedMsg []string + warnMsgs []string + criticalMsgs []string t table.Writer } @@ -65,16 +66,12 @@ func NewSimpleTemplate() Template { {Name: "Passed", WidthMax: 6}, }) return &SimpleTemplate{ - 0, - 0, - 0, - make([]string, 0), - t, + t: t, } } func (c *SimpleTemplate) FailedMsg() string { - return strings.Join(c.failedMsg, ";\n") + return strings.Join(c.criticalMsgs, ";\n") } func (c *SimpleTemplate) Collect(t CheckType, passed bool, msg string) { @@ -83,11 +80,12 @@ func (c *SimpleTemplate) Collect(t CheckType, passed bool, msg string) { switch t { case Critical: c.criticalFailedCount++ + c.criticalMsgs = append(c.criticalMsgs, msg) case Warn: c.warnFailedCount++ + c.warnMsgs = append(c.warnMsgs, msg) } } - c.failedMsg = append(c.failedMsg, msg) c.t.AppendRow(table.Row{c.count, msg, t, passed}) c.t.AppendSeparator() } @@ -108,7 +106,7 @@ func (c *SimpleTemplate) FailedCount(t CheckType) int { func (c *SimpleTemplate) Output() string { c.t.SetAllowedRowLength(170) - c.t.SetRowPainter(table.RowPainter(func(row table.Row) text.Colors { + c.t.SetRowPainter(func(row table.Row) text.Colors { if passed, ok := row[3].(bool); ok { if !passed { if typ, ok := row[2].(CheckType); ok { @@ -122,7 +120,7 @@ func (c *SimpleTemplate) Output() string { } } return nil - })) + }) res := c.t.Render() summary := "\n" if c.criticalFailedCount > 0 { diff --git a/br/pkg/lightning/restore/restore.go b/br/pkg/lightning/restore/restore.go index bf417602bf4d2..beac56b10d4bb 100644 --- a/br/pkg/lightning/restore/restore.go +++ b/br/pkg/lightning/restore/restore.go @@ -1926,8 +1926,7 @@ func (rc *Controller) preCheckRequirements(ctx context.Context) error { if !taskExist && rc.taskMgr != nil { rc.taskMgr.CleanupTask(ctx) } - return errors.Errorf("tidb-lightning check failed."+ - " Please fix the failed check(s):\n %s", rc.checkTemplate.FailedMsg()) + return errors.Errorf("tidb-lightning pre-check failed: %s", rc.checkTemplate.FailedMsg()) } return nil } From dc5a9bb71d93a87021beddc8006a05c2ef1345f1 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 6 Dec 2021 16:53:54 +0800 Subject: [PATCH 2/4] fix test --- br/pkg/lightning/restore/check_info_test.go | 8 ++++---- br/pkg/lightning/restore/check_template.go | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/br/pkg/lightning/restore/check_info_test.go b/br/pkg/lightning/restore/check_info_test.go index 6e99ea2fb3baa..7b3a8c3e6ed89 100644 --- a/br/pkg/lightning/restore/check_info_test.go +++ b/br/pkg/lightning/restore/check_info_test.go @@ -17,11 +17,11 @@ package restore import ( "context" "fmt" - "github.com/pingcap/failpoint" "os" "path/filepath" . "github.com/pingcap/check" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/br/pkg/lightning/checkpoints" "github.com/pingcap/tidb/br/pkg/lightning/config" @@ -432,7 +432,7 @@ func (s *checkInfoSuite) TestLocalResource(c *C) { tmpl := rc.checkTemplate.(*SimpleTemplate) c.Assert(tmpl.warnFailedCount, Equals, 1) c.Assert(tmpl.criticalFailedCount, Equals, 0) - c.Assert(tmpl.failedMsg[1], Matches, "local disk resources are rich, estimate sorted data size 1000B, local available is 2KiB") + c.Assert(tmpl.warnMsgs[1], Matches, "local disk resources are rich, estimate sorted data size 1000B, local available is 2KiB") // 2. source-size is bigger than disk-size, with default disk-quota will trigger a critical error rc.checkTemplate = NewSimpleTemplate() @@ -441,7 +441,7 @@ func (s *checkInfoSuite) TestLocalResource(c *C) { tmpl = rc.checkTemplate.(*SimpleTemplate) c.Assert(tmpl.warnFailedCount, Equals, 1) c.Assert(tmpl.criticalFailedCount, Equals, 1) - c.Assert(tmpl.failedMsg[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB, please set `tikv-importer.disk-quota` to a smaller value than 2KiB or change `mydumper.sorted-kv-dir` to another disk with enough space to finish imports") + c.Assert(tmpl.criticalMsgs[0], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB, please set `tikv-importer.disk-quota` to a smaller value than 2KiB or change `mydumper.sorted-kv-dir` to another disk with enough space to finish imports") // 3. source-size is bigger than disk-size, with a vaild disk-quota will trigger a warning rc.checkTemplate = NewSimpleTemplate() @@ -451,5 +451,5 @@ func (s *checkInfoSuite) TestLocalResource(c *C) { tmpl = rc.checkTemplate.(*SimpleTemplate) c.Assert(tmpl.warnFailedCount, Equals, 1) c.Assert(tmpl.criticalFailedCount, Equals, 0) - c.Assert(tmpl.failedMsg[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB,we will use disk-quota \\(size: 1KiB\\) to finish imports, which may slow down import") + c.Assert(tmpl.warnMsgs[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB,we will use disk-quota \\(size: 1KiB\\) to finish imports, which may slow down import") } diff --git a/br/pkg/lightning/restore/check_template.go b/br/pkg/lightning/restore/check_template.go index 24b56da4f6074..f38e23aa00f8e 100644 --- a/br/pkg/lightning/restore/check_template.go +++ b/br/pkg/lightning/restore/check_template.go @@ -51,7 +51,7 @@ type SimpleTemplate struct { count int warnFailedCount int criticalFailedCount int - warnMsgs []string + normalMsgs []string // only used in unit test now criticalMsgs []string t table.Writer } @@ -80,12 +80,15 @@ func (c *SimpleTemplate) Collect(t CheckType, passed bool, msg string) { switch t { case Critical: c.criticalFailedCount++ - c.criticalMsgs = append(c.criticalMsgs, msg) case Warn: c.warnFailedCount++ - c.warnMsgs = append(c.warnMsgs, msg) } } + if !passed && t == Critical { + c.criticalMsgs = append(c.criticalMsgs, msg) + } else { + c.normalMsgs = append(c.normalMsgs, msg) + } c.t.AppendRow(table.Row{c.count, msg, t, passed}) c.t.AppendSeparator() } From d892f0acfef71095bd734518b24e894e8be1fd6f Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 6 Dec 2021 16:55:58 +0800 Subject: [PATCH 3/4] fix import --- br/pkg/lightning/common/storage_unix.go | 2 +- br/pkg/lightning/common/storage_windows.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/br/pkg/lightning/common/storage_unix.go b/br/pkg/lightning/common/storage_unix.go index 2cf9b9f2c4b14..6cbb59c6f4df0 100644 --- a/br/pkg/lightning/common/storage_unix.go +++ b/br/pkg/lightning/common/storage_unix.go @@ -20,11 +20,11 @@ package common import ( - "github.com/pingcap/failpoint" "reflect" "syscall" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "golang.org/x/sys/unix" ) diff --git a/br/pkg/lightning/common/storage_windows.go b/br/pkg/lightning/common/storage_windows.go index c72c0271704cd..8617e28d30893 100644 --- a/br/pkg/lightning/common/storage_windows.go +++ b/br/pkg/lightning/common/storage_windows.go @@ -24,6 +24,7 @@ import ( "unsafe" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" ) var ( From 1c69a1b76e941ad1a93d2f3834cd23124095c1dc Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 20 Dec 2021 18:59:05 +0800 Subject: [PATCH 4/4] fix unit test --- br/pkg/lightning/restore/check_info_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/br/pkg/lightning/restore/check_info_test.go b/br/pkg/lightning/restore/check_info_test.go index 7b3a8c3e6ed89..ccc4aa74c0c28 100644 --- a/br/pkg/lightning/restore/check_info_test.go +++ b/br/pkg/lightning/restore/check_info_test.go @@ -432,7 +432,7 @@ func (s *checkInfoSuite) TestLocalResource(c *C) { tmpl := rc.checkTemplate.(*SimpleTemplate) c.Assert(tmpl.warnFailedCount, Equals, 1) c.Assert(tmpl.criticalFailedCount, Equals, 0) - c.Assert(tmpl.warnMsgs[1], Matches, "local disk resources are rich, estimate sorted data size 1000B, local available is 2KiB") + c.Assert(tmpl.normalMsgs[1], Matches, "local disk resources are rich, estimate sorted data size 1000B, local available is 2KiB") // 2. source-size is bigger than disk-size, with default disk-quota will trigger a critical error rc.checkTemplate = NewSimpleTemplate() @@ -451,5 +451,5 @@ func (s *checkInfoSuite) TestLocalResource(c *C) { tmpl = rc.checkTemplate.(*SimpleTemplate) c.Assert(tmpl.warnFailedCount, Equals, 1) c.Assert(tmpl.criticalFailedCount, Equals, 0) - c.Assert(tmpl.warnMsgs[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB,we will use disk-quota \\(size: 1KiB\\) to finish imports, which may slow down import") + c.Assert(tmpl.normalMsgs[1], Matches, "local disk space may not enough to finish import, estimate sorted data size is 4KiB, but local available is 2KiB,we will use disk-quota \\(size: 1KiB\\) to finish imports, which may slow down import") }