-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
puller/sorter: remove manual GC settings #2214
puller/sorter: remove manual GC settings #2214
Conversation
Signed-off-by: Neil Shen <overvenus@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trace result from #2202 shows the GC pause time reduces from 25s to 7s after turning off gc percent adjust. However 7s
is still quite a long GC pause time. Is there any more PRs to address this problem.
7s pause is likely caused by memory leak with new ower enabled, here is the issue https://github.com/pingcap/ticdc/issues/2212 |
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
@ben1009: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments. In response to this: 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 ti-community-infra/tichi repository. |
/hold |
This pr will be evaluated by more tests |
I am going to merge this PR. Tests (https://github.com/pingcap/ticdc/pull/2699#issue-724007913) show without changing default GC setting, TiCDC can handle 140G incremental data (no OOM). |
/merge |
@overvenus: In response to this:
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 ti-community-infra/tichi repository. |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 610833b
|
/run-kafka-tests |
Codecov Report
@@ Coverage Diff @@
## master #2214 +/- ##
================================================
+ Coverage 55.8716% 61.0962% +5.2245%
================================================
Files 169 161 -8
Lines 20667 17916 -2751
================================================
- Hits 11547 10946 -601
+ Misses 8012 5975 -2037
+ Partials 1108 995 -113 |
In response to a cherrypick label: new pull request created: #2718. |
In response to a cherrypick label: new pull request created: #2719. |
In response to a cherrypick label: new pull request created: #2720. |
In response to a cherrypick label: new pull request created: #2721. |
What problem does this PR solve?
Do not change default GC settings, it could reduce the minimum mutator utilization and lead to #2202.
Close #2202
Check List
Tests
Capturing 6k tables.
Related changes
Release note