diff --git a/executor/executor_test.go b/executor/executor_test.go index fbaf69e0bec6f..35e3c41f8181a 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2027,7 +2027,7 @@ func (s *testSuite) TestHistoryRead(c *C) { // For mocktikv, safe point is not initialized, we manually insert it for snapshot to use. safePointName := "tikv_gc_safe_point" - safePointValue := "20060102-15:04:05 -0700 MST" + safePointValue := "20060102-15:04:05 -0700" safePointComment := "All versions after safe point can be accessed. (DO NOT EDIT)" updateSafePoint := fmt.Sprintf(`INSERT INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') ON DUPLICATE KEY diff --git a/executor/set.go b/executor/set.go index 9564e96bf6e5f..e4372cc2dd2d1 100644 --- a/executor/set.go +++ b/executor/set.go @@ -16,7 +16,6 @@ package executor import ( "fmt" "strings" - "time" "github.com/pingcap/errors" "github.com/pingcap/parser/ast" @@ -28,6 +27,7 @@ import ( "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/sqlexec" @@ -206,8 +206,7 @@ func validateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { return errors.New("can not get 'tikv_gc_safe_point'") } safePointString := rows[0].GetString(0) - const gcTimeFormat = "20060102-15:04:05 -0700 MST" - safePointTime, err := time.Parse(gcTimeFormat, safePointString) + safePointTime, err := util.CompatibleParseGCTime(safePointString) if err != nil { return errors.Trace(err) } diff --git a/store/tikv/gcworker/gc_worker.go b/store/tikv/gcworker/gc_worker.go index 4cc7b58511818..4381445b53dc3 100644 --- a/store/tikv/gcworker/gc_worker.go +++ b/store/tikv/gcworker/gc_worker.go @@ -37,6 +37,7 @@ import ( "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/store/tikv/oracle" "github.com/pingcap/tidb/store/tikv/tikvrpc" + tidbutil "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/logutil" "go.uber.org/zap" "golang.org/x/net/context" @@ -94,7 +95,6 @@ func (w *GCWorker) Close() { } const ( - gcTimeFormat = "20060102-15:04:05 -0700 MST" gcWorkerTickInterval = time.Minute gcJobLogTickInterval = time.Minute * 10 gcWorkerLease = time.Minute * 2 @@ -942,7 +942,7 @@ func (w *GCWorker) saveSafePoint(kv tikv.SafePointKV, key string, t uint64) erro } func (w *GCWorker) saveTime(key string, t time.Time) error { - err := w.saveValueToSysTable(key, t.Format(gcTimeFormat)) + err := w.saveValueToSysTable(key, t.Format(tidbutil.GCTimeFormat)) return errors.Trace(err) } @@ -954,7 +954,7 @@ func (w *GCWorker) loadTime(key string) (*time.Time, error) { if str == "" { return nil, nil } - t, err := time.Parse(gcTimeFormat, str) + t, err := tidbutil.CompatibleParseGCTime(str) if err != nil { return nil, errors.Trace(err) } @@ -1094,7 +1094,7 @@ func NewMockGCWorker(store tikv.Storage) (*MockGCWorker, error) { return &MockGCWorker{worker: worker}, nil } -// DeleteRanges call deleteRanges internally, just for test. +// DeleteRanges calls deleteRanges internally, just for test. func (w *MockGCWorker) DeleteRanges(ctx context.Context, safePoint uint64) error { logutil.Logger(ctx).Error("deleteRanges is called") return w.worker.deleteRanges(ctx, safePoint) diff --git a/util/misc.go b/util/misc.go index e48f8e9632b55..da3ed8f2618b0 100644 --- a/util/misc.go +++ b/util/misc.go @@ -15,6 +15,7 @@ package util import ( "runtime" + "strings" "time" "github.com/pingcap/errors" @@ -30,6 +31,8 @@ const ( DefaultMaxRetries = 30 // RetryInterval indicates retry interval. RetryInterval uint64 = 500 + // GCTimeFormat is the format that gc_worker used to store times. + GCTimeFormat = "20060102-15:04:05 -0700" ) // RunWithRetry will run the f with backoff and retry. @@ -100,3 +103,23 @@ func SyntaxError(err error) error { return parser.ErrParse.GenWithStackByArgs(syntaxErrorPrefix, err.Error()) } + +// CompatibleParseGCTime parses a string with `GCTimeFormat` and returns a time.Time. If `value` can't be parsed as that +// format, truncate to last space and try again. This function is only useful when loading times that saved by +// gc_worker. We have changed the format that gc_worker saves time (removed the last field), but when loading times it +// should be compatible with the old format. +func CompatibleParseGCTime(value string) (time.Time, error) { + t, err := time.Parse(GCTimeFormat, value) + + if err != nil { + // Remove the last field that separated by space + parts := strings.Split(value, " ") + prefix := strings.Join(parts[:len(parts)-1], " ") + t, err = time.Parse(GCTimeFormat, prefix) + } + + if err != nil { + err = errors.Errorf("string \"%v\" doesn't has a prefix that matches format \"%v\"", value, GCTimeFormat) + } + return t, err +} diff --git a/util/misc_test.go b/util/misc_test.go index 5060d25278ada..1ae47e6e2f6a7 100644 --- a/util/misc_test.go +++ b/util/misc_test.go @@ -14,6 +14,8 @@ package util import ( + "time" + . "github.com/pingcap/check" "github.com/pingcap/errors" "github.com/pingcap/tidb/util/testleak" @@ -30,7 +32,7 @@ func (s *testMiscSuite) SetUpSuite(c *C) { func (s *testMiscSuite) TearDownSuite(c *C) { } -func (s testMiscSuite) TestRunWithRetry(c *C) { +func (s *testMiscSuite) TestRunWithRetry(c *C) { defer testleak.AfterTest(c)() // Run succ. cnt := 0 @@ -68,3 +70,46 @@ func (s testMiscSuite) TestRunWithRetry(c *C) { c.Assert(err, NotNil) c.Assert(cnt, Equals, 1) } + +func (s *testMiscSuite) TestCompatibleParseGCTime(c *C) { + values := []string{ + "20181218-19:53:37 +0800 CST", + "20181218-19:53:37 +0800 MST", + "20181218-19:53:37 +0800 FOO", + "20181218-19:53:37 +0800 +08", + "20181218-19:53:37 +0800", + "20181218-19:53:37 +0800 ", + "20181218-11:53:37 +0000", + } + + invalidValues := []string{ + "", + " ", + "foo", + "20181218-11:53:37", + "20181218-19:53:37 +0800CST", + "20181218-19:53:37 +0800 FOO BAR", + "20181218-19:53:37 +0800FOOOOOOO BAR", + "20181218-19:53:37 ", + } + + expectedTime := time.Date(2018, 12, 18, 11, 53, 37, 0, time.UTC) + expectedTimeFormatted := "20181218-19:53:37 +0800" + + beijing, err := time.LoadLocation("Asia/Shanghai") + c.Assert(err, IsNil) + + for _, value := range values { + t, err := CompatibleParseGCTime(value) + c.Assert(err, IsNil) + c.Assert(t.Equal(expectedTime), Equals, true) + + formatted := t.In(beijing).Format(GCTimeFormat) + c.Assert(formatted, Equals, expectedTimeFormatted) + } + + for _, value := range invalidValues { + _, err := CompatibleParseGCTime(value) + c.Assert(err, NotNil) + } +}