Skip to content

Set proxy's memory limit by TiFlash#9753

Merged
ti-chi-bot[bot] merged 12 commits intopingcap:masterfrom
CalvinNeo:set-memory-tiflash
Jan 3, 2025
Merged

Set proxy's memory limit by TiFlash#9753
ti-chi-bot[bot] merged 12 commits intopingcap:masterfrom
CalvinNeo:set-memory-tiflash

Conversation

@CalvinNeo
Copy link
Member

@CalvinNeo CalvinNeo commented Dec 31, 2024 •
edited by JaySon-Huang
Loading

What problem does this PR solve?

Issue Number: close #9745

Problem Summary:

After this, the memory_usage_limit on proxy's side will be:

  • If memory_usage_limit is set on proxy's side, use it
  • Otherwise:
    • If max_memory_usage_for_all_queries is not zero, use its size or ratio and compute on proxy's side.
    • Use default value MEMORY_USAGE_LIMIT_RATE

What is changed and how it works?

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

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Fix an issue that TiFlash may unexpectedly reject raft message when memory usage is not high

@CalvinNeo
initial config
127a124 
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
@CalvinNeo
upd proxy
b77eb37 
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Dec 31, 2024
CalvinNeo and others added 2 commits December 31, 2024 13:51
@CalvinNeo
upd proxy
a4f0f73 
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
@CalvinNeo
Merge branch 'master' into set-memory-tiflash
b18f180
@CalvinNeo
format
9e8b337 
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
@CalvinNeo
Merge branch 'set-memory-tiflash' of github.com:CalvinNeo/tics into s…
b1195e5 
…et-memory-tiflash
JinheLin
JinheLin reviewed Dec 31, 2024
View reviewed changes
@CalvinNeo @JinheLin
Update dbms/src/Server/Server.cpp
012d5bb 
Co-authored-by: jinhelin <linjinhe33@gmail.com>
@CalvinNeo
Copy link
Member Author

CalvinNeo commented Dec 31, 2024

/hold wait proxy

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 31, 2024
@ti-chi-bot ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 31, 2024
@CalvinNeo
Copy link
Member Author

CalvinNeo commented Jan 2, 2025

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 2, 2025
@CalvinNeo
Copy link
Member Author

CalvinNeo commented Jan 2, 2025

/retest

@CalvinNeo
metrics
dfd8b93 
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 2, 2025
@CalvinNeo
Copy link
Member Author

CalvinNeo commented Jan 2, 2025 •
edited
Loading

The metrics:

Log Replication Rejected shows the rate that TiFlash rejects msgAppend. We can see it could work when memory reaches high water.

image

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 3, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 3, 2025

[LGTM Timeline notifier]

Timeline:

  • 2024-12-31 10:02:33.426425084 +0000 UTC m=+431088.782429619: ☑️ agreed by JinheLin.
  • 2025-01-03 03:13:39.505568415 +0000 UTC m=+665754.861572955: ☑️ agreed by JaySon-Huang.

@ti-chi-bot ti-chi-bot bot merged commit 2549b07 into pingcap:master Jan 3, 2025
@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jan 8, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 9, 2025

In response to a cherrypick label: failed to apply #9753 on top of branch "release-8.5":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

JaySon-Huang added a commit to CalvinNeo/tiflash that referenced this pull request Jan 9, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (pingcap#9753)
a20e4b7 
close pingcap#9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
ti-chi-bot bot pushed a commit that referenced this pull request Jan 9, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (#9753) and update tiflash proxy (#…
9ff0d60 
…9776)

ref #4982, close #9745

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
@CalvinNeo
Copy link
Member Author

CalvinNeo commented Jan 9, 2025

In response to a cherrypick label: failed to apply #9753 on top of branch "release-8.5":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

Manually cherry-picked to release-8.5 by #9776

@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Jan 21, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 21, 2025

In response to a cherrypick label: failed to apply #9753 on top of branch "release-6.5":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Feb 6, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 6, 2025 •
edited by CalvinNeo
Loading

In response to a cherrypick label: failed to apply #9753 on top of branch "release-8.1":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

By #9841

ti-chi-bot bot pushed a commit that referenced this pull request Feb 6, 2025
@CalvinNeo
Set proxy's memory limit by TiFlash (#9753) and update tiflash proxy (#…
bfccc6f 
…9841)

ref #4982, close #9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
CalvinNeo added a commit to CalvinNeo/tiflash that referenced this pull request Feb 8, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (pingcap#9753)
0918afd 
close pingcap#9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 10, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 10, 2025

In response to a cherrypick label: failed to apply #9753 on top of branch "release-7.5":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

ti-chi-bot bot pushed a commit that referenced this pull request Feb 10, 2025
@CalvinNeo
Set proxy's memory limit by TiFlash (#9753) and update tiflash proxy (#…
7d751d6 
…9853)

ref #4982, close #9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
CalvinNeo added a commit to CalvinNeo/tiflash that referenced this pull request Feb 13, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (pingcap#9753)
508429e 
close pingcap#9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Feb 13, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 13, 2025

In response to a cherrypick label: failed to apply #9753 on top of branch "release-7.1":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]

ti-chi-bot bot pushed a commit that referenced this pull request Feb 14, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (#9753) and update tiflash proxy (#…
e191f36 
…9873)

ref #4982, close #9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
CalvinNeo added a commit to CalvinNeo/tiflash that referenced this pull request Feb 17, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (pingcap#9753)
e31fa88 
close pingcap#9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon <tshent@qq.com>
CalvinNeo added a commit to CalvinNeo/tiflash that referenced this pull request Feb 17, 2025
@CalvinNeo
Revert "Set proxy's memory limit by TiFlash (pingcap#9753)"
30f25a5 
This reverts commit e31fa88.
@JaySon-Huang JaySon-Huang mentioned this pull request Feb 17, 2025
Closed
12 tasks
@ti-chi-bot ti-chi-bot bot removed the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Feb 18, 2025
@JaySon-Huang JaySon-Huang mentioned this pull request Feb 18, 2025
Merged
12 tasks
@ti-chi-bot ti-chi-bot mentioned this pull request Feb 19, 2025
Merged
12 tasks
JaySon-Huang pushed a commit to JaySon-Huang/tiflash that referenced this pull request Apr 25, 2025
@CalvinNeo @JaySon-Huang
Set proxy's memory limit by TiFlash (pingcap#9753) and update tiflash…
78a1256 
… proxy (pingcap#9841)

ref pingcap#4982, close pingcap#9745

Signed-off-by: Calvin Neo <calvinneo1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JaySon-Huang JaySon-Huang JaySon-Huang approved these changes

@JinheLin JinheLin JinheLin approved these changes

@Lloyd-Pottiger Lloyd-Pottiger Awaiting requested review from Lloyd-Pottiger

Assignees

No one assigned

Labels

approved lgtm needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

memory_usage_limit is deduced much less then expected

4 participants

@CalvinNeo @ti-chi-bot @JaySon-Huang @JinheLin