From dbcfb32b370cab7aa656c933f1430807d454903c Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Tue, 6 Jun 2023 17:50:45 +0800 Subject: [PATCH 1/2] refactor error Signed-off-by: Cabinfever_B --- executor/calibrate_resource.go | 21 ++++++++----- executor/calibrate_resource_test.go | 47 ++++++++++++++++++----------- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/executor/calibrate_resource.go b/executor/calibrate_resource.go index 0ca6f5cba29f9..63f935127d45d 100644 --- a/executor/calibrate_resource.go +++ b/executor/calibrate_resource.go @@ -196,6 +196,11 @@ func (e *calibrateResourceExec) Next(ctx context.Context, req *chunk.Chunk) erro return e.staticCalibrate(ctx, req, exec) } +var ( + errTooFewMetricsPoints = errors.Normalize("There are too few metrics points available in selected time window, %v") + errNoCPUQuotaMetrics = errors.Normalize("There is no CPU quota metrics, %v") +) + func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk.Chunk, exec sqlexec.RestrictedSQLExecutor) error { startTs, endTs, err := e.parseCalibrateDuration(ctx) if err != nil { @@ -206,23 +211,23 @@ func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk totalKVCPUQuota, err := getTiKVTotalCPUQuota(ctx, exec) if err != nil { - return err + return errNoCPUQuotaMetrics.FastGenByArgs(err.Error()) } totalTiDBCPU, err := getTiDBTotalCPUQuota(ctx, exec) if err != nil { - return err + return errNoCPUQuotaMetrics.FastGenByArgs(err.Error()) } rus, err := getRUPerSec(ctx, e.ctx, exec, startTime, endTime) if err != nil { - return err + return errTooFewMetricsPoints.FastGenByArgs(err.Error()) } tikvCPUs, err := getComponentCPUUsagePerSec(ctx, e.ctx, exec, "tikv", startTime, endTime) if err != nil { - return err + return errTooFewMetricsPoints.FastGenByArgs(err.Error()) } tidbCPUs, err := getComponentCPUUsagePerSec(ctx, e.ctx, exec, "tidb", startTime, endTime) if err != nil { - return err + return errTooFewMetricsPoints.FastGenByArgs(err.Error()) } quotas := make([]float64, 0) lowCount := 0 @@ -256,7 +261,7 @@ func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk tikvCPUs.next() } if len(quotas) < 5 { - return errors.Errorf("There are too few metrics points available in selected time window") + return errTooFewMetricsPoints.FastGenByArgs("low usage") } if float64(len(quotas))/float64(len(quotas)+lowCount) > percentOfPass { sort.Slice(quotas, func(i, j int) bool { @@ -287,11 +292,11 @@ func (e *calibrateResourceExec) staticCalibrate(ctx context.Context, req *chunk. totalKVCPUQuota, err := getTiKVTotalCPUQuota(ctx, exec) if err != nil { - return err + return errNoCPUQuotaMetrics.FastGenByArgs(err.Error()) } totalTiDBCPU, err := getTiDBTotalCPUQuota(ctx, exec) if err != nil { - return err + return errNoCPUQuotaMetrics.FastGenByArgs(err.Error()) } // The default workload to calculate the RU capacity. diff --git a/executor/calibrate_resource_test.go b/executor/calibrate_resource_test.go index f4850f9d056c8..c5f2ed37c3309 100644 --- a/executor/calibrate_resource_test.go +++ b/executor/calibrate_resource_test.go @@ -95,24 +95,30 @@ func TestCalibrateResource(t *testing.T) { return time } - mockData := map[string][][]types.Datum{ - "tikv_cpu_quota": { - types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-0", 8.0), - types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-1", 8.0), - types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-2", 8.0), - types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-0", 8.0), - types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-1", 8.0), - types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-2", 8.0), - }, - "tidb_server_maxprocs": { - types.MakeDatums(datetime("2020-02-12 10:35:00"), "tidb-0", 40.0), - types.MakeDatums(datetime("2020-02-12 10:36:00"), "tidb-0", 40.0), - }, - } + mockData := make(map[string][][]types.Datum) ctx := context.WithValue(context.Background(), "__mockMetricsTableData", mockData) ctx = failpoint.WithHook(ctx, func(_ context.Context, fpname string) bool { return fpName == fpname }) + rs, err = tk.Exec("CALIBRATE RESOURCE") + require.NoError(t, err) + require.NotNil(t, rs) + err = rs.Next(ctx, rs.NewChunk(nil)) + // because when mock metrics is empty, error is always `pd unavailable`, don't check detail. + require.ErrorContains(t, err, "There is no CPU quota metrics, query metric error: pd unavailable") + + mockData["tikv_cpu_quota"] = [][]types.Datum{ + types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-0", 8.0), + types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-1", 8.0), + types.MakeDatums(datetime("2020-02-12 10:35:00"), "tikv-2", 8.0), + types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-0", 8.0), + types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-1", 8.0), + types.MakeDatums(datetime("2020-02-12 10:36:00"), "tikv-2", 8.0), + } + mockData["tidb_server_maxprocs"] = [][]types.Datum{ + types.MakeDatums(datetime("2020-02-12 10:35:00"), "tidb-0", 40.0), + types.MakeDatums(datetime("2020-02-12 10:36:00"), "tidb-0", 40.0), + } tk.MustQueryWithContext(ctx, "CALIBRATE RESOURCE").Check(testkit.Rows("69768")) tk.MustQueryWithContext(ctx, "CALIBRATE RESOURCE WORKLOAD TPCC").Check(testkit.Rows("69768")) tk.MustQueryWithContext(ctx, "CALIBRATE RESOURCE WORKLOAD OLTP_READ_WRITE").Check(testkit.Rows("55823")) @@ -402,7 +408,7 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window") + require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") ru3 := [][]types.Datum{ types.MakeDatums(datetime("2020-02-12 10:25:00"), 2200.0), @@ -442,7 +448,7 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window") + require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") // flash back to init data. mockData["resource_manager_resource_unit"] = ru1 @@ -553,7 +559,14 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window") + require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") + + delete(mockData, "process_cpu_usage") + rs, err = tk.Exec("CALIBRATE RESOURCE START_TIME '2020-02-12 10:35:00' END_TIME '2020-02-12 10:45:00'") + require.NoError(t, err) + require.NotNil(t, rs) + err = rs.Next(ctx, rs.NewChunk(nil)) + require.ErrorContains(t, err, "There are too few metrics points available in selected time window, query metric error: pd unavailable") } type mockResourceGroupProvider struct { From 3845106b25b56a617446f1ec388184d15f667fed Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Tue, 6 Jun 2023 20:40:56 +0800 Subject: [PATCH 2/2] address comment Signed-off-by: Cabinfever_B --- executor/calibrate_resource.go | 17 +++++++---------- executor/calibrate_resource_test.go | 8 ++++---- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/executor/calibrate_resource.go b/executor/calibrate_resource.go index 63f935127d45d..ef6e8424a3489 100644 --- a/executor/calibrate_resource.go +++ b/executor/calibrate_resource.go @@ -197,8 +197,8 @@ func (e *calibrateResourceExec) Next(ctx context.Context, req *chunk.Chunk) erro } var ( - errTooFewMetricsPoints = errors.Normalize("There are too few metrics points available in selected time window, %v") - errNoCPUQuotaMetrics = errors.Normalize("There is no CPU quota metrics, %v") + errLowUsage = errors.Errorf("The workload in selected time window is too low, with which TiDB is unable to reach a capacity estimation; please select another time window with higher workload, or calibrate resource by hardware instead") + errNoCPUQuotaMetrics = errors.Normalize("There is no CPU quota metrics, %v") ) func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk.Chunk, exec sqlexec.RestrictedSQLExecutor) error { @@ -219,15 +219,15 @@ func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk } rus, err := getRUPerSec(ctx, e.ctx, exec, startTime, endTime) if err != nil { - return errTooFewMetricsPoints.FastGenByArgs(err.Error()) + return err } tikvCPUs, err := getComponentCPUUsagePerSec(ctx, e.ctx, exec, "tikv", startTime, endTime) if err != nil { - return errTooFewMetricsPoints.FastGenByArgs(err.Error()) + return err } tidbCPUs, err := getComponentCPUUsagePerSec(ctx, e.ctx, exec, "tidb", startTime, endTime) if err != nil { - return errTooFewMetricsPoints.FastGenByArgs(err.Error()) + return err } quotas := make([]float64, 0) lowCount := 0 @@ -261,7 +261,7 @@ func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk tikvCPUs.next() } if len(quotas) < 5 { - return errTooFewMetricsPoints.FastGenByArgs("low usage") + return errLowUsage } if float64(len(quotas))/float64(len(quotas)+lowCount) > percentOfPass { sort.Slice(quotas, func(i, j int) bool { @@ -276,7 +276,7 @@ func (e *calibrateResourceExec) dynamicCalibrate(ctx context.Context, req *chunk quota := sum / float64(upperBound-lowerBound) req.AppendUint64(0, uint64(quota)) } else { - return errors.Errorf("The workload in selected time window is too low, with which TiDB is unable to reach a capacity estimation; please select another time window with higher workload, or calibrate resource by hardware instead") + return errLowUsage } return nil } @@ -396,9 +396,6 @@ func getValuesFromMetrics(ctx context.Context, sctx sessionctx.Context, exec sql if err != nil { return nil, errors.Trace(err) } - if len(rows) == 0 { - return nil, errors.Errorf("metrics '%s' is empty", metrics) - } ret := make([]*timePointValue, 0, len(rows)) for _, row := range rows { if tp, err := row.GetTime(0).AdjustedGoTime(sctx.GetSessionVars().Location()); err == nil { diff --git a/executor/calibrate_resource_test.go b/executor/calibrate_resource_test.go index c5f2ed37c3309..21bde73c79029 100644 --- a/executor/calibrate_resource_test.go +++ b/executor/calibrate_resource_test.go @@ -408,7 +408,7 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") + require.ErrorContains(t, err, "The workload in selected time window is too low") ru3 := [][]types.Datum{ types.MakeDatums(datetime("2020-02-12 10:25:00"), 2200.0), @@ -448,7 +448,7 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") + require.ErrorContains(t, err, "The workload in selected time window is too low") // flash back to init data. mockData["resource_manager_resource_unit"] = ru1 @@ -559,14 +559,14 @@ func TestCalibrateResource(t *testing.T) { require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window, low usage") + require.ErrorContains(t, err, "The workload in selected time window is too low") delete(mockData, "process_cpu_usage") rs, err = tk.Exec("CALIBRATE RESOURCE START_TIME '2020-02-12 10:35:00' END_TIME '2020-02-12 10:45:00'") require.NoError(t, err) require.NotNil(t, rs) err = rs.Next(ctx, rs.NewChunk(nil)) - require.ErrorContains(t, err, "There are too few metrics points available in selected time window, query metric error: pd unavailable") + require.ErrorContains(t, err, "query metric error: pd unavailable") } type mockResourceGroupProvider struct {