From 07c7f43d19dbb63665829338781f058344e5a4ff Mon Sep 17 00:00:00 2001 From: JmPotato Date: Wed, 2 Sep 2020 11:58:05 +0800 Subject: [PATCH 1/4] Optimize the err defines for tso Signed-off-by: JmPotato --- pkg/errs/errno.go | 3 +++ server/tso/global_allocator.go | 9 ++++----- server/tso/tso.go | 13 ++++++------- tests/server/tso/tso_test.go | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go index 08a7263faf6..4bb46a9577a 100644 --- a/pkg/errs/errno.go +++ b/pkg/errs/errno.go @@ -18,6 +18,9 @@ import "github.com/pingcap/errors" // The internal error which is generated in PD project. // tso errors var ( + ErrSaveTimestamp = errors.Normalize("save timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrSaveTimestamp")) + ErrResetUserTimestamp = errors.Normalize("reset user timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrResetUserTimestamp")) + ErrGenerateTimestamp = errors.Normalize("generate timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrGenerateTimestamp")) ErrInvalidTimestamp = errors.Normalize("invalid timestamp", errors.RFCCodeText("PD:tso:ErrInvalidTimestamp")) ErrLogicOverflow = errors.Normalize("logic part overflow", errors.RFCCodeText("PD:tso:ErrLogicOverflow")) ErrIncorrectSystemTime = errors.Normalize("incorrect system time", errors.RFCCodeText("PD:tso:ErrIncorrectSystemTime")) diff --git a/server/tso/global_allocator.go b/server/tso/global_allocator.go index 98da0a70d6d..474b81d3093 100644 --- a/server/tso/global_allocator.go +++ b/server/tso/global_allocator.go @@ -17,7 +17,6 @@ import ( "sync/atomic" "time" - "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/pdpb" "github.com/pingcap/log" @@ -87,7 +86,7 @@ func (gta *GlobalTSOAllocator) GenerateTSO(count uint32) (pdpb.Timestamp, error) var resp pdpb.Timestamp if count == 0 { - return resp, errors.New("tso count should be positive") + return resp, errs.ErrGenerateTimestamp.FastGenByArgs("tso count should be positive") } maxRetryCount := 10 @@ -105,7 +104,7 @@ func (gta *GlobalTSOAllocator) GenerateTSO(count uint32) (pdpb.Timestamp, error) continue } log.Error("invalid timestamp", zap.Any("timestamp", current), errs.ZapError(errs.ErrInvalidTimestamp)) - return pdpb.Timestamp{}, errors.New("can not get timestamp, may be not leader") + return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("may be not leader") } resp.Physical = current.physical.UnixNano() / int64(time.Millisecond) @@ -120,11 +119,11 @@ func (gta *GlobalTSOAllocator) GenerateTSO(count uint32) (pdpb.Timestamp, error) } // In case lease expired after the first check. if !gta.leadership.Check() { - return pdpb.Timestamp{}, errors.New("alloc timestamp failed, lease expired") + return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("may be not leader") } return resp, nil } - return resp, errors.New("can not get timestamp") + return resp, errs.ErrGenerateTimestamp.FastGenByArgs("maximum number of retries exceeded") } // Reset is used to reset the TSO allocator. diff --git a/server/tso/tso.go b/server/tso/tso.go index d7979ca708b..911ce1dafcc 100644 --- a/server/tso/tso.go +++ b/server/tso/tso.go @@ -19,7 +19,6 @@ import ( "time" "unsafe" - "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/log" "github.com/tikv/pd/pkg/errs" @@ -67,7 +66,7 @@ func (t *timestampOracle) getTimestampPath() string { func (t *timestampOracle) loadTimestamp() (time.Time, error) { data, err := etcdutil.GetValue(t.client, t.getTimestampPath()) if err != nil { - return typeutil.ZeroTime, err + return typeutil.ZeroTime, errs.ErrEtcdKVGet.Wrap(err).FastGenWithCause() } if len(data) == 0 { return typeutil.ZeroTime, nil @@ -84,10 +83,10 @@ func (t *timestampOracle) saveTimestamp(leadership *election.Leadership, ts time Then(clientv3.OpPut(key, string(data))). Commit() if err != nil { - return errors.WithStack(err) + return errs.ErrEtcdTxn.Wrap(err).GenWithStackByCause() } if !resp.Succeeded { - return errors.New("save timestamp failed, maybe we lost leader") + return errs.ErrSaveTimestamp.FastGenByArgs("maybe we lost leader") } t.lastSavedTime.Store(ts) @@ -141,7 +140,7 @@ func (t *timestampOracle) SyncTimestamp(leadership *election.Leadership) error { func (t *timestampOracle) ResetUserTimestamp(leadership *election.Leadership, tso uint64) error { if !leadership.Check() { tsoCounter.WithLabelValues("err_lease_reset_ts").Inc() - return errors.New("Setup timestamp failed, lease expired") + return errs.ErrResetUserTimestamp.FastGenByArgs("lease expired") } physical, _ := tsoutil.ParseTS(tso) next := physical.Add(time.Millisecond) @@ -150,12 +149,12 @@ func (t *timestampOracle) ResetUserTimestamp(leadership *election.Leadership, ts // do not update if typeutil.SubTimeByWallClock(next, prev.physical) <= 3*updateTimestampGuard { tsoCounter.WithLabelValues("err_reset_small_ts").Inc() - return errors.New("the specified ts too small than now") + return errs.ErrResetUserTimestamp.FastGenByArgs("the specified ts too small than now") } if typeutil.SubTimeByWallClock(next, prev.physical) >= t.maxResetTSGap() { tsoCounter.WithLabelValues("err_reset_large_ts").Inc() - return errors.New("the specified ts too large than now") + return errs.ErrResetUserTimestamp.FastGenByArgs("the specified ts too large than now") } save := next.Add(t.saveInterval) diff --git a/tests/server/tso/tso_test.go b/tests/server/tso/tso_test.go index b594901c28d..11fa407918c 100644 --- a/tests/server/tso/tso_test.go +++ b/tests/server/tso/tso_test.go @@ -395,6 +395,6 @@ func (s *testFollowerTsoSuite) TestRequest(c *C) { c.Assert(err, IsNil) _, err = tsoClient.Recv() c.Assert(err, NotNil) - c.Assert(strings.Contains(err.Error(), "can not get timestamp"), IsTrue) + c.Assert(strings.Contains(err.Error(), "generate timestamp failed"), IsTrue) failpoint.Disable("github.com/tikv/pd/server/tso/skipRetryGetTS") } From 1e9c7515be735a144574e8df4df076bbcc33665f Mon Sep 17 00:00:00 2001 From: JmPotato Date: Wed, 2 Sep 2020 12:44:11 +0800 Subject: [PATCH 2/4] Address the comment Co-authored-by: Howard Lau Signed-off-by: JmPotato --- server/tso/tso.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tso/tso.go b/server/tso/tso.go index 911ce1dafcc..b8fd9759bfd 100644 --- a/server/tso/tso.go +++ b/server/tso/tso.go @@ -66,7 +66,7 @@ func (t *timestampOracle) getTimestampPath() string { func (t *timestampOracle) loadTimestamp() (time.Time, error) { data, err := etcdutil.GetValue(t.client, t.getTimestampPath()) if err != nil { - return typeutil.ZeroTime, errs.ErrEtcdKVGet.Wrap(err).FastGenWithCause() + return typeutil.ZeroTime, err } if len(data) == 0 { return typeutil.ZeroTime, nil From 8f56ab2a865051652758dc2fb2725f8e7369845d Mon Sep 17 00:00:00 2001 From: JmPotato Date: Wed, 2 Sep 2020 13:13:41 +0800 Subject: [PATCH 3/4] Update etcd error Signed-off-by: JmPotato --- pkg/errs/errno.go | 1 - server/tso/tso.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go index 4bb46a9577a..52934e15973 100644 --- a/pkg/errs/errno.go +++ b/pkg/errs/errno.go @@ -18,7 +18,6 @@ import "github.com/pingcap/errors" // The internal error which is generated in PD project. // tso errors var ( - ErrSaveTimestamp = errors.Normalize("save timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrSaveTimestamp")) ErrResetUserTimestamp = errors.Normalize("reset user timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrResetUserTimestamp")) ErrGenerateTimestamp = errors.Normalize("generate timestamp failed, %s", errors.RFCCodeText("PD:tso:ErrGenerateTimestamp")) ErrInvalidTimestamp = errors.Normalize("invalid timestamp", errors.RFCCodeText("PD:tso:ErrInvalidTimestamp")) diff --git a/server/tso/tso.go b/server/tso/tso.go index b8fd9759bfd..72c4433eb57 100644 --- a/server/tso/tso.go +++ b/server/tso/tso.go @@ -83,10 +83,10 @@ func (t *timestampOracle) saveTimestamp(leadership *election.Leadership, ts time Then(clientv3.OpPut(key, string(data))). Commit() if err != nil { - return errs.ErrEtcdTxn.Wrap(err).GenWithStackByCause() + return errs.ErrEtcdKVPut.Wrap(err).GenWithStackByCause() } if !resp.Succeeded { - return errs.ErrSaveTimestamp.FastGenByArgs("maybe we lost leader") + return errs.ErrEtcdTxn.FastGenByArgs() } t.lastSavedTime.Store(ts) From d3c81f26297c81cae5a4f11245c57d0a7c90e939 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Wed, 2 Sep 2020 13:29:44 +0800 Subject: [PATCH 4/4] Distinguish the cause of the error Signed-off-by: JmPotato --- server/tso/global_allocator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/tso/global_allocator.go b/server/tso/global_allocator.go index 474b81d3093..8674d6cc735 100644 --- a/server/tso/global_allocator.go +++ b/server/tso/global_allocator.go @@ -104,7 +104,7 @@ func (gta *GlobalTSOAllocator) GenerateTSO(count uint32) (pdpb.Timestamp, error) continue } log.Error("invalid timestamp", zap.Any("timestamp", current), errs.ZapError(errs.ErrInvalidTimestamp)) - return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("may be not leader") + return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("timestamp in memory isn't initialized") } resp.Physical = current.physical.UnixNano() / int64(time.Millisecond) @@ -119,7 +119,7 @@ func (gta *GlobalTSOAllocator) GenerateTSO(count uint32) (pdpb.Timestamp, error) } // In case lease expired after the first check. if !gta.leadership.Check() { - return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("may be not leader") + return pdpb.Timestamp{}, errs.ErrGenerateTimestamp.FastGenByArgs("not the pd leader") } return resp, nil }