gc_worker: remove timezone name from the times that are saved in mysq…

…l.tidb (#8745) (#10637)

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

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)
 	}

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)

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
+}

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)
+	}
+}