Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

worker: delete taskState when task is stopped/finished #1606

Merged
merged 10 commits into from
Apr 20, 2021
Merged

worker: delete taskState when task is stopped/finished #1606

merged 10 commits into from
Apr 20, 2021

Conversation

lichunzhu
Copy link
Contributor

@lichunzhu lichunzhu commented Apr 16, 2021 •
edited
Loading

What problem does this PR solve?

fix #1594

What is changed and how it works?

delete taskState when task is stopped/finished

Check List

Tests

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

When task is running:

dm_worker_cpu_usage 3.528409857613362
# HELP dm_worker_task_state state of task, 0 - invalidStage, 1 - New, 2 - Running, 3 - Paused, 4 - Stopped, 5 - Finished
# TYPE dm_worker_task_state gauge
dm_worker_task_state{source_id="mysql-replica-01",task="testdm"} 2
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary

After task is stopped:

# HELP dm_worker_cpu_usage the cpu usage of worker
# TYPE dm_worker_cpu_usage gauge
dm_worker_cpu_usage 3.521312888533776
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary
go_gc_duration_seconds{quantile="0"} 2.6124e-05
go_gc_duration_seconds{quantile="0.25"} 3.0232e-05

Related changes

  • Need to cherry-pick to the release branch

@ti-chi-bot ti-chi-bot requested review from GMHDBJD and lance6716 April 16, 2021 11:13
@lance6716
Copy link
Collaborator

there's a check_metric script, which could be used in tests.

@ti-chi-bot ti-chi-bot added size/M and removed size/S labels Apr 16, 2021
@lance6716 lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated labels Apr 19, 2021
@lance6716 lance6716 added this to the v2.0.3 milestone Apr 19, 2021
@lance6716 lance6716 added the needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated label Apr 19, 2021
lance6716
lance6716 approved these changes Apr 19, 2021
View reviewed changes
dm/worker/subtask.go

func updateTaskState(task, sourceID string, stage pb.Stage) {
if stage == pb.Stage_Stopped || stage == pb.Stage_Finished {
taskState.DeleteAllAboutLabels(prometheus.Labels{"task": task, "source_id": sourceID})
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might update the description of metics in dm.json and dm_instance.json

"The current state of subtasks..."

tests/_utils/check_metric_not_contains Outdated
sleep 1
done

echo "metric $metric_name has invalid value $metric"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
echo "metric $metric_name has invalid value $metric"
echo "metric $metric_name has invalid count $metric"

@ti-chi-bot ti-chi-bot added the status/LGT1 One reviewer already commented LGTM label Apr 19, 2021
@lance6716
Copy link
Collaborator

/cc @GMHDBJD

GMHDBJD
GMHDBJD reviewed Apr 20, 2021
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

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

Please resolve conflict. rest LGTM

@lance6716
Copy link
Collaborator

/run-all-tests

@lance6716
Copy link
Collaborator 

[2021-04-20T07:01:21.280Z] tests/_utils/check_metric: line 20: [: 1: unary operator expected

[2021-04-20T07:01:21.280Z] wait for valid metric for 1-th time

GMHDBJD
GMHDBJD approved these changes Apr 20, 2021
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD 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-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • GMHDBJD
  • lance6716

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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Apr 20, 2021
@lance6716
Copy link
Collaborator

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 8572a71

@lichunzhu
Copy link
Contributor Author

lichunzhu commented Apr 20, 2021 •
edited
Loading

integration test shardddl1 seems still unstable:
https://internal.pingcap.net/idc-jenkins/job/dm_ghpr_test/8962/display

@ti-chi-bot ti-chi-bot merged commit a557c8b into pingcap:master Apr 20, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Apr 20, 2021
@lichunzhu @ti-srebot
cherry pick pingcap#1606 to release-2.0
8368a7b 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Apr 20, 2021
Merged
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #1614

@ti-srebot ti-srebot added already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 labels Apr 20, 2021
ti-chi-bot pushed a commit that referenced this pull request Apr 21, 2021
@ti-srebot
worker: delete taskState when task is stopped/finished (#1606) (#1614)
22eec08
@lichunzhu lichunzhu deleted the fix1594 branch April 21, 2021 03:13
@lance6716 lance6716 removed the needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated label May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lance6716 lance6716 lance6716 approved these changes

@GMHDBJD GMHDBJD GMHDBJD approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated size/M status/can-merge status/LGT2 Two reviewers already commented LGTM, ready for merge
Projects
None yet
Milestone
v2.0.3
Development

Successfully merging this pull request may close these issues.

task state is not deleted in prometheus when task is stopped
5 participants
@lichunzhu @lance6716 @ti-chi-bot @ti-srebot @GMHDBJD