Skip to content

executor: fix log desensitization bug in prestmt (#19392)#20048

Merged
ti-srebot merged 2 commits intopingcap:release-4.0from
ti-srebot:release-4.0-9620b710a006
Sep 17, 2020
Merged

executor: fix log desensitization bug in prestmt (#19392)#20048
ti-srebot merged 2 commits intopingcap:release-4.0from
ti-srebot:release-4.0-9620b710a006

Conversation

@ti-srebot
Copy link
Contributor

cherry-pick #19392 to release-4.0


Signed-off-by: crazycs520 crazycs520@gmail.com

What problem does this PR solve?

Close #18566 (comment)

When tidb log enables desensitization (set @@global.tidb_log_desensitization = 1;), the slow-log and log may still out put the previous statement that doesn't desensitization:

before

...
# Prev_stmt: insert into t values (1,1),(2,2),(3,3),(4,4),(5,5)
commit;

this PR

...
# Prev_stmt: insert into t values ( ... ) , ( ... ) , ( ... ) , ( ... ) , ( ... )
commit;

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Manual test
set @@tidb_slow_log_threshold=0;
begin;insert into t values (1,1),(2,2),(3,3),(4,4),(5,5); 
commit;
set @@tidb_slow_log_threshold=100;
-- Then check the tidb-slow-log file.

Side effects

  • Performance regression
    • Consumes more CPU

Release note

  • Fix log desensitization bug in prestmt

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

Signed-off-by: crazycs520 <crazycs520@gmail.com>
Copy link
Contributor

@jackysp jackysp 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 Sep 17, 2020
@crazycs520
Copy link
Contributor

/run-all-tests

Copy link
Contributor

@tangenta tangenta 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 status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 17, 2020
Copy link
Contributor

@AilinKid AilinKid 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 status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Sep 17, 2020
@AilinKid
Copy link
Contributor

/run-unit-test

@jackysp
Copy link
Contributor

jackysp commented Sep 17, 2020

/merge

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

/run-all-tests

@ti-srebot ti-srebot merged commit 4ae5f54 into pingcap:release-4.0 Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Everything related with security sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/4.0-cherry-pick

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants