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

Refine error code of ErrBkWriteRequest #1060

Closed
wants to merge 1 commit into from

Conversation

Tovi163
Copy link
Collaborator

@Tovi163 Tovi163 commented Jun 29, 2022

Signed-off-by: Tovi163 [email protected]

Refine the ErrBkWriteRequest to ErrBkWriteRequestByBackend and ErrBkWriteRequestClient

Signed-off-by: Tovi163 <[email protected]>

Refine the ErrBkWriteRequest to ErrBkWriteRequestByBackend and ErrBkWriteRequestClient
@kwanhur
Copy link
Contributor

kwanhur commented Jun 30, 2022

Need to fix the DCO problem first.

request.ErrMsg = err.Error()
p.proxyState.ErrBkWriteRequest.Inc(1)
p.proxyState.ErrBkWriteRequestByClient.Inc(1)
Copy link
Member

Choose a reason for hiding this comment

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

这里还未确定错误的具体原因(由client导致 或 backend导致)。因此,不能直接对ErrBkWriteRequestByClient进行计数

allowRetry = checkAllowRetry(cluster.RetryLevel(), outreq)

// if error is caused by backend server
rerr := err.(bfe_http.WriteRequestError)
if !rerr.CheckTargetError(request.RemoteAddr) {
request.ErrCode = bfe_basic.ErrBkWriteRequestByBackend
p.proxyState.ErrBkWriteRequestByBackend.Inc(1)
Copy link
Member

Choose a reason for hiding this comment

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

合理的做法是: 删除line 385/line387,在line 396后增加 :

else {
    request.ErrCode = bfe_basic.ErrBkWriteRequestByClient
    p.proxyState.ErrBkWriteRequestByClient.Inc(1)
}

@@ -40,7 +40,8 @@ var (
ErrBkNoBackend = errors.New("BK_NO_BACKEND") // no backend found
ErrBkRequestBackend = errors.New("BK_REQUEST_BACKEND") // forward request to backend error
ErrBkConnectBackend = errors.New("BK_CONNECT_BACKEND") // connect backend error
ErrBkWriteRequest = errors.New("BK_WRITE_REQUEST") // write request error (caused by bk or client)
ErrBkWriteRequestByBackend = errors.New("BK_WRITE_REQUEST_CAUSED_BY_BACKEND") // write request error (caused by backend)
ErrBkWriteRequestByClient = errors.New("BK_WRITE_REQUEST_CAUSED_BY_CLIENT") // write request error (caused by client)
Copy link
Member

Choose a reason for hiding this comment

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

建议:

  1. 把line 44 ErrBkWriteRequestByClient移动到line33之后(代表client类错误)
  2. 命名修改下 ErrClientReadRequest

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@iyangsj iyangsj closed this Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants