raftstore:Fix append log entry oom by tier-cap · Pull Request #12247 · tikv/tikv

tier-cap · 2022-03-23T10:14:01Z

What is changed and how it works?

Issue Number: Close #11598
Close #11379
Close #12107
What's Changed:

when memory usage is high water, add flow control on the append log entries.
disable log entry cache evict functions.

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
Breaking backward compatibility

Release note

support append log flow control when memory usage high water to avoid oom.

Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

ti-chi-bot · 2022-03-23T10:14:02Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

5kbpers
innerr

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.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

5kbpers · 2022-03-24T09:04:29Z

Could you open an issue and illustrate the problem this PR solved and the effect of this PR?

innerr · 2022-03-24T09:24:25Z

src/server/service/kv.rs

Maybe warn is better?

Changed to debug based on discussion comments with @5kbpers
Compared to metrics monitoring, the log does not have any other useful information.

innerr · 2022-03-24T09:38:50Z

components/raftstore/src/store/memory.rs

Looks weird...

close tikv#11598 close tikv#11379 Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

innerr

LGTM

tier-cap · 2022-03-29T05:10:40Z

Could you open an issue and illustrate the problem this PR solved and the effect of this PR?

Can refer to #11598

5kbpers

rest LGTM

5kbpers · 2022-03-29T08:06:58Z

src/server/service/kv.rs

Will this log be too verbose when memory usage is high?

And better to add a metric for the size of rejected messages

Yes, we have a raft message reject metrics in handle_raft_message.

If reject once, the leader will not send any log append to this peer until next heartbeat response received. The rate of heartbeat is 1s as default. So warning log may be not too much.

Changed to debug based on discussion comments

close tikv#12107 Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

5kbpers · 2022-03-30T03:51:58Z

/merge

ti-chi-bot · 2022-03-30T03:51:58Z

@5kbpers: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

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 · 2022-03-30T03:52:00Z

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

Details

Commit hash: 0c21919

ti-chi-bot · 2022-03-30T03:52:13Z

@tier-cap: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

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.

close tikv#11379, close tikv#11598, close tikv#12107 when memory usage is high water, add flow control on the append log entries. disable log entry cache evict functions. Signed-off-by: tier-cap <zhengxiaojin@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

close tikv#11379, close tikv#11598, close tikv#12107 when memory usage is high water, add flow control on the append log entries. disable log entry cache evict functions. Signed-off-by: tier-cap <zhengxiaojin@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

tier-cap and others added 12 commits January 10, 2022 15:47

verify the meta lock acquire blocked when destroy peer takes much time.

129eab7

Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

Merge branch 'tikv:master' into master

82251f8

Merge branch 'master' of github.com:tikv/tikv

d1edc0c

Merge branch 'master' of github.com:tikv/tikv

afeca8d

Merge branch 'master' of github.com:tikv/tikv

e65fe51

Merge branch 'master' of github.com:tikv/tikv

03cbb48

Merge branch 'master' of github.com:tikv/tikv

75a92f6

Merge branch 'master' of github.com:tikv/tikv

2c74cf3

Merge branch 'master' of github.com:tikv/tikv

1129292

remove the destroy peer test code

0339933

Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

Merge branch 'master' of github.com:tikv/tikv

87f611d

fix append log entry oom

490a12a

Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

tier-cap requested review from 5kbpers and NingLin-P March 24, 2022 08:53

innerr reviewed Mar 24, 2022

View reviewed changes

components/raftstore/src/store/memory.rs Outdated

Copy link

Contributor

innerr Mar 24, 2022

Choose a reason for hiding this comment

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

Looks weird...

add metrics on dropped log append messages

b05d70a

close tikv#11598 close tikv#11379 Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

tier-cap force-pushed the fix-append-log-entry-oom branch 3 times, most recently from 659723e to c255cd4 Compare March 24, 2022 13:44

innerr approved these changes Mar 25, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 25, 2022

5kbpers reviewed Mar 29, 2022

View reviewed changes

change log level when drop messages

0c21919

close tikv#12107 Signed-off-by: tier-cap <zhengxiaojin@pingcap.com>

tier-cap force-pushed the fix-append-log-entry-oom branch from c255cd4 to 0c21919 Compare March 30, 2022 03:49

5kbpers approved these changes Mar 30, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 30, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 30, 2022

Merge branch 'master' into fix-append-log-entry-oom

692b8ba

ti-chi-bot merged commit 6c67b11 into tikv:master Mar 30, 2022

CalvinNeo mentioned this pull request May 16, 2022

update proxy pingcap/tiflash#4894

Merged

12 tasks

mayjiang0203 mentioned this pull request Jun 14, 2022

v5.2.2: 2 tikvs oom and can't start after 1 hours #11581

Closed

CalvinNeo mentioned this pull request Aug 18, 2022

TiFlash Proxy may OOM/bcast too many messages/drop apply res pingcap/tiflash#5646

Closed

flowbehappy mentioned this pull request Sep 27, 2022

Reduce the memory usage for huge transactions pingcap/tiflash#4728

Open

2 tasks

Conversation

tier-cap commented Mar 23, 2022 • 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!

ti-chi-bot commented Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

5kbpers commented Mar 24, 2022

Uh oh!

innerr Mar 24, 2022

Choose a reason for hiding this comment

Uh oh!

tier-cap Mar 24, 2022

Choose a reason for hiding this comment

Uh oh!

tier-cap Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

innerr Mar 24, 2022

Choose a reason for hiding this comment

Uh oh!

innerr left a comment

Choose a reason for hiding this comment

Uh oh!

tier-cap commented Mar 29, 2022

Uh oh!

5kbpers left a comment

Choose a reason for hiding this comment

Uh oh!

5kbpers Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

5kbpers Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tier-cap Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tier-cap Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

5kbpers commented Mar 30, 2022

Uh oh!

ti-chi-bot commented Mar 30, 2022

Uh oh!

ti-chi-bot commented Mar 30, 2022

Uh oh!

ti-chi-bot commented Mar 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tier-cap commented Mar 23, 2022 •

edited

Loading

ti-chi-bot commented Mar 23, 2022 •

edited

Loading

tier-cap Mar 30, 2022 •

edited

Loading

5kbpers Mar 29, 2022 •

edited

Loading

tier-cap Mar 29, 2022 •

edited

Loading