dxf: fix the task num count incorrectly #57127

fzzf678 · 2024-11-05T08:42:50Z

What problem does this PR solve?

Issue Number: close #57172

Problem Summary:

What changed and how does it work?

The task count metric is updating in scheduler loop, but the task maybe cleaned up in doCleanupTask(), the scheduler will not handle the finished/failed... state, so the task count metric will not be reduced.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
Use sysbench prepare 100 tables with 10000 rows. Watch the pannel:

No need to test
- I checked and no code files have been changed.

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

tiprow · 2024-11-05T08:43:07Z

Hi @fzzf678. 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.

codecov · 2024-11-05T08:58:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.6273%. Comparing base (7e491c3) to head (9aead88).
Report is 78 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #57127         +/-   ##
=================================================
- Coverage   73.1780%   57.6273%   -15.5507%     
=================================================
  Files          1655       1811        +156     
  Lines        456268     655664     +199396     
=================================================
+ Hits         333888     377842      +43954     
- Misses       101864     252146     +150282     
- Partials      20516      25676       +5160

Flag	Coverage Δ
integration	`39.6672% <100.0000%> (?)`
unit	`72.6146% <100.0000%> (+0.1504%)`	⬆️

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

Components	Coverage Δ
dumpling	`55.2602% <ø> (+2.3124%)`	⬆️
parser	`∅ <ø> (∅)`
br	`63.1869% <ø> (+17.4961%)`	⬆️

tangenta · 2024-11-06T09:16:09Z

pkg/metrics/grafana/tidb.json

@@ -16487,7 +16487,7 @@
          "targets": [
            {
              "exemplar": true,
-              "expr": "sum(tidb_disttask_subtasks{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", instance=~\"$instance\", status=~\"pending|running|reverting|revert_pending|paused\"}) by (exec_id)",
+              "expr": "sum(tidb_disttask_subtasks{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", instance=~\"$instance\", status=~\"pending|running|failed|canceled|paused\"}) by (exec_id)",


I think "failed" and "canceled" should not be considered as running?

The panel is Uncompleted Subtask Distribution on TiDB Nodes, so i think the except for 'succeed' tasks, all status should be included ?

fzzf678 · 2024-11-06T10:45:00Z

/retest

tiprow · 2024-11-06T10:45:55Z

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

In response to this:

/retest

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.

fzzf678 · 2024-11-06T11:58:50Z

/check-issue-triage-complete

ti-chi-bot · 2024-11-07T07:54:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CbcWestwolf, tangenta

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~~ [CbcWestwolf,tangenta]
~~pkg/disttask/OWNERS~~ [tangenta]

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

ti-chi-bot · 2024-11-07T07:54:45Z

[LGTM Timeline notifier]

Timeline:

2024-11-07 05:58:02.34515494 +0000 UTC m=+1106995.184310485: ☑️ agreed by tangenta.
2024-11-07 07:54:44.468091251 +0000 UTC m=+1113997.307246789: ☑️ agreed by CbcWestwolf.

fzzf678 · 2024-11-07T10:51:57Z

/retest

tiprow · 2024-11-07T10:52:20Z

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

In response to this:

/retest

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-11-14T08:59:10Z

In response to a cherrypick label: new pull request created to branch release-8.5: #57386.

close #57172

fzzf678 added 2 commits November 5, 2024 14:51

Update disttask.go

525ea66

update

14f9dfb

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 5, 2024

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Nov 5, 2024

fzzf678 added 3 commits November 5, 2024 18:15

Update collector.go

8342289

log

0fb03c5

Update scheduler_manager.go

b665e3a

ti-chi-bot bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 5, 2024

fzzf678 added 7 commits November 5, 2024 19:18

collect before exit

7edc2ca

log

47580ed

log

3dea90c

Update collector.go

1f136dc

use all subtasks

58d0161

reset all state

40a95a5

reset the count

2267bc1

ti-chi-bot 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 Nov 5, 2024

use history table

03dc1eb

fzzf678 marked this pull request as draft November 5, 2024 15:11

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2024

clear

5763910

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

Update collector.go

197b01e

fzzf678 added 3 commits November 6, 2024 15:11

Update collector.go

5c30f08

update finished metrics when cleanup

af206b4

update schedule metrics together

5cdc1ec

ti-chi-bot bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 6, 2024

update tidb.json expr

9aead88

ti-chi-bot bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Nov 6, 2024

fzzf678 marked this pull request as ready for review November 6, 2024 08:09

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

tangenta reviewed Nov 6, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Nov 6, 2024

tangenta approved these changes Nov 7, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 7, 2024

CbcWestwolf approved these changes Nov 7, 2024

View reviewed changes

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

ti-chi-bot bot merged commit 559f218 into pingcap:master Nov 7, 2024
23 checks passed

fzzf678 added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Nov 14, 2024

ti-chi-bot mentioned this pull request Nov 14, 2024

dxf: fix the task num count incorrectly (#57127) #57386

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Nov 14, 2024

dxf: fix the task num count incorrectly (#57127) (#57386)

7b466d3

close #57172

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dxf: fix the task num count incorrectly #57127

dxf: fix the task num count incorrectly #57127

fzzf678 commented Nov 5, 2024 •

edited

Loading

tiprow bot commented Nov 5, 2024

codecov bot commented Nov 5, 2024 •

edited

Loading

tangenta Nov 6, 2024

fzzf678 Nov 6, 2024

fzzf678 commented Nov 6, 2024

tiprow bot commented Nov 6, 2024

fzzf678 commented Nov 6, 2024

ti-chi-bot bot commented Nov 7, 2024

ti-chi-bot bot commented Nov 7, 2024

fzzf678 commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

ti-chi-bot commented Nov 14, 2024

dxf: fix the task num count incorrectly #57127

dxf: fix the task num count incorrectly #57127

Conversation

fzzf678 commented Nov 5, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Nov 5, 2024

codecov bot commented Nov 5, 2024 • edited Loading

Codecov Report

tangenta Nov 6, 2024

Choose a reason for hiding this comment

fzzf678 Nov 6, 2024

Choose a reason for hiding this comment

fzzf678 commented Nov 6, 2024

tiprow bot commented Nov 6, 2024

fzzf678 commented Nov 6, 2024

ti-chi-bot bot commented Nov 7, 2024

ti-chi-bot bot commented Nov 7, 2024

[LGTM Timeline notifier]

fzzf678 commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

ti-chi-bot commented Nov 14, 2024

fzzf678 commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading