Skip to content

Commit f6a78fe

Browse files
wddevriesxhebox
wddevries
authored and
xhebox
committed
workloadrepo: Use better error codes for errors in the Workload Repository. (pingcap#58866)
ref pingcap#58247
1 parent ba30630 commit f6a78fe

File tree

9 files changed

+59
-24
lines changed

9 files changed

+59
-24
lines changed

‎pkg/util/workloadrepo/BUILD.bazel

+6
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,28 @@ go_library(
1616
deps = [
1717
"//pkg/domain",
1818
"//pkg/domain/infosync",
19+
"//pkg/errno",
1920
"//pkg/executor",
2021
"//pkg/infoschema",
2122
"//pkg/kv",
2223
"//pkg/meta/model",
2324
"//pkg/owner",
2425
"//pkg/parser/model",
26+
"//pkg/parser/mysql",
2527
"//pkg/sessionctx",
2628
"//pkg/sessionctx/variable",
2729
"//pkg/sessiontxn",
2830
"//pkg/util",
2931
"//pkg/util/chunk",
32+
"//pkg/util/dbterror",
3033
"//pkg/util/logutil",
3134
"//pkg/util/slice",
3235
"//pkg/util/sqlescape",
3336
"//pkg/util/sqlexec",
3437
"//pkg/util/stmtsummary",
3538
"@com_github_ngaut_pools//:pools",
3639
"@com_github_pingcap_errors//:errors",
40+
"@com_github_pingcap_failpoint//:failpoint",
3741
"@io_etcd_go_etcd_client_v3//:client",
3842
"@org_uber_go_zap//:zap",
3943
],
@@ -53,7 +57,9 @@ go_test(
5357
"//pkg/owner",
5458
"//pkg/parser/model",
5559
"//pkg/sessionctx",
60+
"//pkg/sessionctx/variable",
5661
"//pkg/testkit",
62+
"@com_github_pingcap_failpoint//:failpoint",
5763
"@com_github_stretchr_testify//require",
5864
"@io_etcd_go_etcd_client_v3//:client",
5965
"@io_etcd_go_etcd_server_v3//embed",

‎pkg/util/workloadrepo/const.go

+6
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ package workloadrepo
1717
import (
1818
"time"
1919

20+
"github.com/pingcap/tidb/pkg/errno"
2021
"github.com/pingcap/tidb/pkg/parser/model"
22+
"github.com/pingcap/tidb/pkg/parser/mysql"
23+
"github.com/pingcap/tidb/pkg/util/dbterror"
2124
)
2225

2326
const (
@@ -43,4 +46,7 @@ const (
4346
var (
4447
workloadSchemaCIStr = model.NewCIStr(WorkloadSchema)
4548
zeroTime = time.Time{}
49+
50+
errWrongValueForVar = dbterror.ClassUtil.NewStd(errno.ErrWrongValueForVar)
51+
errUnsupportedEtcdRequired = dbterror.ClassUtil.NewStdErr(errno.ErrNotSupportedYet, mysql.Message("etcd client required for workload repository", nil))
4652
)

‎pkg/util/workloadrepo/housekeeper.go

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func createPartition(ctx context.Context, is infoschema.InfoSchema, tbl *reposit
5151
sqlescape.MustFormatSQL(sb, "ALTER TABLE %n.%n ADD PARTITION (", WorkloadSchema, tbl.destTable)
5252
skip, err := generatePartitionRanges(sb, tbInfo, now)
5353
if err != nil {
54+
logutil.BgLogger().Info("workload repository cannot generate partition definitions", zap.String("table", tbl.destTable), zap.NamedError("err", err))
5455
return err
5556
}
5657
if !skip {

‎pkg/util/workloadrepo/sampling.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ package workloadrepo
1616

1717
import (
1818
"context"
19+
"errors"
1920
"strconv"
2021
"time"
2122

23+
"github.com/pingcap/failpoint"
2224
"github.com/pingcap/tidb/pkg/sessionctx"
2325
"github.com/pingcap/tidb/pkg/util"
2426
"github.com/pingcap/tidb/pkg/util/logutil"
@@ -80,8 +82,13 @@ func (w *worker) resetSamplingInterval(newRate int32) {
8082

8183
func (w *worker) changeSamplingInterval(_ context.Context, d string) error {
8284
n, err := strconv.Atoi(d)
85+
86+
failpoint.Inject("FastRunawayGC", func() {
87+
err = errors.New("fake error")
88+
})
89+
8390
if err != nil {
84-
return err
91+
return errWrongValueForVar.GenWithStackByArgs(repositorySamplingInterval, d)
8592
}
8693

8794
w.Lock()

‎pkg/util/workloadrepo/snapshot.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"time"
2323

2424
"github.com/pingcap/errors"
25+
"github.com/pingcap/failpoint"
2526
"github.com/pingcap/tidb/pkg/sessionctx"
2627
"github.com/pingcap/tidb/pkg/util"
2728
"github.com/pingcap/tidb/pkg/util/logutil"
@@ -278,8 +279,13 @@ func (w *worker) resetSnapshotInterval(newRate int32) {
278279

279280
func (w *worker) changeSnapshotInterval(_ context.Context, d string) error {
280281
n, err := strconv.Atoi(d)
282+
283+
failpoint.Inject("FastRunawayGC", func() {
284+
err = errors.New("fake error")
285+
})
286+
281287
if err != nil {
282-
return err
288+
return errWrongValueForVar.GenWithStackByArgs(repositorySnapshotInterval, d)
283289
}
284290

285291
w.Lock()

‎pkg/util/workloadrepo/table.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,16 @@ func (w *worker) createAllTables(ctx context.Context, now time.Time) error {
128128
if tbl.tableType == metadataTable {
129129
sb := &strings.Builder{}
130130
fmt.Fprint(sb, createStmt)
131-
generatePartitionDef(sb, "BEGIN_TIME", now)
131+
if err := generatePartitionDef(sb, "BEGIN_TIME", now); err != nil {
132+
return err
133+
}
132134
createStmt = sb.String()
133135
} else {
134136
sb := &strings.Builder{}
135137
fmt.Fprint(sb, createStmt)
136-
generatePartitionDef(sb, "TS", now)
138+
if err := generatePartitionDef(sb, "TS", now); err != nil {
139+
return err
140+
}
137141
createStmt = sb.String()
138142
}
139143

‎pkg/util/workloadrepo/utils.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package workloadrepo
1616

1717
import (
1818
"context"
19-
"errors"
2019
"fmt"
2120
"strconv"
2221
"strings"
@@ -25,11 +24,19 @@ import (
2524
"github.com/pingcap/tidb/pkg/meta/model"
2625
)
2726

28-
func generatePartitionDef(sb *strings.Builder, col string, now time.Time) {
27+
func generatePartitionDef(sb *strings.Builder, col string, now time.Time) error {
2928
fmt.Fprintf(sb, " PARTITION BY RANGE( TO_DAYS(%s) ) (", col)
3029
// tbInfo is nil, retval must be false
31-
_, _ = generatePartitionRanges(sb, nil, now)
30+
allExisted, err := generatePartitionRanges(sb, nil, now)
31+
if err != nil {
32+
return err
33+
}
34+
if allExisted {
35+
return fmt.Errorf("could not generate partition ranges")
36+
}
37+
3238
fmt.Fprintf(sb, ")")
39+
return nil
3340
}
3441

3542
func generatePartitionName(t time.Time) string {
@@ -80,7 +87,7 @@ func generatePartitionRanges(sb *strings.Builder, tbInfo *model.TableInfo, now t
8087
func (w *worker) setRetentionDays(_ context.Context, d string) error {
8188
n, err := strconv.Atoi(d)
8289
if err != nil {
83-
return err
90+
return errWrongValueForVar.GenWithStackByArgs(repositoryRetentionDays, d)
8491
}
8592
w.Lock()
8693
defer w.Unlock()
@@ -92,7 +99,7 @@ func validateDest(orig string) (string, error) {
9299
// validate S3 URL, etc...
93100
orig = strings.ToLower(orig)
94101
if orig != "" && orig != "table" {
95-
return "", errors.New("invalid repository destination")
102+
return "", errWrongValueForVar.GenWithStack("Variable '%-.64s' can't be set to the value of '%-.200s': valid values are '' and 'table'", repositoryDest, orig)
96103
}
97104
return orig, nil
98105
}

‎pkg/util/workloadrepo/worker.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ func (w *worker) start() error {
355355
}
356356

357357
if w.etcdClient == nil {
358-
return errors.New("etcd client required for workload repository")
358+
return errUnsupportedEtcdRequired.GenWithStackByArgs()
359359
}
360360

361361
_ = stmtsummary.StmtSummaryByDigestMap.SetHistoryEnabled(false)

‎pkg/util/workloadrepo/worker_test.go

+12-14
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ import (
2323
"testing"
2424
"time"
2525

26+
"github.com/pingcap/failpoint"
2627
"github.com/pingcap/tidb/pkg/domain"
2728
"github.com/pingcap/tidb/pkg/infoschema"
2829
"github.com/pingcap/tidb/pkg/kv"
2930
"github.com/pingcap/tidb/pkg/owner"
3031
"github.com/pingcap/tidb/pkg/parser/model"
3132
"github.com/pingcap/tidb/pkg/sessionctx"
33+
"github.com/pingcap/tidb/pkg/sessionctx/variable"
3234
"github.com/pingcap/tidb/pkg/testkit"
3335
"github.com/stretchr/testify/require"
3436
clientv3 "go.etcd.io/etcd/client/v3"
@@ -504,18 +506,16 @@ func TestSettingSQLVariables(t *testing.T) {
504506
eventuallyWithLock(t, wrk, func() bool { return int32(7200) == wrk.snapshotInterval })
505507
eventuallyWithLock(t, wrk, func() bool { return int32(365) == wrk.retentionDays })
506508

507-
// Test invalid value for sampling interval
508-
err := tk.ExecToErr("set @@global." + repositorySamplingInterval + " = 'invalid'")
509-
require.Error(t, err)
510-
require.Contains(t, err.Error(), "Incorrect argument type")
509+
// Test invalid values for intervals
510+
tk.MustGetDBError("set @@global."+repositorySamplingInterval+" = 'invalid'", variable.ErrWrongTypeForVar)
511+
tk.MustGetDBError("set @@global."+repositorySnapshotInterval+" = 'invalid'", variable.ErrWrongTypeForVar)
512+
tk.MustGetDBError("set @@global."+repositoryRetentionDays+" = 'invalid'", variable.ErrWrongTypeForVar)
511513

512-
err = tk.ExecToErr("set @@global." + repositorySnapshotInterval + " = 'invalid'")
513-
require.Error(t, err)
514-
require.Contains(t, err.Error(), "Incorrect argument type")
515-
516-
err = tk.ExecToErr("set @@global." + repositoryRetentionDays + " = 'invalid'")
517-
require.Error(t, err)
518-
require.Contains(t, err.Error(), "Incorrect argument type")
514+
// Test that if the strconv.Atoi call fails that the error is correctly handled.
515+
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/util/workloadrepo/FastRunawayGC", `return(true)`))
516+
tk.MustGetDBError("set @@global."+repositorySamplingInterval+" = 10", errWrongValueForVar)
517+
tk.MustGetDBError("set @@global."+repositorySnapshotInterval+" = 901", errWrongValueForVar)
518+
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/util/workloadrepo/FastRunawayGC"))
519519

520520
trueWithLock(t, wrk, func() bool { return int32(600) == wrk.samplingInterval })
521521
trueWithLock(t, wrk, func() bool { return int32(7200) == wrk.snapshotInterval })
@@ -530,9 +530,7 @@ func TestSettingSQLVariables(t *testing.T) {
530530
eventuallyWithLock(t, wrk, func() bool { return !wrk.enabled })
531531

532532
// Test invalid value for repository destination
533-
err = tk.ExecToErr("set @@global." + repositoryDest + " = 'invalid'")
534-
require.Error(t, err)
535-
require.Contains(t, err.Error(), "invalid repository destination")
533+
tk.MustGetDBError("set @@global."+repositoryDest+" = 'invalid'", errWrongValueForVar)
536534
}
537535

538536
func getTable(t *testing.T, tableName string, wrk *worker) *repositoryTable {

0 commit comments

Comments
 (0)