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

store/tikv: fix log of sendTxnHeartBeat #21017

Merged
merged 3 commits into from
Nov 13, 2020

Conversation

youjiali1995
Copy link
Contributor

Signed-off-by: youjiali1995 zlwgx1023@gmail.com

What problem does this PR solve?

Issue Number: close #15654

Problem Summary:

Refine the log of sendTxnHeartBeat.

What is changed and how it works?

What's Changed:
Refine the log of sendTxnHeartBeat.

Related changes

Check List

Tests

  • Manual test (add detailed scripts or steps below)
[2020/11/12 16:06:02.202 +08:00] [INFO] [2pc.go:831] ["send TxnHeartBeat"] [startTS=420785251087286273] [newTTL=35449]
[2020/11/12 16:06:02.207 +08:00] [WARN] [2pc.go:837] ["send TxnHeartBeat failed"] [error="txn 420785251087286273 heartbeat fail, primary key = 74800000000000003e5f728000000000000001, err = txn 420785251087286273 not found"] [txnStartTS=420785251087286273]

Release note

  • No release note.

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
@youjiali1995 youjiali1995 added type/bugfix This PR fixes a bug. sig/transaction SIG:Transaction labels Nov 12, 2020
@cfzjywxk
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 12, 2020
@@ -874,7 +875,7 @@ func sendTxnHeartBeat(bo *Backoffer, store *tikvStore, primary []byte, startTS,
}
cmdResp := resp.Resp.(*pb.TxnHeartBeatResponse)
if keyErr := cmdResp.GetError(); keyErr != nil {
return 0, errors.Errorf("txn %d heartbeat fail, primary key = %v, err = %s", startTS, primary, keyErr.Abort)
return 0, errors.Errorf("txn %d heartbeat fail, primary key = %v, err = %s", startTS, hex.EncodeToString(primary), extractKeyErr(keyErr))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to use prettyWriteKey to print?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we don't care what the key represents(a concrete table key or index key) in this case. Hex format is enough and easy for debug.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

@sticnarf
Copy link
Contributor

/lgtm

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 12, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 12, 2020
@youjiali1995
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 12, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@youjiali1995
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@youjiali1995 merge failed.

@youjiali1995
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@youjiali1995 merge failed.

@youjiali1995
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8a4b52e into pingcap:master Nov 13, 2020
@youjiali1995 youjiali1995 deleted the fix-log-of-txn-heartbeat branch November 13, 2020 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/transaction SIG:Transaction status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sendTxnHeartBeat can't print key error from TiKV
4 participants