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

pd, openapi(ticdc): retry one reset by peer when kill the pd instance, fix healthy panic #7069

Merged
merged 9 commits into from
Sep 16, 2022
10 changes: 9 additions & 1 deletion cdc/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,15 @@ func (h *OpenAPI) ServerStatus(c *gin.Context) {
func (h *OpenAPI) Health(c *gin.Context) {
ctx := c.Request.Context()

health, err := h.statusProvider().IsHealthy(ctx)
var err error
provider := h.statusProvider()
if provider == nil {
err = cerror.ErrOwnerNotFound.FastGenByArgs()
c.JSON(http.StatusInternalServerError, model.NewHTTPError(err))
return
}

health, err := provider.IsHealthy(ctx)
if err != nil {
c.IndentedJSON(http.StatusInternalServerError, model.NewHTTPError(err))
return
Expand Down
20 changes: 15 additions & 5 deletions cdc/api/v1/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,21 @@ func TestHealth(t *testing.T) {
// capture is owner
ctrl := gomock.NewController(t)
cp := mock_capture.NewMockCapture(ctrl)
sp := mock_owner.NewMockStatusProvider(ctrl)
ownerRouter := newRouter(cp, sp)
api := testCase{url: "/api/v1/health", method: "GET"}
ownerRouter := newRouter(cp, nil)

cp.EXPECT().IsReady().Return(true).AnyTimes()
cp.EXPECT().StatusProvider().Return(nil).Times(1)

api := testCase{url: "/api/v1/health", method: "GET"}

w := httptest.NewRecorder()
req, _ := http.NewRequestWithContext(context.Background(), api.method, api.url, nil)
ownerRouter.ServeHTTP(w, req)
require.Equal(t, 500, w.Code)

sp := mock_owner.NewMockStatusProvider(ctrl)
ownerRouter = newRouter(cp, sp)

cp.EXPECT().Info().DoAndReturn(func() (model.CaptureInfo, error) {
return model.CaptureInfo{}, nil
}).AnyTimes()
Expand All @@ -911,8 +921,8 @@ func TestHealth(t *testing.T) {
// IsHealthy returns error.
isHealthError := sp.EXPECT().IsHealthy(gomock.Any()).
Return(false, cerror.ErrOwnerNotFound)
w := httptest.NewRecorder()
req, _ := http.NewRequestWithContext(context.Background(), api.method, api.url, nil)
w = httptest.NewRecorder()
req, _ = http.NewRequestWithContext(context.Background(), api.method, api.url, nil)
ownerRouter.ServeHTTP(w, req)
require.Equal(t, 500, w.Code)

Expand Down
5 changes: 5 additions & 0 deletions pkg/errorutil/ignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,10 @@ func IsRetryableEtcdError(err error) bool {
if strings.Contains(etcdErr.Error(), "received prior goaway: code: NO_ERROR") {
return true
}

// this may happen if the PD instance shutdown by `kill -9`, no matter the instance is the leader or not.
if strings.Contains(etcdErr.Error(), "connection reset by peer") {
overvenus marked this conversation as resolved.
Show resolved Hide resolved
return true
}
return false
}
2 changes: 2 additions & 0 deletions pkg/errorutil/ignore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ func TestIsRetryableEtcdError(t *testing.T) {
{errors.New("rpc error: code = Unavailable desc = closing transport due to: " +
"connection error: desc = \\\"error reading from server: EOF\\\", " +
"received prior goaway: code: NO_ERROR\""), true},
{errors.New("rpc error: code = Unavailable desc = error reading from server: " +
"xxx: read: connection reset by peer"), true},
}

for _, item := range cases {
Expand Down