runaway: Add processed_keys and request_ru to check #55147

HuSharp · 2024-08-02T00:48:45Z

What problem does this PR solve?

Issue Number: ref #54434, merged after pingcap/kvproto#1261 (review)

Problem Summary:

What changed and how does it work?

supports PROCESSED_KEYS and REQUEST_UNIT to check runaway

Explain

Can check doc https://github.com/pingcap/docs/pull/18858/files

Processed_keys: The number of keys that Coprocessor has processed. Compared with total_keys, processed_keys does not include the old versions of MVCC. A great difference between processed_keys and total_keys indicates that many old versions exist.

can check in Process_keys of slow query log

REQUEST_UNIT: The number of request units produced by the query. It's the total number of Request_unit_read and Request_unit_write.

can check in REQUEST_UNIT of statement fields desciption

Display

mysql> ALTER RESOURCE GROUP default QUERY_LIMIT=(EXEC_ELAPSED='2s', PROCESSED_KEYS=100, ACTION=KILL,  WATCH=EXACT DURATION='10m');
Query OK, 0 rows affected (0.06 sec)

mysql> SELECT * FROM information_schema.resource_groups;
+---------+------------+----------+-----------+--------------------------------------------------------------------------------+------------+
| NAME    | RU_PER_SEC | PRIORITY | BURSTABLE | QUERY_LIMIT                                                                    | BACKGROUND |
+---------+------------+----------+-----------+--------------------------------------------------------------------------------+------------+
| default | UNLIMITED  | MEDIUM   | YES       | EXEC_ELAPSED='2s', PROCESSED_KEYS=100, ACTION=KILL, WATCH=EXACT DURATION='10m0s' | NULL       |
+---------+------------+----------+-----------+--------------------------------------------------------------------------------+------------+
1 row in set (0.00 sec)

can check in log

[ERROR] [domain.go:1908] ["LoadSysVarCacheLoop failed"] [error="[executor:8253]Query execution was interrupted, identified as runaway query"]

check query result

mysql> select * from mysql.tidb_runaway_watch limit 3 \G;
*************************** 1. row ***************************
                 id: 2
resource_group_name: default
         start_time: 2024-09-06 08:54:01.886568
           end_time: 2024-09-06 09:04:01.886568
              watch: 1
         watch_text: SELECT variable_name, variable_value FROM mysql.global_variables
             source: 127.0.0.1:4000
             action: 3
  switch_group_name:
               rule: ProcessedKeys = 666(10)

mysql> SELECT * FROM mysql.tidb_RUNAWAY_queries limit 10 \G;
*************************** 1. row ***************************
resource_group_name: default
               time: 2024-09-06 17:17:42
         match_type: watch
             action: kill
       original_sql: SELECT variable_name, variable_value FROM mysql.global_variables
        plan_digest: e5003a13344e26b749329d8e96cae94e844a11bb7e027bd3ea5c2a39f7c07ee7
        tidb_server: 127.0.0.1:4000
               rule: ProcessedKeys = 666(10)

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Documentation

Contains experimental features

Release note

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

runaway: Add processed_keys and request_ru to check

ti-chi-bot · 2024-08-02T00:48:48Z

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

tiprow · 2024-08-02T00:49:04Z

Hi @HuSharp. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ti-chi-bot · 2024-08-16T08:04:17Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

HuSharp · 2024-08-21T04:20:36Z

/test all

tiprow · 2024-08-21T04:20:57Z

@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

codecov · 2024-08-21T04:47:42Z

Codecov Report

Attention: Patch coverage is 82.90909% with 47 lines in your changes missing coverage. Please review.

Project coverage is 75.8774%. Comparing base (e87affb) to head (30156c0).
Report is 1 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #55147        +/-   ##
================================================
+ Coverage   73.4116%   75.8774%   +2.4658%     
================================================
  Files          1601       1653        +52     
  Lines        444010     454562     +10552     
================================================
+ Hits         325955     344910     +18955     
+ Misses        98086      88121      -9965     
- Partials      19969      21531      +1562

