Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support arena.i.purge and malloc_stats_print when jemalloc is used #402

Merged
merged 10 commits into from
Dec 11, 2024
Merged

Support arena.i.purge and malloc_stats_print when jemalloc is used #402

merged 10 commits into from
Dec 11, 2024

Conversation

CalvinNeo
Copy link
Member

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

What is changed and how it works?

Issue Number: Close #xxx

What's Changed:
  • Provide an http interface "/debug/pprof/arena_purge" for purging the dirty/muzzy page in jemalloc. This could help reduce the memory consumption after large txn is committed.
    Reference: https://jemalloc.net/jemalloc.3.html#opt.muzzy_decay_ms
  • Provide an http interface "/debug/pprof/memory_status" to get memory allocation information is using jemalloc. Note that the return result may take dozens MiB.

image

image

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

@CalvinNeo
purge
0cbef62 
Signed-off-by: Calvin Neo <[email protected]>
@ti-chi-bot ti-chi-bot bot added the size/L label Dec 7, 2024
@CalvinNeo
add memory status
2dcda8a 
Signed-off-by: Calvin Neo <[email protected]>
@CalvinNeo
make compiler happy
561478d 
Signed-off-by: Calvin Neo <[email protected]>
@JaySon-Huang
Copy link
Collaborator

Is this PR ready to be merged? Seem the test still failed

@CalvinNeo
Copy link
Member Author

Is this PR ready to be merged? Seem the test still failed

No, I added another interface to get memory status from jemalloc in the meantime, I am still testing

@CalvinNeo
format
e004e93 
Signed-off-by: Calvin Neo <[email protected]>
@CalvinNeo
clippy
833906c 
Signed-off-by: Calvin Neo <[email protected]>
@CalvinNeo
Copy link
Member Author

/retest

1 similar comment
@CalvinNeo
Copy link
Member Author

/retest

@CalvinNeo
Copy link
Member Author

@JaySon-Huang It is now OK for merge

@CalvinNeo CalvinNeo mentioned this pull request Dec 11, 2024
Open
12 tasks
JaySon-Huang
JaySon-Huang reviewed Dec 11, 2024
View reviewed changes
proxy_components/proxy_ffi/src/jemalloc_utils.rs Outdated
malloc_stats_print(
Some(write_to_string),
&context as *const _ as *mut c_void,
std::ptr::null(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

The malloc_stats_print() function writes summary statistics .... The statistics are presented in human-readable form unless “J” is specified as a character within the opts string, in which case the statistics are presented in JSON format.

Maybe passing "J" instead of std::ptr::null() is a more suitable format of a http API response?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I will fix that

@CalvinNeo
f
a1f16b8 
Signed-off-by: Calvin Neo <[email protected]>
JaySon-Huang
JaySon-Huang approved these changes Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@JaySon-Huang JaySon-Huang 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-chi-bot ti-chi-bot bot added the lgtm label Dec 11, 2024
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 11, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-12-11 06:59:48.82025592 +0000 UTC m=+421778.909058464: ☑️ agreed by JaySon-Huang.

@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JaySon-Huang

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

The pull request process is described here

Needs approval from an approver in each of these files:

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 ti-chi-bot bot added the approved label Dec 11, 2024
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 11, 2024

@CalvinNeo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-unit-test a1f16b8 link true /test pull-unit-test

Full PR test history. Your PR dashboard.

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 kubernetes/test-infra repository. I understand the commands that are listed here.

@CalvinNeo CalvinNeo merged commit 50b8d2e into pingcap:raftstore-proxy Dec 11, 2024
2 of 5 checks passed
@CalvinNeo CalvinNeo changed the title Support arena.i.purge when jemalloc is used Support arena.i.purge and malloc_stats_print when jemalloc is used Dec 11, 2024
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

Assignees
No one assigned
Labels
approved lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CalvinNeo @JaySon-Huang