diff --git a/executor/admin_test.go b/executor/admin_test.go index 4df2fec231878..195eb0697feb0 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -551,7 +551,7 @@ func (s *testSuite2) TestAdminCheckWithSnapshot(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/executor_test.go b/executor/executor_test.go index db5b36f04f6c2..8e5610df50c57 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2021,7 +2021,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 84e329792b1ed..9b67a749b735a 100644 --- a/executor/set.go +++ b/executor/set.go @@ -17,7 +17,6 @@ import ( "context" "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/sqlexec" log "github.com/sirupsen/logrus" @@ -194,8 +194,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 bf096f3ca73cc..4561b586f176f 100644 --- a/store/tikv/gcworker/gc_worker.go +++ b/store/tikv/gcworker/gc_worker.go @@ -95,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 @@ -932,7 +931,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) } @@ -944,7 +943,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) } @@ -1078,7 +1077,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 { log.Errorf("deleteRanges is called") return w.worker.deleteRanges(ctx, safePoint) diff --git a/util/misc.go b/util/misc.go index 3060f610c687d..496ef43ca036f 100644 --- a/util/misc.go +++ b/util/misc.go @@ -15,6 +15,7 @@ package util import ( "runtime" + "strings" "time" "github.com/pingcap/errors" @@ -26,6 +27,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. @@ -72,3 +75,23 @@ func WithRecovery(exec func(), recoverFn func(r interface{})) { }() exec() } + +// 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) + } +}