Skip to content

Commit

Permalink
gc_worker: Remove timezone name from the times that are saved in mysq…
Browse files Browse the repository at this point in the history
…l.tidb (pingcap#8745)

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
  • Loading branch information
MyonKeminta authored and yu34po committed Jan 2, 2019
1 parent 61be66b commit 42db62e
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 10 deletions.
2 changes: 1 addition & 1 deletion executor/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions executor/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"context"
"fmt"
"strings"
"time"

"github.com/pingcap/errors"
"github.com/pingcap/parser/ast"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 3 additions & 4 deletions store/tikv/gcworker/gc_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions util/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package util

import (
"runtime"
"strings"
"time"

"github.com/pingcap/errors"
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
47 changes: 46 additions & 1 deletion util/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package util

import (
"time"

. "github.com/pingcap/check"
"github.com/pingcap/errors"
"github.com/pingcap/tidb/util/testleak"
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}

0 comments on commit 42db62e

Please sign in to comment.