cdc: do not send too much tasks if cannot be handled in time by 3AceShowHand · Pull Request #17724 · tikv/tikv

3AceShowHand · 2024-10-28T15:23:55Z

What is changed and how it works?

Issue Number: Close #17696

What's Changed:

* take cdc tasks into memory quota to prevent the TiKV OOM caused by too many pending tasks

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

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

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

fix the TiKV may oom if too much internal pending tasks due to read old value too slow

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

components/cdc/src/endpoint.rs

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

overvenus

Rest LGTM

components/cdc/src/endpoint.rs

components/txn_types/src/types.rs

overvenus · 2024-11-05T04:29:48Z

components/cdc/src/observer.rs

        };
+
+        let size = cmd_batches.iter().map(|b| b.size()).sum();
+        self.memory_quota.alloc_force(size);


Please add a test that ensures that too many pending CmdBatch causes TxnExtra to be dropped due to memory quota.

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Signed-off-by: Neil Shen <overvenus@gmail.com>

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

ti-chi-bot · 2024-11-06T06:05:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hicqu, overvenus

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [hicqu,overvenus]

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-06T06:05:50Z

[LGTM Timeline notifier]

Timeline:

2024-11-06 05:51:16.660780405 +0000 UTC m=+1020189.499935943: ☑️ agreed by overvenus.
2024-11-06 06:05:49.668532044 +0000 UTC m=+1021062.507687592: ☑️ agreed by hicqu.

3AceShowHand · 2024-11-06T06:47:08Z

/cherry-pick release-8.4

ti-chi-bot · 2024-11-06T06:47:47Z

@3AceShowHand: new pull request created to branch release-8.4: #17777.

Details

In response to this:

/cherry-pick release-8.4

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.

ti-chi-bot · 2024-11-06T06:54:10Z

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

ti-chi-bot · 2024-11-06T06:54:10Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against tikv/tikv#release-8.4 from head ti-chi-bot:cherry-pick-17724-to-release-8.4: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-17724-to-release-8.4."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

…#17777) close #17696 * take cdc tasks into memory quota to prevent the TiKV OOM caused by too many pending tasks Signed-off-by: 3AceShowHand <jinl1037@hotmail.com> Signed-off-by: Neil Shen <overvenus@gmail.com> Co-authored-by: 3AceShowHand <jinl1037@hotmail.com> Co-authored-by: Neil Shen <overvenus@gmail.com>

…#17778) close #17696 * take cdc tasks into memory quota to prevent the TiKV OOM caused by too many pending tasks Signed-off-by: Neil Shen <overvenus@gmail.com> Signed-off-by: 3AceShowHand <jinl1037@hotmail.com> Co-authored-by: Neil Shen <overvenus@gmail.com> Co-authored-by: 3AceShowHand <jinl1037@hotmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

3AceShowHand added 6 commits October 28, 2024 17:04

alloca memory for the observer on flush cmd

60a87ac

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

alloca memory for the observer on flush cmd

e63972c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

sleep 100ms to mock scan take too long

3c2d9cb

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

also track txn extra

34f9975

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add metrics to track each task

fc7c4b0

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add metrics to track each task

6a7118a

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

cancel multi batch tasks

3dc5ecc

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

overvenus reviewed Oct 29, 2024

View reviewed changes

components/cdc/src/endpoint.rs Show resolved Hide resolved

3AceShowHand added 8 commits October 29, 2024 12:05

alloc memory by force

c1a95ed

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

alloc memory by force

14e58b1

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

alloc memory by force

3a0f358

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

server.rs fix memory quota

1fabac3

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

remove debug logs

7effe5a

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

alloc memory quota for txn extra no force

fff3d5c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

ready for test

1cc6257

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

remove count metrics.

2bd3c8c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

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

make format

ae04669

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

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

3AceShowHand changed the title ~~[DNM] cdc: do not send too much tasks if cannot be handled in time~~ cdc: do not send too much tasks if cannot be handled in time Nov 5, 2024

3AceShowHand requested a review from overvenus November 5, 2024 03:41

make format

cb1ea15

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

overvenus reviewed Nov 5, 2024

View reviewed changes

record error metrics if drop txn extra

e59c60c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Apply suggestions from code review

5bb6d28

Signed-off-by: Neil Shen <overvenus@gmail.com>

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

3AceShowHand added 2 commits November 6, 2024 11:42

fix unit test

df83c64

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

fix make format

d7d18e9

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand requested a review from overvenus November 6, 2024 05:45

overvenus approved these changes Nov 6, 2024

View reviewed changes

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

hicqu approved these changes Nov 6, 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 6, 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 do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 6, 2024

Merge branch 'master' into cdc-quota-task-channel

29ad772

ti-chi-bot bot merged commit bc2c652 into tikv:master Nov 6, 2024

ti-chi-bot bot added this to the Pool milestone Nov 6, 2024

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

cdc: do not send too much tasks if cannot be handled in time (#17724) #17777

Merged

9 tasks

overvenus added needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-cherry-pick-release-8.4 Should cherry pick this PR to release-8.4 branch. labels Nov 6, 2024

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

cdc: do not send too much tasks if cannot be handled in time (#17724) #17778

Merged

9 tasks

This was referenced Nov 7, 2024

CDC lag up to 2.5h when titan is on and region size is large (280MB+) #17781

Closed

TiDB latency not stable and TiKV OOM if running cdc changefeed when titan is on and average region size is large #17696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cdc: do not send too much tasks if cannot be handled in time#17724

cdc: do not send too much tasks if cannot be handled in time#17724
ti-chi-bot[bot] merged 27 commits intotikv:masterfrom
3AceShowHand:cdc-quota-task-channel

3AceShowHand commented Oct 28, 2024 •

edited

Loading

Uh oh!

Uh oh!

overvenus left a comment

Uh oh!

Uh oh!

Uh oh!

overvenus Nov 5, 2024

Uh oh!

ti-chi-bot bot commented Nov 6, 2024

Uh oh!

ti-chi-bot bot commented Nov 6, 2024

Uh oh!

3AceShowHand commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

3AceShowHand commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

Uh oh!

overvenus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

overvenus Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Nov 6, 2024

Uh oh!

ti-chi-bot bot commented Nov 6, 2024

[LGTM Timeline notifier]

Uh oh!

3AceShowHand commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

ti-chi-bot commented Nov 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

3AceShowHand commented Oct 28, 2024 •

edited

Loading