Skip to content

Add time and thread_id for snapshot to check stale snapshots#2229

Merged
ti-srebot merged 12 commits intopingcap:masterfrom
JaySon-Huang:stale_ps_snapshot_logging
Jun 23, 2021
Merged

Add time and thread_id for snapshot to check stale snapshots#2229
ti-srebot merged 12 commits intopingcap:masterfrom
JaySon-Huang:stale_ps_snapshot_logging

Conversation

@JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Jun 21, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: related to #2199

Problem Summary: Suspicious that some tasks hold snapshot for a long time, prevent PageStorage from GCing old PageFiles

What is changed and how it works?

Add create_time and thread_id when creating snapshots. After PageStorage::gc, log the oldest snapshot living time and its thread_id.
We can trace to thread_id to know whether the snapshot is created for reading / background task.
When the create_time and thread_id and the period of logging files, we can know exactly which task holds the snapshot for a long time.

Related changes

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • Deploy a TiFlash with these changes to check the logging

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM

Release note

  • No release note

@JaySon-Huang
Split header
697a09d 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Add some metrics for snapshots list length
f75c461 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Add metrics for creating snapshot
2bdd170 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Add logging for longest snapshot lifetime, and its thread id
44f8245 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Revert "Add metrics for creating snapshot"
fca59a7 
This reverts commit 0a8060ae9eaa48765ca0e449f8b78eef3c98f9d9.
@JaySon-Huang
Cleanup
06a66cf 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang JaySon-Huang self-assigned this Jun 21, 2021
@JaySon-Huang JaySon-Huang requested a review from flowbehappy June 21, 2021 14:03
@JaySon-Huang
Log warning message for suspicious stale snapshots
efce00c 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
flowbehappy
flowbehappy reviewed Jun 22, 2021
View reviewed changes
flowbehappy
flowbehappy reviewed Jun 22, 2021
View reviewed changes
flowbehappy
flowbehappy reviewed Jun 22, 2021
View reviewed changes
@JaySon-Huang
Address comment, better naming
e701d8e 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
JaySon-Huang
JaySon-Huang commented Jun 22, 2021
View reviewed changes
dbms/src/Storages/DeltaMerge/SegmentReadTaskPool.cpp
@@ -0,0 +1,77 @@
#include <Common/CurrentMetrics.h>
Copy link
Contributor Author

@JaySon-Huang JaySon-Huang Jun 22, 2021

Choose a reason for hiding this comment

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

Split these function from SegmentReadTaskPool.h into SegmentReadTaskPool.cpp because if we include CurrentMetrics.h in SegmentReadTaskPool.h, it causes link error.
Split these functions def into cpp file and only include CurrentMetrics.h in the cpp file to fix that link error.

JaySon-Huang and others added 3 commits June 22, 2021 13:11
@JaySon-Huang
Add metrics for DeltaTree level num of snapshot
6e84520 
Revert "Revert "Add metrics for creating snapshot""

This reverts commit fca59a7.
@JaySon-Huang
Merge branch 'master' into stale_ps_snapshot_logging
cda9c94
@JaySon-Huang
Fix ut compile error
a905081 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jun 22, 2021

/run-all-tests

@JaySon-Huang JaySon-Huang mentioned this pull request Jun 22, 2021
Merged
@JaySon-Huang
Add a Grafana panel for snapshots
7883525 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jun 22, 2021

@flowbehappy PTAL again

And also the changes in TiDB side: pingcap/tidb#25678 /cc @lidezhu

@flowbehappy flowbehappy self-requested a review June 23, 2021 02:39
flowbehappy
flowbehappy approved these changes Jun 23, 2021
View reviewed changes
Copy link
Contributor

@flowbehappy flowbehappy 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 Jun 23, 2021
@JaySon-Huang JaySon-Huang added needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 needs-cherry-pick-release-5.1 PR which needs to be cherry-picked to release-5.1 labels Jun 23, 2021
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jun 23, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 23, 2021
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jun 23, 2021

/run-all-tests

@ti-srebot ti-srebot merged commit 8f8b729 into pingcap:master Jun 23, 2021
@JaySon-Huang JaySon-Huang deleted the stale_ps_snapshot_logging branch June 23, 2021 02:56
@ti-srebot ti-srebot mentioned this pull request Jun 23, 2021
Closed
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jun 23, 2021

cherry pick to release-4.0 in PR #2238

@ti-srebot ti-srebot mentioned this pull request Jun 23, 2021
Merged
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jun 23, 2021

cherry pick to release-5.0 in PR #2239

@ti-srebot ti-srebot mentioned this pull request Jun 23, 2021
Merged
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jun 23, 2021

cherry pick to release-5.1 in PR #2240

This was referenced Jun 23, 2021
Closed
Merged
Merged
@JaySon-Huang JaySon-Huang mentioned this pull request Jun 23, 2021
Merged
JaySon-Huang added a commit that referenced this pull request Jun 23, 2021
@ti-srebot @JaySon-Huang
Add time and thread_id for snapshot to check stale snapshots (#2229) (#…
6eab68e 
…2239)

* cherry pick #2229 to release-5.0
* Add snapshot metrics for compact delta

Co-authored-by: JaySon-Huang <jayson.hjs@gmail.com>
ti-srebot added a commit that referenced this pull request Jun 28, 2021
@ti-srebot
Add time and thread_id for snapshot to check stale snapshots (#2229) (#…
64120d2 
…2240)
LittleFall pushed a commit that referenced this pull request Jun 28, 2021
@ti-srebot @LittleFall
Add time and thread_id for snapshot to check stale snapshots (#2229) (#…
839e6be 
…2240)
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Jun 29, 2021
@JaySon-Huang
Revert "Add time and thread_id for snapshot to check stale snapshots (p…
5a4db79 
…ingcap#2229) (pingcap#2239)"

This reverts commit 6eab68e.
This was referenced Jun 30, 2021
Merged
Merged
This was referenced Jun 30, 2021
Merged
Merged
@JaySon-Huang JaySon-Huang mentioned this pull request Jul 19, 2021
Merged
@JaySon-Huang JaySon-Huang mentioned this pull request Aug 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flowbehappy flowbehappy flowbehappy approved these changes

Assignees

@JaySon-Huang JaySon-Huang

Labels

needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 needs-cherry-pick-release-5.1 PR which needs to be cherry-picked to release-5.1 status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JaySon-Huang @ti-srebot @flowbehappy