*: add plan hint to slow log files when redaction is enabled #42170

jackysp · 2023-03-13T13:10:25Z

What problem does this PR solve?

Issue Number: close #42169

Problem Summary:
Plan hint will be redacted when enabling the log redaction in slow log files.

What is changed and how it works?

Add the hint as an item to the slow log files.

An example looks like this:

# Time: 2023-03-13T21:47:35.044936+08:00
...
# Plan: tidb_decode_plan('kwPwZTAJMV81CTAJMC44MAllcSh0ZXN0LnQuaSwgc2xlZXAoMSkpCTAJdGltZToxcywgbG9vcHM6MQkzODAgQnl0ZXMJTi9BCjEJMzFfNwkwCTEJZGF0YTpUYWJsZUZ1bGxTY2FuXzYJMQlBHDc5Mi4xwrVzEUd0MiwgY29wX3Rhc2s6IHtudW06IDEsIG1heDogNTY0BSlEcHJvY19rZXlzOiAwLCBycGNfEScBDAB0AZcUIDQ4Ni41BS6AY29wcl9jYWNoZV9oaXRfcmF0aW86IDAuMDAsIGJ1aWxkBWwIX2R1BRoYbjogMzkuNwU68GVtYXhfZGlzdHNxbF9jb25jdXJyZW5jeTogMX0JMTg2IEJ5dGVzCU4vQQoyCTQzXzYJMV8wCTEJdGFibGU6dCwga2VlcCBvcmRlcjpmYWxzZSwgc3RhdHM6cHNldWRvCTEJdGlrdl8F6gB7BbYQNDQ2LjgFeylTKDB9CU4vQQlOL0EK')
# Plan_digest: 21c0867e8b9be5e6cfd2a7cbcf7e9f757942f21798074863465d8cb05f48194e
# Binary_plan: tidb_decode_binary_plan('/wOYCvoDCgtTZWxlY3Rpb25fNRKcAwoNVGFibGVSZWFkZXJfNxKQAQoPBRJQRnVsbFNjYW5fNiGamZmZmVlkQCkABQHwXvA/MAE4AkACSgsKCQoEdGVzdBIBdFIea2VlcCBvcmRlcjpmYWxzZSwgc3RhdHM6cHNldWRvaiJ0aWt2X3Rhc2s6e3RpbWU6NDQ2LjjCtXMsIGxvb3BzOjB9cP//////AQMEAXgBBgUBIAEhXB0TPJgnLjJ/ADQBQAFSFGRhdGE6VGFibB2tMFoWdGltZTo3OTIuMcIZXRgyYqMBY29wCYBEIHtudW06IDEsIG1heDogNTY0BSpEcHJvY19rZXlzOiAwLCBycGNfEScBDAVXFCA0ODYuNQUugGNvcHJfY2FjaGVfaGl0X3JhdGlvOiAwLjAwLCBidWlsZAVsCF9kdQUaGG46IDM5LjcFOnBtYXhfZGlzdHNxbF9jb25jdXJyZW5jeTogMX1wujL6ACRF/RuhjD5QQCmaIYIMmek/OAH4DBZlcSghdkAudC5pLCBzbGVlcCgxKSlaEAWjFDFzLCBsbyFRQDFw/AJ4////////////ARgB')
select /*+ use_index(t, primary) */ * from t use index (`primary`) where i = sleep(?);

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2023-03-13T13:10:27Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

AndreMouche
qw4990

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2023-03-13T13:10:27Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

jackysp · 2023-03-13T13:56:37Z

/test all

jackysp · 2023-03-13T14:43:00Z

/test unit-test

jackysp · 2023-03-13T15:05:16Z

/test unit-test

AndreMouche · 2023-03-14T09:20:10Z

executor/slow_query_test.go

