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

server: redact some error code, part(2/3) #20591

Merged
merged 4 commits into from
Dec 18, 2020

Conversation

cfzjywxk
Copy link
Contributor

@cfzjywxk cfzjywxk commented Oct 22, 2020

What problem does this PR solve?

Issue Number:

Problem Summary:
Part(2/3) of #18566 error message log redact, from ErrLogInUse to ErrSkippingLoggedTransaction.

What is changed and how it works?

What's Changed:
Add some redact parameters for these error messages.

How it Works:

Related changes

Check List

Tests

  • Unit test

Side effects

Release note

  • No release note

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

Would you like to add some cases in TestErrorRedact

@SunRunAway SunRunAway changed the title server: redact some error code server: redact some error code, part(2/3) Dec 17, 2020
@SunRunAway SunRunAway requested review from bb7133 and removed request for lysu and jackysp December 17, 2020 13:48
errno/errname.go Outdated
@@ -527,8 +527,8 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
ErrPartState: mysql.Message("Partition state cannot be defined from CREATE/ALTER TABLE", nil),
ErrLimitedPartRange: mysql.Message("The %-.64s handler only supports 32 bit integers in VALUES", nil),
ErrPluginIsNotLoaded: mysql.Message("Plugin '%-.192s' is not loaded", nil),
ErrWrongValue: mysql.Message("Incorrect %-.32s value: '%-.128s'", nil),
ErrNoPartitionForGivenValue: mysql.Message("Table has no partition for value %-.64s", nil),
ErrWrongValue: mysql.Message("Incorrect %-.32s value: '%-.128s'", []int{0, 1}),
Copy link
Contributor

Choose a reason for hiding this comment

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

The first argument is the Type and should not be redacted, am I right?

errno/errname.go Outdated
@@ -670,9 +670,9 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
ErrInsideTransactionPreventsSwitchBinlogDirect: mysql.Message("Cannot modify @@session.binlogDirectNonTransactionalUpdates inside a transaction", nil),
ErrStoredFunctionPreventsSwitchBinlogDirect: mysql.Message("Cannot change the binlog direct flag inside a stored function or trigger", nil),
ErrSpatialMustHaveGeomCol: mysql.Message("A SPATIAL index may only contain a geometrical type column", nil),
ErrTooLongIndexComment: mysql.Message("Comment for index '%-.64s' is too long (max = %d)", nil),
ErrTooLongIndexComment: mysql.Message("Comment for index '%-.64s' is too long (max = %d)", []int{0}),
Copy link
Contributor

Choose a reason for hiding this comment

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

The index name is not necessary to be redacted

errno/errname.go Outdated
ErrLockAborted: mysql.Message("Wait on a lock was aborted due to a pending exclusive lock", nil),
ErrDataOutOfRange: mysql.Message("%s value is out of range in '%s'", nil),
ErrDataOutOfRange: mysql.Message("%s value is out of range in '%s'", []int{0, 1}),
Copy link
Contributor

Choose a reason for hiding this comment

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

The first argument is the Type and should not be redacted.

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 18, 2020
@bb7133
Copy link
Member

bb7133 commented Dec 18, 2020

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 18, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 18, 2020
@bb7133
Copy link
Member

bb7133 commented Dec 18, 2020

/merge

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

/run-all-tests

@ti-srebot ti-srebot merged commit dd3087f into pingcap:master Dec 18, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 20, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21891

ti-srebot added a commit that referenced this pull request Jan 26, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants