Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gc_worker: Remove timezone name from the times that are saved in mysql.tidb #8745

Merged
merged 10 commits into from
Dec 27, 2018
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 @@ -1998,7 +1998,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"
MyonKeminta marked this conversation as resolved.
Show resolved Hide resolved
)

// 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], " ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it out of range?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it should returns at least one parts. You split "" by " " then you should got [""]. You can also see that the test has passed.

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