Flag	Coverage Δ
integration	`51.9031% <16.0000%> (?)`
unit	`72.5323% <82.9090%> (+0.0565%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.9478% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`62.9171% <ø> (+17.0309%)`	⬆️

HuSharp · 2024-08-22T08:58:14Z

/test all

tiprow · 2024-08-22T08:58:36Z

@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

HuSharp · 2024-09-02T10:01:25Z

/retest

Signed-off-by: husharp <[email protected]>

glorv · 2024-09-14T02:52:19Z

/retest

hawkingrei · 2024-09-14T03:29:04Z

/test all

nolouch · 2024-09-19T02:06:08Z

/retest-required

Signed-off-by: husharp <[email protected]>

HuSharp · 2024-09-19T09:11:46Z

@glorv @JmPotato PTAL, thx~

HuSharp · 2024-09-19T09:28:03Z

/retest-required

pkg/parser/go.mod

JmPotato · 2024-09-20T06:18:20Z

pkg/ddl/resource_group.go

@@ -222,6 +222,8 @@ func SetDirectResourceGroupSettings(groupInfo *model.ResourceGroupInfo, opt *ast
 	case ast.ResourceGroupRunaway:
 		if len(opt.RunawayOptionList) == 0 {
 			resourceGroupSettings.Runaway = nil
+		} else {
+			resourceGroupSettings.Runaway = &model.ResourceGroupRunawaySettings{}


This part changed the behavior, for the old code, if resourceGroupSettings.Runaway is not nil, it won't be overwritten in SetDirectResourceGroupRunawayOption, but now it will be reset to an empty new one if len(opt.RunawayOptionList) > 0. Will this be a problem?

~~Nice catch, fixed.~~ Yes, the new behavior has been discussed with PM.

JmPotato · 2024-09-20T06:22:25Z

pkg/store/copr/coprocessor.go

@@ -915,6 +918,10 @@ func (sender *copIteratorTaskSender) run(connID uint64) {
 	if sender.respChan != nil {
 		close(sender.respChan)
 	}
+	if checker != nil {


No need to check since ResetTotalProcessedKeys will do it also.

struct's checker field is of type RunawayChecker, which is an interface, but it hasn't been initialized with a instance. When calling checker.ResetTotalProcessedKeys(), the value of checker is nil, leading to a runtime error: invalid memory address or nil pointer dereference.

You can check this playground

Instead of the following approach, maybe we can judge in ResetTotalProcessedKeys?

if checker != nil { if rcChecker, ok := checker.(*Checker); ok && rcChecker != nil { rcChecker.ResetTotalProcessedKeys() }

ti-chi-bot · 2024-09-20T07:22:21Z

[LGTM Timeline notifier]

Timeline:

2024-09-12 09:52:49.453778086 +0000 UTC m=+522839.194202023: ☑️ agreed by nolouch.
2024-09-20 07:22:20.585327261 +0000 UTC m=+1205010.325751200: ☑️ agreed by glorv.

glorv · 2024-09-20T07:23:02Z

@tangenta PTAL

Signed-off-by: husharp <[email protected]>

HuSharp · 2024-09-20T09:34:41Z

/retest-required

Signed-off-by: husharp <[email protected]>

glorv · 2024-09-23T02:24:48Z

/retest

Signed-off-by: husharp <[email protected]>

yudongusa

LGTM

ti-chi-bot · 2024-09-24T06:39:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: glorv, nolouch, tangenta, yudongusa

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [glorv,tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]
~~pkg/infoschema/OWNERS~~ [tangenta]
~~pkg/meta/OWNERS~~ [tangenta]
~~pkg/parser/OWNERS~~ [tangenta,yudongusa]
~~pkg/session/OWNERS~~ [yudongusa]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

HuSharp · 2024-09-24T06:54:11Z

/retest-required

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Aug 2, 2024

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 2, 2024

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2024

HuSharp changed the title ~~[DNM] runaway: add more params~~ [DNM] runaway: Add processed_keys and request_ru to check Aug 16, 2024

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 16, 2024

HuSharp force-pushed the add_more_param_in_runaway branch from cf6ddd7 to f4fdef6 Compare August 16, 2024 02:16

HuSharp changed the title ~~[DNM] runaway: Add processed_keys and request_ru to check~~ runaway: Add processed_keys and request_ru to check Aug 16, 2024

HuSharp force-pushed the add_more_param_in_runaway branch from f4fdef6 to db7e3f5 Compare August 16, 2024 02:51

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2024

ti-chi-bot bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/needs-linked-issue labels Aug 16, 2024

HuSharp force-pushed the add_more_param_in_runaway branch from 3ee3027 to 9fa3c96 Compare August 21, 2024 04:20

HuSharp force-pushed the add_more_param_in_runaway branch from 9fa3c96 to 4667c22 Compare August 22, 2024 08:56

HuSharp marked this pull request as ready for review August 23, 2024 02:12

ti-chi-bot bot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 23, 2024

HuSharp force-pushed the add_more_param_in_runaway branch from 4667c22 to dca78c7 Compare September 2, 2024 04:49

revert wrong merge

e668657

Signed-off-by: husharp <[email protected]>

HuSharp force-pushed the add_more_param_in_runaway branch from 8316701 to e668657 Compare September 13, 2024 14:28

resolve conflict

6766cf9

Signed-off-by: husharp <[email protected]>

JmPotato reviewed Sep 20, 2024

View reviewed changes

glorv approved these changes Sep 20, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 20, 2024

resolve conflict and address comment

caeb7fd

Signed-off-by: husharp <[email protected]>

revert overwriten

06ca6e6

Signed-off-by: husharp <[email protected]>

tangenta approved these changes Sep 23, 2024

View reviewed changes

resolve conflict

ca42322

Signed-off-by: husharp <[email protected]>

HuSharp requested a review from JmPotato September 23, 2024 06:54

resolve conflict

30156c0

Signed-off-by: husharp <[email protected]>

yudongusa approved these changes Sep 24, 2024

View reviewed changes

ti-chi-bot bot added the approved label Sep 24, 2024

ti-chi-bot bot merged commit 19caf52 into pingcap:master Sep 24, 2024
25 checks passed

HuSharp deleted the add_more_param_in_runaway branch September 24, 2024 07:42

nolouch mentioned this pull request Sep 26, 2024

Enhance runaway query management features #54434

Open

10 tasks

apollodafoni mentioned this pull request Oct 12, 2024

[FATAL] [bootstrap.go:1703] ["doReentrantDDL error"] [error="[schema:1054]Unknown column 'time' in 'tidb_runaway_queries'"] #56574

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runaway: Add processed_keys and request_ru to check #55147

runaway: Add processed_keys and request_ru to check #55147

HuSharp commented Aug 2, 2024 •

edited

Loading

ti-chi-bot bot commented Aug 2, 2024

tiprow bot commented Aug 2, 2024

ti-chi-bot bot commented Aug 16, 2024

HuSharp commented Aug 21, 2024

tiprow bot commented Aug 21, 2024

codecov bot commented Aug 21, 2024 •

edited

Loading

HuSharp commented Aug 22, 2024

tiprow bot commented Aug 22, 2024

HuSharp commented Sep 2, 2024

glorv commented Sep 14, 2024

hawkingrei commented Sep 14, 2024

nolouch commented Sep 19, 2024

HuSharp commented Sep 19, 2024

HuSharp commented Sep 19, 2024

JmPotato Sep 20, 2024

HuSharp Sep 20, 2024 •

edited

Loading

JmPotato Sep 20, 2024

HuSharp Sep 20, 2024 •

edited

Loading

ti-chi-bot bot commented Sep 20, 2024

glorv commented Sep 20, 2024

HuSharp commented Sep 20, 2024

glorv commented Sep 23, 2024

yudongusa left a comment

ti-chi-bot bot commented Sep 24, 2024

HuSharp commented Sep 24, 2024

runaway: Add processed_keys and request_ru to check #55147

runaway: Add processed_keys and request_ru to check #55147

Conversation

HuSharp commented Aug 2, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Explain

Display

Check List

Release note

ti-chi-bot bot commented Aug 2, 2024

tiprow bot commented Aug 2, 2024

ti-chi-bot bot commented Aug 16, 2024

HuSharp commented Aug 21, 2024

tiprow bot commented Aug 21, 2024

codecov bot commented Aug 21, 2024 • edited Loading

Codecov Report

HuSharp commented Aug 22, 2024

tiprow bot commented Aug 22, 2024

HuSharp commented Sep 2, 2024

glorv commented Sep 14, 2024

hawkingrei commented Sep 14, 2024

nolouch commented Sep 19, 2024

HuSharp commented Sep 19, 2024

HuSharp commented Sep 19, 2024

JmPotato Sep 20, 2024

Choose a reason for hiding this comment

HuSharp Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

JmPotato Sep 20, 2024

Choose a reason for hiding this comment

HuSharp Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 20, 2024

[LGTM Timeline notifier]

glorv commented Sep 20, 2024

HuSharp commented Sep 20, 2024

glorv commented Sep 23, 2024

yudongusa left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 24, 2024

HuSharp commented Sep 24, 2024

HuSharp commented Aug 2, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading

HuSharp Sep 20, 2024 •

edited

Loading

HuSharp Sep 20, 2024 •

edited

Loading