From c5592bb90e88d982520d400f59d781fa51df42d4 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Mon, 3 Jun 2024 15:58:12 +0800 Subject: [PATCH] Unify service discovery error message Signed-off-by: JmPotato --- client/errs/errno.go | 4 ++-- client/errs/errs.go | 3 ++- client/http/client.go | 6 ++++-- client/pd_service_discovery_test.go | 3 ++- client/resource_manager_client.go | 6 +----- errors.toml | 5 +++++ pkg/errs/errno.go | 1 + pkg/utils/apiutil/serverapi/middleware.go | 2 +- tests/integrations/mcs/tso/keyspace_group_manager_test.go | 5 +++-- tests/server/cluster/cluster_test.go | 2 +- 10 files changed, 22 insertions(+), 15 deletions(-) diff --git a/client/errs/errno.go b/client/errs/errno.go index a5fb453e4c0e..0dbcb4fe147a 100644 --- a/client/errs/errno.go +++ b/client/errs/errno.go @@ -25,7 +25,7 @@ const ( // NoLeaderErr indicates there is no leader in the cluster currently. NoLeaderErr = "no leader" // NotLeaderErr indicates the non-leader member received the requests which should be received by leader. - NotLeaderErr = "is not leader" + NotLeaderErr = "not leader" // MismatchLeaderErr indicates the non-leader member received the requests which should be received by leader. MismatchLeaderErr = "mismatch leader id" // NotServedErr indicates an tso node/pod received the requests for the keyspace groups which are not served by it. @@ -33,7 +33,7 @@ const ( // RetryTimeoutErr indicates the server is busy. RetryTimeoutErr = "retry timeout" // NotPrimaryErr indicates the non-primary member received the requests which should be received by primary. - NotPrimaryErr = "is not primary" + NotPrimaryErr = "not primary" ) // client errors diff --git a/client/errs/errs.go b/client/errs/errs.go index 2c25e009849c..da333efda4c3 100644 --- a/client/errs/errs.go +++ b/client/errs/errs.go @@ -31,7 +31,8 @@ func IsLeaderChange(err error) bool { return true } errMsg := err.Error() - return strings.Contains(errMsg, NotLeaderErr) || + return strings.Contains(errMsg, NoLeaderErr) || + strings.Contains(errMsg, NotLeaderErr) || strings.Contains(errMsg, MismatchLeaderErr) || strings.Contains(errMsg, NotServedErr) || strings.Contains(errMsg, NotPrimaryErr) diff --git a/client/http/client.go b/client/http/client.go index 161019713afb..123ca6164220 100644 --- a/client/http/client.go +++ b/client/http/client.go @@ -128,8 +128,8 @@ func (ci *clientInner) requestWithRetry( ) execFunc := func() error { defer func() { - // - If the status code is 503, it indicates that there may be PD leader/follower changes. - // - If the error message contains the leader/primary change information, it indicates that there may be PD leader/primary change. + // If the status code is 503, it indicates that there may be PD leader/follower changes. + // If the error message contains the leader/primary change information, it indicates that there may be PD leader/primary change. if statusCode == http.StatusServiceUnavailable || errs.IsLeaderChange(err) { ci.sd.ScheduleCheckMemberChanged() } @@ -243,6 +243,8 @@ func (ci *clientInner) doRequest( if readErr != nil { logFields = append(logFields, zap.NamedError("read-body-error", err)) } else { + // API server will return a JSON body containing the detailed error message + // when the status code is not `http.StatusOK` 200. bs = bytes.TrimSpace(bs) logFields = append(logFields, zap.ByteString("body", bs)) } diff --git a/client/pd_service_discovery_test.go b/client/pd_service_discovery_test.go index f4cde0e1911c..44171873b1a4 100644 --- a/client/pd_service_discovery_test.go +++ b/client/pd_service_discovery_test.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/kvproto/pkg/pdpb" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/tikv/pd/client/errs" "github.com/tikv/pd/client/grpcutil" "github.com/tikv/pd/client/testutil" "google.golang.org/grpc" @@ -205,7 +206,7 @@ func (suite *serviceClientTestSuite) TestServiceClient() { re.NotNil(leaderConn) _, err := pb.NewGreeterClient(followerConn).SayHello(suite.ctx, &pb.HelloRequest{Name: "pd"}) - re.ErrorContains(err, "not leader") + re.ErrorContains(err, errs.NotLeaderErr) resp, err := pb.NewGreeterClient(leaderConn).SayHello(suite.ctx, &pb.HelloRequest{Name: "pd"}) re.NoError(err) re.Equal("Hello pd", resp.GetMessage()) diff --git a/client/resource_manager_client.go b/client/resource_manager_client.go index 872b241cfe72..2cd4e8030359 100644 --- a/client/resource_manager_client.go +++ b/client/resource_manager_client.go @@ -35,10 +35,6 @@ const ( modify actionType = 1 groupSettingsPathPrefix = "resource_group/settings" controllerConfigPathPrefix = "resource_group/controller" - // errNotPrimary is returned when the requested server is not primary. - errNotPrimary = "not primary" - // errNotLeader is returned when the requested server is not pd leader. - errNotLeader = "not leader" ) // GroupSettingsPathPrefixBytes is used to watch or get resource groups. @@ -83,7 +79,7 @@ func (c *client) resourceManagerClient() (rmpb.ResourceManagerClient, error) { // gRPCErrorHandler is used to handle the gRPC error returned by the resource manager service. func (c *client) gRPCErrorHandler(err error) { - if strings.Contains(err.Error(), errNotPrimary) || strings.Contains(err.Error(), errNotLeader) { + if strings.Contains(err.Error(), errs.NotPrimaryErr) || strings.Contains(err.Error(), errs.NotLeaderErr) { c.pdSvcDiscovery.ScheduleCheckMemberChanged() } } diff --git a/errors.toml b/errors.toml index a275cfa75016..a61c23a6fbdf 100644 --- a/errors.toml +++ b/errors.toml @@ -16,6 +16,11 @@ error = ''' redirect failed ''' +["PD:apiutil:ErrRedirectNoLeader"] +error = ''' +redirect finds no leader +''' + ["PD:apiutil:ErrRedirectToNotLeader"] error = ''' redirect to not leader diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go index 8f67c59cfccb..1f56a821032f 100644 --- a/pkg/errs/errno.go +++ b/pkg/errs/errno.go @@ -197,6 +197,7 @@ var ( var ( ErrRedirect = errors.Normalize("redirect failed", errors.RFCCodeText("PD:apiutil:ErrRedirect")) ErrOptionNotExist = errors.Normalize("the option %s does not exist", errors.RFCCodeText("PD:apiutil:ErrOptionNotExist")) + ErrRedirectNoLeader = errors.Normalize("redirect finds no leader", errors.RFCCodeText("PD:apiutil:ErrRedirectNoLeader")) ErrRedirectToNotLeader = errors.Normalize("redirect to not leader", errors.RFCCodeText("PD:apiutil:ErrRedirectToNotLeader")) ErrRedirectToNotPrimary = errors.Normalize("redirect to not primary", errors.RFCCodeText("PD:apiutil:ErrRedirectToNotPrimary")) ) diff --git a/pkg/utils/apiutil/serverapi/middleware.go b/pkg/utils/apiutil/serverapi/middleware.go index 756ec571e6c1..0718702b5a55 100755 --- a/pkg/utils/apiutil/serverapi/middleware.go +++ b/pkg/utils/apiutil/serverapi/middleware.go @@ -210,7 +210,7 @@ func (h *redirector) ServeHTTP(w http.ResponseWriter, r *http.Request, next http leader := h.waitForLeader(r) // The leader has not been elected yet. if leader == nil { - http.Error(w, "no leader", http.StatusServiceUnavailable) + http.Error(w, errs.ErrRedirectNoLeader.FastGenByArgs().Error(), http.StatusServiceUnavailable) return } // If the leader is the current server now, we can handle the request directly. diff --git a/tests/integrations/mcs/tso/keyspace_group_manager_test.go b/tests/integrations/mcs/tso/keyspace_group_manager_test.go index 25d9516bf632..6d861962d9b6 100644 --- a/tests/integrations/mcs/tso/keyspace_group_manager_test.go +++ b/tests/integrations/mcs/tso/keyspace_group_manager_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" pd "github.com/tikv/pd/client" + clierrs "github.com/tikv/pd/client/errs" "github.com/tikv/pd/pkg/election" "github.com/tikv/pd/pkg/errs" mcsutils "github.com/tikv/pd/pkg/mcs/utils" @@ -467,8 +468,8 @@ func (suite *tsoKeyspaceGroupManagerTestSuite) dispatchClient( errMsg := err.Error() // Ignore the errors caused by the split and context cancellation. if strings.Contains(errMsg, "context canceled") || - strings.Contains(errMsg, "not leader") || - strings.Contains(errMsg, "not served") || + strings.Contains(errMsg, clierrs.NotLeaderErr) || + strings.Contains(errMsg, clierrs.NotServedErr) || strings.Contains(errMsg, "ErrKeyspaceNotAssigned") || strings.Contains(errMsg, "ErrKeyspaceGroupIsMerging") { continue diff --git a/tests/server/cluster/cluster_test.go b/tests/server/cluster/cluster_test.go index 07bcf3ee2a19..25e764acdaf0 100644 --- a/tests/server/cluster/cluster_test.go +++ b/tests/server/cluster/cluster_test.go @@ -662,7 +662,7 @@ func TestNotLeader(t *testing.T) { grpcStatus, ok := status.FromError(err) re.True(ok) re.Equal(codes.Unavailable, grpcStatus.Code()) - re.Equal("not leader", grpcStatus.Message()) + re.Equal(server.ErrNotLeader, grpcStatus.Message()) } func TestStoreVersionChange(t *testing.T) {