kv: add error code for package kv #12866

jackysp · 2019-10-22T07:17:56Z

What problem does this PR solve?

Not all the errors in kv package have error code.

What is changed and how it works?

Add them. Related to pingcap/parser#584.

Check List

Tests

Unit test

Code changes

Has exported variable/fields change

Side effects

Breaking backward compatibility

codecov · 2019-10-22T11:24:17Z

Codecov Report

Merging #12866 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12866   +/-   ##
===========================================
  Coverage   80.4066%   80.4066%           
===========================================
  Files           468        468           
  Lines        111946     111946           
===========================================
  Hits          90012      90012           
  Misses        15209      15209           
  Partials       6725       6725

jackysp · 2019-10-26T04:01:41Z

This PR fixes two issues. One is to separate the error that was originally mapped to ErrUnknown 1105 into multiple error codes, and the other is to put https://github.com/pingcap/tidb/pull/12866/files#diff -7e048d4e2f4a505912ffd33bd900b6d3R43 is changed to ErrDupUnknownInIndex. The first one I think is ok, the second one, I think it needs to review. PTAL @zimulala @crazycs520

jackysp · 2019-10-26T04:02:20Z

ddl/rollingback.go

@@ -40,7 +40,7 @@ func convertAddIdxJob2RollbackJob(t *meta.Meta, job *model.Job, tblInfo *model.T
 	}

 	if kv.ErrKeyExists.Equal(err) {
-		return ver, kv.ErrKeyExists.GenWithStack("Duplicate for key %s", indexInfo.Name.O)
+		return ver, ErrDupUnknownInIndex.GenWithStackByArgs(indexInfo.Name.O)


@zimulala @crazycs520 PTAL

Why change this error code? ErrDupUnknownInIndex error code is 1859.

mysql> select * from t; +------+ | a | +------+ | 1 | | 1 | +------+ 2 rows in set (0.01 sec) mysql> alter table t add unique index idx(a); ERROR 1062 (23000): Duplicate entry '1' for key 'idx'

Suggested change

return ver, ErrDupUnknownInIndex.GenWithStackByArgs(indexInfo.Name.O)

return ver, ErrKeyExists.GenWithStackByArgs("", indexInfo.Name.O)

how about this?

PTAL @zimulala

SunRunAway · 2019-10-29T04:41:56Z

store/tikv/snapshot.go

@@ -376,7 +376,7 @@ func extractKeyErr(keyErr *pb.KeyError) error {
 		return newWriteConflictError(keyErr.Conflict)
 	}
 	if keyErr.Retryable != "" {
-		return kv.ErrTxnRetryable.FastGenByArgs("tikv restarts txn: " + keyErr.GetRetryable())
+		return kv.ErrTxnRetryable.FastGenByArgs(keyErr.GetRetryable())
 	}
 	if keyErr.Abort != "" {
 		err := errors.Errorf("tikv aborts txn: %s", keyErr.GetAbort())


Should this error have an error code?

Yes, but not in this plan. There are many similar problems.

zimulala

LGTM

lysu

LGTM

sre-bot · 2019-10-30T05:29:12Z

Your auto merge job has been accepted, waiting for 13003, 12916

sre-bot · 2019-10-30T05:49:24Z

/run-all-tests

sre-bot · 2019-10-30T05:55:24Z

@jackysp merge failed.

lysu · 2019-10-30T15:01:02Z

/merge

sre-bot · 2019-10-30T15:01:23Z

/run-all-tests

sre-bot · 2019-10-30T15:03:27Z

@jackysp merge failed.

jackysp · 2019-11-01T06:58:35Z

/merge

sre-bot · 2019-11-01T07:01:03Z

/run-all-tests

sre-bot · 2019-11-01T07:03:07Z

@jackysp merge failed.

zyxbest · 2019-11-01T07:10:12Z

/run-mybatis-test

jackysp added the type/enhancement The issue or PR belongs to an enhancement. label Oct 22, 2019

kennytm mentioned this pull request Oct 25, 2019

mysql: add some errors for TiDB pingcap/parser#584

Merged

jackysp requested review from zimulala and crazycs520 October 26, 2019 04:01

jackysp commented Oct 26, 2019

View reviewed changes

jackysp requested review from lysu and coocood October 26, 2019 04:05

SunRunAway reviewed Oct 29, 2019

View reviewed changes

zimulala reviewed Oct 30, 2019

View reviewed changes

jackysp force-pushed the errors branch from 17746a8 to ee4eb2e Compare October 30, 2019 04:26

jackysp requested a review from SunRunAway October 30, 2019 04:45

jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 30, 2019

lysu previously approved these changes Oct 30, 2019

View reviewed changes

lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 30, 2019

jackysp added the status/can-merge Indicates a PR has been approved by a committer. label Oct 30, 2019

zz-jason added the status/ReqChange label Oct 30, 2019

jackysp added 6 commits October 30, 2019 22:43

kv: add error code for common errors

5cba7a9

fix fmt

c902054

fix tidy

191d1af

change test

3927f3f

clean go.mod

1f349e6

fix tidy again

00da851

jackysp dismissed lysu’s stale review via 00da851 October 30, 2019 14:43

jackysp force-pushed the errors branch from bb3bb05 to 00da851 Compare October 30, 2019 14:43

jackysp removed the status/ReqChange label Oct 30, 2019

lysu previously approved these changes Oct 30, 2019

View reviewed changes

revert go.mod go.sum

b525a91

jackysp dismissed lysu’s stale review via b525a91 October 30, 2019 14:58

lysu approved these changes Oct 30, 2019

View reviewed changes

Merge branch 'master' into errors

2f5b21b

jackysp merged commit ff6a4f7 into pingcap:master Nov 1, 2019

jackysp added the status/all tests passed label Nov 1, 2019

lysu mentioned this pull request Nov 7, 2019

clean up error code and message for parser pingcap/parser#618

Merged

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

kv: add error code for package kv (pingcap#12866)

f669f0f

jackysp deleted the errors branch February 27, 2020 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kv: add error code for package kv #12866

kv: add error code for package kv #12866

jackysp commented Oct 22, 2019 •

edited

Loading

codecov bot commented Oct 22, 2019 •

edited

Loading

jackysp commented Oct 26, 2019

jackysp Oct 26, 2019

zimulala Oct 28, 2019

jackysp Oct 28, 2019

jackysp Oct 28, 2019

SunRunAway Oct 29, 2019 •

edited

Loading

jackysp Oct 29, 2019

zimulala left a comment

lysu left a comment

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

lysu commented Oct 30, 2019

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

jackysp commented Nov 1, 2019

sre-bot commented Nov 1, 2019

sre-bot commented Nov 1, 2019

zyxbest commented Nov 1, 2019

	return ver, ErrDupUnknownInIndex.GenWithStackByArgs(indexInfo.Name.O)
	return ver, ErrKeyExists.GenWithStackByArgs("", indexInfo.Name.O)

kv: add error code for package kv #12866

kv: add error code for package kv #12866

Conversation

jackysp commented Oct 22, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Oct 22, 2019 • edited Loading

Codecov Report

jackysp commented Oct 26, 2019

jackysp Oct 26, 2019

Choose a reason for hiding this comment

zimulala Oct 28, 2019

Choose a reason for hiding this comment

jackysp Oct 28, 2019

Choose a reason for hiding this comment

jackysp Oct 28, 2019

Choose a reason for hiding this comment

SunRunAway Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

jackysp Oct 29, 2019

Choose a reason for hiding this comment

zimulala left a comment

Choose a reason for hiding this comment

lysu left a comment

Choose a reason for hiding this comment

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

lysu commented Oct 30, 2019

sre-bot commented Oct 30, 2019

sre-bot commented Oct 30, 2019

jackysp commented Nov 1, 2019

sre-bot commented Nov 1, 2019

sre-bot commented Nov 1, 2019

zyxbest commented Nov 1, 2019

jackysp commented Oct 22, 2019 •

edited

Loading

codecov bot commented Oct 22, 2019 •

edited

Loading

SunRunAway Oct 29, 2019 •

edited

Loading