@@ -186,7 +186,7 @@ select * from t;`
 		`0,0,0,0,0,0,0,0,0,0,0,0,,0,0,0,0,0,0,0.38,0.021,0,0,0,1,637,0,10,10,10,10,100,,,1,42a1c8aae6f133e934d4bf0147491709a8812ea05ff8819ec522780fe657b772,t1:1,t2:2,` +
 		`0.1,0.2,0.03,127.0.0.1:20160,0.05,0.6,0.8,0.0.0.0:20160,70724,65536,0,0,0,0,0,,` +
 		`Cop_backoff_regionMiss_total_times: 200 Cop_backoff_regionMiss_total_time: 0.2 Cop_backoff_regionMiss_max_time: 0.2 Cop_backoff_regionMiss_max_addr: 127.0.0.1 Cop_backoff_regionMiss_avg_time: 0.2 Cop_backoff_regionMiss_p90_time: 0.2 Cop_backoff_rpcPD_total_times: 200 Cop_backoff_rpcPD_total_time: 0.2 Cop_backoff_rpcPD_max_time: 0.2 Cop_backoff_rpcPD_max_addr: 127.0.0.1 Cop_backoff_rpcPD_avg_time: 0.2 Cop_backoff_rpcPD_p90_time: 0.2 Cop_backoff_rpcTiKV_total_times: 200 Cop_backoff_rpcTiKV_total_time: 0.2 Cop_backoff_rpcTiKV_max_time: 0.2 Cop_backoff_rpcTiKV_max_addr: 127.0.0.1 Cop_backoff_rpcTiKV_avg_time: 0.2 Cop_backoff_rpcTiKV_p90_time: 0.2,` +
-		`0,0,1,0,1,1,0,,60e9378c746d9a2be1c791047e008967cf252eb6de9167ad3aa6098fa2d523f4,` +
+		`0,0,1,0,1,1,0,,60e9378c746d9a2be1c791047e008967cf252eb6de9167ad3aa6098fa2d523f4,,` +


could you please add a test with hint?

I don't know how to generate a real slow log in unit test framework. Seems all the tests of slow logs are parsing/reading the hand-writting files. The useful test is to check how the hint to be generated.

jackysp · 2023-03-15T12:26:04Z

/rebuild

jackysp · 2023-03-17T07:40:17Z

/hold

AndreMouche

LGTM

hawkingrei · 2023-03-17T14:32:44Z

/test all

jackysp · 2023-03-17T15:40:58Z

/unhold

kennytm · 2023-03-20T06:33:49Z

parser/lexer.go

@@ -520,7 +528,7 @@ func startWithSlash(s *Scanner) (tok int, pos Pos, lit string) {

 	case '+': // '/*+' optimizer hints
 		// See https://dev.mysql.com/doc/refman/5.7/en/optimizer-hints.html
-		if _, ok := hintedTokens[s.lastKeyword]; ok {
+		if _, ok := hintedTokens[s.lastKeyword]; ok || s.keepHint {


Why is this change needed? The check is to mimic MySQL's behavior that a hint must follow certain keywords (SELECT, INSERT, etc.); I don't think normalization will move the hint elsewhere.

When normalizing SQL, the value of lastKeyword will always be 0, resulting in the omission of all hints.

I am unable to find an appropriate method of handling it.

qw4990 · 2023-03-21T12:20:04Z

/merge

ti-chi-bot · 2023-03-21T12:20:09Z

This pull request has been accepted and is ready to merge.

Commit hash: 49ff26235bd29ea91efefe8b061fb9024cfcef47

hawkingrei · 2023-03-21T16:04:10Z

/retest

hawkingrei · 2023-03-21T17:59:09Z

/retest

hawkingrei · 2023-03-22T01:07:57Z

/retest

hawkingrei · 2023-03-22T08:45:50Z

/retest

jackysp · 2023-03-22T09:11:10Z

/retest

jackysp · 2023-03-22T12:55:14Z

/retest

hawkingrei · 2023-03-23T04:21:47Z

/retest

jackysp · 2023-03-24T01:56:04Z

/retest

hawkingrei · 2023-03-24T06:01:09Z

/test all

hawkingrei · 2023-03-25T05:01:05Z

/retest

Signed-off-by: Jack Yu <jackysp@gmail.com>

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei · 2023-03-25T06:38:40Z

/merge

ti-chi-bot · 2023-03-25T06:38:44Z

This pull request has been accepted and is ready to merge.

Commit hash: 6007b0e

jackysp marked this pull request as ready for review March 13, 2023 13:56

jackysp requested a review from a team as a code owner March 13, 2023 13:56

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 13, 2023

jackysp requested review from qw4990, AndreMouche and bb7133 March 13, 2023 15:58

jackysp added the type/enhancement label Mar 13, 2023

jackysp force-pushed the slow_log_hint branch from 5f0b7f0 to 3a9801d Compare March 14, 2023 04:23

AndreMouche reviewed Mar 14, 2023

View reviewed changes

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 17, 2023

AndreMouche approved these changes Mar 17, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 17, 2023

jackysp force-pushed the slow_log_hint branch from 3a9801d to acf32e6 Compare March 17, 2023 13:48

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 17, 2023

jackysp changed the title *: add plan hint to slow log files *: add plan hint to slow log files when redacting is enabled Mar 17, 2023

jackysp changed the title *: add plan hint to slow log files when redacting is enabled *: add plan hint to slow log files when redaction is enabled Mar 17, 2023

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 17, 2023

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 17, 2023

jackysp requested a review from kennytm March 17, 2023 15:41

kennytm reviewed Mar 20, 2023

View reviewed changes

qw4990 approved these changes Mar 21, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 21, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 21, 2023

jackysp and others added 3 commits March 25, 2023 14:37

*: keep hint when redact logs in slow log files

63e7fc4

Signed-off-by: Jack Yu <jackysp@gmail.com>

add a test case

c16d645

Signed-off-by: Jack Yu <jackysp@gmail.com>

update

6007b0e

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei force-pushed the slow_log_hint branch from 49ff262 to 6007b0e Compare March 25, 2023 06:37

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 25, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 25, 2023

ti-chi-bot merged commit 0344a3b into pingcap:master Mar 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add plan hint to slow log files when redaction is enabled #42170

*: add plan hint to slow log files when redaction is enabled #42170

jackysp commented Mar 13, 2023 •

edited

ti-chi-bot commented Mar 13, 2023 •

edited

ti-chi-bot commented Mar 13, 2023

jackysp commented Mar 13, 2023

jackysp commented Mar 13, 2023

jackysp commented Mar 13, 2023

AndreMouche Mar 14, 2023

jackysp Mar 14, 2023

jackysp commented Mar 15, 2023

jackysp commented Mar 17, 2023

AndreMouche left a comment

hawkingrei commented Mar 17, 2023

jackysp commented Mar 17, 2023

kennytm Mar 20, 2023

jackysp Mar 20, 2023

jackysp Mar 20, 2023

qw4990 commented Mar 21, 2023

ti-chi-bot commented Mar 21, 2023

hawkingrei commented Mar 21, 2023

hawkingrei commented Mar 21, 2023

hawkingrei commented Mar 22, 2023

hawkingrei commented Mar 22, 2023

jackysp commented Mar 22, 2023

jackysp commented Mar 22, 2023

hawkingrei commented Mar 23, 2023

jackysp commented Mar 24, 2023

hawkingrei commented Mar 24, 2023

hawkingrei commented Mar 25, 2023

hawkingrei commented Mar 25, 2023

ti-chi-bot commented Mar 25, 2023

*: add plan hint to slow log files when redaction is enabled #42170

*: add plan hint to slow log files when redaction is enabled #42170

Conversation

jackysp commented Mar 13, 2023 • edited

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 13, 2023 • edited

ti-chi-bot commented Mar 13, 2023

jackysp commented Mar 13, 2023

jackysp commented Mar 13, 2023

jackysp commented Mar 13, 2023

AndreMouche Mar 14, 2023

Choose a reason for hiding this comment

jackysp Mar 14, 2023

Choose a reason for hiding this comment

jackysp commented Mar 15, 2023

jackysp commented Mar 17, 2023

AndreMouche left a comment

Choose a reason for hiding this comment

hawkingrei commented Mar 17, 2023

jackysp commented Mar 17, 2023

kennytm Mar 20, 2023

Choose a reason for hiding this comment

jackysp Mar 20, 2023

Choose a reason for hiding this comment

jackysp Mar 20, 2023

Choose a reason for hiding this comment

qw4990 commented Mar 21, 2023

ti-chi-bot commented Mar 21, 2023

hawkingrei commented Mar 21, 2023

hawkingrei commented Mar 21, 2023

hawkingrei commented Mar 22, 2023

hawkingrei commented Mar 22, 2023

jackysp commented Mar 22, 2023

jackysp commented Mar 22, 2023

hawkingrei commented Mar 23, 2023

jackysp commented Mar 24, 2023

hawkingrei commented Mar 24, 2023

hawkingrei commented Mar 25, 2023

hawkingrei commented Mar 25, 2023

ti-chi-bot commented Mar 25, 2023

jackysp commented Mar 13, 2023 •

edited

ti-chi-bot commented Mar 13, 2023 •

edited