From f51f9134558e793e7ea08fc32cef672c7afa37b7 Mon Sep 17 00:00:00 2001 From: Yongbo Jiang Date: Wed, 13 Dec 2023 19:27:19 +0800 Subject: [PATCH] errs: remove redundant `FastGenWithCause` in `ZapError` (#7497) close tikv/pd#7496 Signed-off-by: Cabinfever_B Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> --- client/errs/errno.go | 2 +- client/errs/errs.go | 2 +- client/tso_dispatcher.go | 6 ++-- errors.toml | 2 +- pkg/autoscaling/prometheus.go | 2 +- pkg/errs/errno.go | 2 +- pkg/errs/errs.go | 2 +- pkg/errs/errs_test.go | 35 +++++++++++++++------- pkg/schedule/hbstream/heartbeat_streams.go | 2 +- pkg/schedule/plugin_interface.go | 6 ++-- pkg/schedule/schedulers/evict_leader.go | 2 +- pkg/schedule/schedulers/grant_leader.go | 2 +- pkg/schedule/schedulers/init.go | 10 +++---- pkg/schedule/schedulers/scheduler.go | 4 +-- pkg/schedule/schedulers/utils.go | 4 +-- pkg/tso/keyspace_group_manager.go | 2 +- pkg/utils/logutil/log.go | 2 +- pkg/utils/tempurl/check_env_linux.go | 2 +- server/handler.go | 2 +- 19 files changed, 53 insertions(+), 38 deletions(-) diff --git a/client/errs/errno.go b/client/errs/errno.go index 646af81929d..0f93ebf1472 100644 --- a/client/errs/errno.go +++ b/client/errs/errno.go @@ -54,7 +54,7 @@ var ( ErrClientGetMultiResponse = errors.Normalize("get invalid value response %v, must only one", errors.RFCCodeText("PD:client:ErrClientGetMultiResponse")) ErrClientGetServingEndpoint = errors.Normalize("get serving endpoint failed", errors.RFCCodeText("PD:client:ErrClientGetServingEndpoint")) ErrClientFindGroupByKeyspaceID = errors.Normalize("can't find keyspace group by keyspace id", errors.RFCCodeText("PD:client:ErrClientFindGroupByKeyspaceID")) - ErrClientWatchGCSafePointV2Stream = errors.Normalize("watch gc safe point v2 stream failed, %s", errors.RFCCodeText("PD:client:ErrClientWatchGCSafePointV2Stream")) + ErrClientWatchGCSafePointV2Stream = errors.Normalize("watch gc safe point v2 stream failed", errors.RFCCodeText("PD:client:ErrClientWatchGCSafePointV2Stream")) ) // grpcutil errors diff --git a/client/errs/errs.go b/client/errs/errs.go index e715056b055..47f7c29a467 100644 --- a/client/errs/errs.go +++ b/client/errs/errs.go @@ -27,7 +27,7 @@ func ZapError(err error, causeError ...error) zap.Field { } if e, ok := err.(*errors.Error); ok { if len(causeError) >= 1 { - err = e.Wrap(causeError[0]).FastGenWithCause() + err = e.Wrap(causeError[0]) } else { err = e.FastGenByArgs() } diff --git a/client/tso_dispatcher.go b/client/tso_dispatcher.go index 0de4dc3a49e..6b2c33ca58d 100644 --- a/client/tso_dispatcher.go +++ b/client/tso_dispatcher.go @@ -412,7 +412,7 @@ tsoBatchLoop: } else { log.Error("[tso] fetch pending tso requests error", zap.String("dc-location", dc), - errs.ZapError(errs.ErrClientGetTSO.FastGenByArgs("when fetch pending tso requests"), err)) + errs.ZapError(errs.ErrClientGetTSO, err)) } return } @@ -495,10 +495,10 @@ tsoBatchLoop: default: } c.svcDiscovery.ScheduleCheckMemberChanged() - log.Error("[tso] getTS error", + log.Error("[tso] getTS error after processing requests", zap.String("dc-location", dc), zap.String("stream-addr", streamAddr), - errs.ZapError(errs.ErrClientGetTSO.FastGenByArgs("after processing requests"), err)) + errs.ZapError(errs.ErrClientGetTSO, err)) // Set `stream` to nil and remove this stream from the `connectionCtxs` due to error. connectionCtxs.Delete(streamAddr) cancel() diff --git a/errors.toml b/errors.toml index a318fc32492..69aa29d2dda 100644 --- a/errors.toml +++ b/errors.toml @@ -83,7 +83,7 @@ get min TSO failed, %v ["PD:client:ErrClientGetTSO"] error = ''' -get TSO failed, %v +get TSO failed ''' ["PD:client:ErrClientGetTSOTimeout"] diff --git a/pkg/autoscaling/prometheus.go b/pkg/autoscaling/prometheus.go index 43ba768a585..91b813b6ef2 100644 --- a/pkg/autoscaling/prometheus.go +++ b/pkg/autoscaling/prometheus.go @@ -94,7 +94,7 @@ func (prom *PrometheusQuerier) queryMetricsFromPrometheus(query string, timestam resp, warnings, err := prom.api.Query(ctx, query, timestamp) if err != nil { - return nil, errs.ErrPrometheusQuery.Wrap(err).FastGenWithCause() + return nil, errs.ErrPrometheusQuery.Wrap(err) } if len(warnings) > 0 { diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go index a4320238374..03fa0f61158 100644 --- a/pkg/errs/errno.go +++ b/pkg/errs/errno.go @@ -86,7 +86,7 @@ var ( var ( ErrClientCreateTSOStream = errors.Normalize("create TSO stream failed, %s", errors.RFCCodeText("PD:client:ErrClientCreateTSOStream")) ErrClientGetTSOTimeout = errors.Normalize("get TSO timeout", errors.RFCCodeText("PD:client:ErrClientGetTSOTimeout")) - ErrClientGetTSO = errors.Normalize("get TSO failed, %v", errors.RFCCodeText("PD:client:ErrClientGetTSO")) + ErrClientGetTSO = errors.Normalize("get TSO failed", errors.RFCCodeText("PD:client:ErrClientGetTSO")) ErrClientGetLeader = errors.Normalize("get leader failed, %v", errors.RFCCodeText("PD:client:ErrClientGetLeader")) ErrClientGetMember = errors.Normalize("get member failed", errors.RFCCodeText("PD:client:ErrClientGetMember")) ErrClientGetMinTSO = errors.Normalize("get min TSO failed, %v", errors.RFCCodeText("PD:client:ErrClientGetMinTSO")) diff --git a/pkg/errs/errs.go b/pkg/errs/errs.go index acc42637733..5746b282f10 100644 --- a/pkg/errs/errs.go +++ b/pkg/errs/errs.go @@ -27,7 +27,7 @@ func ZapError(err error, causeError ...error) zap.Field { } if e, ok := err.(*errors.Error); ok { if len(causeError) >= 1 { - err = e.Wrap(causeError[0]).FastGenWithCause() + err = e.Wrap(causeError[0]) } else { err = e.FastGenByArgs() } diff --git a/pkg/errs/errs_test.go b/pkg/errs/errs_test.go index c242dd994f5..d76c02dc110 100644 --- a/pkg/errs/errs_test.go +++ b/pkg/errs/errs_test.go @@ -81,9 +81,19 @@ func TestError(t *testing.T) { log.Error("test", zap.Error(ErrEtcdLeaderNotFound.FastGenByArgs())) re.Contains(lg.Message(), rfc) err := errors.New("test error") - log.Error("test", ZapError(ErrEtcdLeaderNotFound, err)) - rfc = `[error="[PD:member:ErrEtcdLeaderNotFound]test error` - re.Contains(lg.Message(), rfc) + // use Info() because of no stack for comparing. + log.Info("test", ZapError(ErrEtcdLeaderNotFound, err)) + rfc = `[error="[PD:member:ErrEtcdLeaderNotFound]etcd leader not found: test error` + m1 := lg.Message() + re.Contains(m1, rfc) + log.Info("test", zap.Error(ErrEtcdLeaderNotFound.Wrap(err))) + m2 := lg.Message() + idx1 := strings.Index(m1, "[error") + idx2 := strings.Index(m2, "[error") + re.Equal(m1[idx1:], m2[idx2:]) + log.Info("test", zap.Error(ErrEtcdLeaderNotFound.Wrap(err).FastGenWithCause())) + m3 := lg.Message() + re.NotContains(m3, rfc) } func TestErrorEqual(t *testing.T) { @@ -94,24 +104,24 @@ func TestErrorEqual(t *testing.T) { re.True(errors.ErrorEqual(err1, err2)) err := errors.New("test") - err1 = ErrSchedulerNotFound.Wrap(err).FastGenWithCause() - err2 = ErrSchedulerNotFound.Wrap(err).FastGenWithCause() + err1 = ErrSchedulerNotFound.Wrap(err) + err2 = ErrSchedulerNotFound.Wrap(err) re.True(errors.ErrorEqual(err1, err2)) err1 = ErrSchedulerNotFound.FastGenByArgs() - err2 = ErrSchedulerNotFound.Wrap(err).FastGenWithCause() + err2 = ErrSchedulerNotFound.Wrap(err) re.False(errors.ErrorEqual(err1, err2)) err3 := errors.New("test") err4 := errors.New("test") - err1 = ErrSchedulerNotFound.Wrap(err3).FastGenWithCause() - err2 = ErrSchedulerNotFound.Wrap(err4).FastGenWithCause() + err1 = ErrSchedulerNotFound.Wrap(err3) + err2 = ErrSchedulerNotFound.Wrap(err4) re.True(errors.ErrorEqual(err1, err2)) err3 = errors.New("test1") err4 = errors.New("test") - err1 = ErrSchedulerNotFound.Wrap(err3).FastGenWithCause() - err2 = ErrSchedulerNotFound.Wrap(err4).FastGenWithCause() + err1 = ErrSchedulerNotFound.Wrap(err3) + err2 = ErrSchedulerNotFound.Wrap(err4) re.False(errors.ErrorEqual(err1, err2)) } @@ -135,11 +145,16 @@ func TestErrorWithStack(t *testing.T) { m1 := lg.Message() log.Error("test", zap.Error(errors.WithStack(err))) m2 := lg.Message() + log.Error("test", ZapError(ErrStrconvParseInt.GenWithStackByCause(), err)) + m3 := lg.Message() // This test is based on line number and the first log is in line 141, the second is in line 142. // So they have the same length stack. Move this test to another place need to change the corresponding length. idx1 := strings.Index(m1, "[stack=") re.GreaterOrEqual(idx1, -1) idx2 := strings.Index(m2, "[stack=") re.GreaterOrEqual(idx2, -1) + idx3 := strings.Index(m3, "[stack=") + re.GreaterOrEqual(idx3, -1) re.Len(m2[idx2:], len(m1[idx1:])) + re.Len(m3[idx3:], len(m1[idx1:])) } diff --git a/pkg/schedule/hbstream/heartbeat_streams.go b/pkg/schedule/hbstream/heartbeat_streams.go index e7d7f688035..57a7521c0a7 100644 --- a/pkg/schedule/hbstream/heartbeat_streams.go +++ b/pkg/schedule/hbstream/heartbeat_streams.go @@ -139,7 +139,7 @@ func (s *HeartbeatStreams) run() { if stream, ok := s.streams[storeID]; ok { if err := stream.Send(msg); err != nil { log.Error("send heartbeat message fail", - zap.Uint64("region-id", msg.GetRegionId()), errs.ZapError(errs.ErrGRPCSend.Wrap(err).GenWithStackByArgs())) + zap.Uint64("region-id", msg.GetRegionId()), errs.ZapError(errs.ErrGRPCSend, err)) delete(s.streams, storeID) heartbeatStreamCounter.WithLabelValues(storeAddress, storeLabel, "push", "err").Inc() } else { diff --git a/pkg/schedule/plugin_interface.go b/pkg/schedule/plugin_interface.go index dd1ce3471e6..62ffe2eb900 100644 --- a/pkg/schedule/plugin_interface.go +++ b/pkg/schedule/plugin_interface.go @@ -46,19 +46,19 @@ func (p *PluginInterface) GetFunction(path string, funcName string) (plugin.Symb // open plugin filePath, err := filepath.Abs(path) if err != nil { - return nil, errs.ErrFilePathAbs.Wrap(err).FastGenWithCause() + return nil, errs.ErrFilePathAbs.Wrap(err) } log.Info("open plugin file", zap.String("file-path", filePath)) plugin, err := plugin.Open(filePath) if err != nil { - return nil, errs.ErrLoadPlugin.Wrap(err).FastGenWithCause() + return nil, errs.ErrLoadPlugin.Wrap(err) } p.pluginMap[path] = plugin } // get func from plugin f, err := p.pluginMap[path].Lookup(funcName) if err != nil { - return nil, errs.ErrLookupPluginFunc.Wrap(err).FastGenWithCause() + return nil, errs.ErrLookupPluginFunc.Wrap(err) } return f, nil } diff --git a/pkg/schedule/schedulers/evict_leader.go b/pkg/schedule/schedulers/evict_leader.go index 879aa9869b3..ae8b1ecf1ea 100644 --- a/pkg/schedule/schedulers/evict_leader.go +++ b/pkg/schedule/schedulers/evict_leader.go @@ -80,7 +80,7 @@ func (conf *evictLeaderSchedulerConfig) BuildWithArgs(args []string) error { id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } ranges, err := getKeyRanges(args[1:]) if err != nil { diff --git a/pkg/schedule/schedulers/grant_leader.go b/pkg/schedule/schedulers/grant_leader.go index 885f81e2442..027350536aa 100644 --- a/pkg/schedule/schedulers/grant_leader.go +++ b/pkg/schedule/schedulers/grant_leader.go @@ -63,7 +63,7 @@ func (conf *grantLeaderSchedulerConfig) BuildWithArgs(args []string) error { id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } ranges, err := getKeyRanges(args[1:]) if err != nil { diff --git a/pkg/schedule/schedulers/init.go b/pkg/schedule/schedulers/init.go index f60be1e5b06..57eb4b90985 100644 --- a/pkg/schedule/schedulers/init.go +++ b/pkg/schedule/schedulers/init.go @@ -129,7 +129,7 @@ func schedulersRegister() { id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } ranges, err := getKeyRanges(args[1:]) @@ -180,14 +180,14 @@ func schedulersRegister() { } leaderID, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } storeIDs := make([]uint64, 0) for _, id := range strings.Split(args[1], ",") { storeID, err := strconv.ParseUint(id, 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } storeIDs = append(storeIDs, storeID) } @@ -248,7 +248,7 @@ func schedulersRegister() { id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } ranges, err := getKeyRanges(args[1:]) if err != nil { @@ -365,7 +365,7 @@ func schedulersRegister() { if len(args) == 1 { limit, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - return errs.ErrStrconvParseUint.Wrap(err).FastGenWithCause() + return errs.ErrStrconvParseUint.Wrap(err) } conf.Limit = limit } diff --git a/pkg/schedule/schedulers/scheduler.go b/pkg/schedule/schedulers/scheduler.go index 1c788989454..38fc8f5607d 100644 --- a/pkg/schedule/schedulers/scheduler.go +++ b/pkg/schedule/schedulers/scheduler.go @@ -52,7 +52,7 @@ type Scheduler interface { func EncodeConfig(v interface{}) ([]byte, error) { marshaled, err := json.Marshal(v) if err != nil { - return nil, errs.ErrJSONMarshal.Wrap(err).FastGenWithCause() + return nil, errs.ErrJSONMarshal.Wrap(err) } return marshaled, nil } @@ -61,7 +61,7 @@ func EncodeConfig(v interface{}) ([]byte, error) { func DecodeConfig(data []byte, v interface{}) error { err := json.Unmarshal(data, v) if err != nil { - return errs.ErrJSONUnmarshal.Wrap(err).FastGenWithCause() + return errs.ErrJSONUnmarshal.Wrap(err) } return nil } diff --git a/pkg/schedule/schedulers/utils.go b/pkg/schedule/schedulers/utils.go index fea51798d1c..a22f992bda1 100644 --- a/pkg/schedule/schedulers/utils.go +++ b/pkg/schedule/schedulers/utils.go @@ -218,11 +218,11 @@ func getKeyRanges(args []string) ([]core.KeyRange, error) { for len(args) > 1 { startKey, err := url.QueryUnescape(args[0]) if err != nil { - return nil, errs.ErrQueryUnescape.Wrap(err).FastGenWithCause() + return nil, errs.ErrQueryUnescape.Wrap(err) } endKey, err := url.QueryUnescape(args[1]) if err != nil { - return nil, errs.ErrQueryUnescape.Wrap(err).FastGenWithCause() + return nil, errs.ErrQueryUnescape.Wrap(err) } args = args[2:] ranges = append(ranges, core.NewKeyRange(startKey, endKey)) diff --git a/pkg/tso/keyspace_group_manager.go b/pkg/tso/keyspace_group_manager.go index badcb18d5d8..58534de1642 100644 --- a/pkg/tso/keyspace_group_manager.go +++ b/pkg/tso/keyspace_group_manager.go @@ -542,7 +542,7 @@ func (kgm *KeyspaceGroupManager) InitializeGroupWatchLoop() error { putFn := func(kv *mvccpb.KeyValue) error { group := &endpoint.KeyspaceGroup{} if err := json.Unmarshal(kv.Value, group); err != nil { - return errs.ErrJSONUnmarshal.Wrap(err).FastGenWithCause() + return errs.ErrJSONUnmarshal.Wrap(err) } kgm.updateKeyspaceGroup(group) if group.ID == mcsutils.DefaultKeyspaceGroupID { diff --git a/pkg/utils/logutil/log.go b/pkg/utils/logutil/log.go index 3dc4430b066..8c0977818fa 100644 --- a/pkg/utils/logutil/log.go +++ b/pkg/utils/logutil/log.go @@ -70,7 +70,7 @@ func StringToZapLogLevel(level string) zapcore.Level { func SetupLogger(logConfig log.Config, logger **zap.Logger, logProps **log.ZapProperties, enabled ...bool) error { lg, p, err := log.InitLogger(&logConfig, zap.AddStacktrace(zapcore.FatalLevel)) if err != nil { - return errs.ErrInitLogger.Wrap(err).FastGenWithCause() + return errs.ErrInitLogger.Wrap(err) } *logger = lg *logProps = p diff --git a/pkg/utils/tempurl/check_env_linux.go b/pkg/utils/tempurl/check_env_linux.go index cf0e686cada..58f902f4bb7 100644 --- a/pkg/utils/tempurl/check_env_linux.go +++ b/pkg/utils/tempurl/check_env_linux.go @@ -36,7 +36,7 @@ func checkAddr(addr string) (bool, error) { return s.RemoteAddr.String() == addr || s.LocalAddr.String() == addr }) if err != nil { - return false, errs.ErrNetstatTCPSocks.Wrap(err).FastGenWithCause() + return false, errs.ErrNetstatTCPSocks.Wrap(err) } return len(tabs) < 1, nil } diff --git a/server/handler.go b/server/handler.go index 6c0679bd9f9..b91c8e368f9 100644 --- a/server/handler.go +++ b/server/handler.go @@ -470,7 +470,7 @@ func (h *Handler) PluginLoad(pluginPath string) error { // make sure path is in data dir filePath, err := filepath.Abs(pluginPath) if err != nil || !isPathInDirectory(filePath, h.s.GetConfig().DataDir) { - return errs.ErrFilePathAbs.Wrap(err).FastGenWithCause() + return errs.ErrFilePathAbs.Wrap(err) } c.LoadPlugin(pluginPath, ch)