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.

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

init: add MALLOC_ARENA_MAX=1 to systemd #27642

Closed
wants to merge 1 commit into from

Conversation

sangaman
Copy link
Contributor

This adds the MALLOC_ARENA_MAX=1 environment variable as suggested in https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific to the systemd service file definition.

Without this env var, memory usage can grow significantly especially when rpcthreads is increased above its default value.

Closes #24542.

I have tested this change myself with positive results after dealing with memory consumption issues for a long time using systemd on a 8GB RAM raspi4. I figure a similar change may be desirable for the OpenRC and CentOS init files, but I don't have a way to test them and I'm not even exactly sure how environment variables should be added there (via export statements?).

This adds the `MALLOC_ARENA_MAX=1` environment variable as suggested in
https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific
to the systemd service file definition.

Without this env var, memory usage can grow significantly especially when
`rpcthreads` is increased above its default value.

Closes bitcoin#24542.
@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@willcl-ark
Copy link
Member

I don't believe this affects the majority of users (where the two pools per core default works fine), so I don't see why we'd update the default service file like this.

We have documentation that describes what to do to reduce memory in the event that one wants to do that?

@sangaman
Copy link
Contributor Author

I don't believe this affects the majority of users (where the two pools per core default works fine), so I don't see why we'd update the default service file like this.

We have documentation that describes what to do to reduce memory in the event that one wants to do that?

Maybe it's my fault for not digging into the docs more but I went over a year with inflated, unpredictable RAM usage and even occasional OOM crashes on a machine that I wouldn't consider particularly memory-constrained. It looks like the reason I was seeing it was due to a higher rpcthreads setting, but I'm sure I'm not the only one that has that sort of config and they may be dealing with the same issues.

My thinking is that if in fact the performance impact is "small or absent" in bitcoind then this is a sensible default for linux machines.

@Sjors
Copy link
Member

Sjors commented May 12, 2023

From the doc:

However, in Bitcoin Core very little parallel allocation happens, so the impact is expected to be small or absent.

The "expected to be" could use some benchmarks before making it a default. Does the problem only happen with high rpcthreads or equally with other (potentially more common) settings like a high -dbcache or -maxmempool?

@sangaman
Copy link
Contributor Author

Does the problem only happen with high rpcthreads or equally with other (potentially more common) settings like a high -dbcache or -maxmempool?

From my understanding/reading of MALLOC_ARENA_MAX it affects memory usage where multiple threads are concerned. rpcthreads is the only configuration option afaict that impacts the number of threads running. Increasing dbcache or maxmempool wouldn't create new threads, only increase a set limit of RAM usage, so I don't think those alone would trigger the behavior I was seeing.

@sangaman
Copy link
Contributor Author

More relevant discussion here: #6876 (comment)

@Sjors
Copy link
Member

Sjors commented May 12, 2023

@sangaman maybe -par as well? It defaults to the number of cores - 1 and is capped at 15.

@martinus
Copy link
Contributor

Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for CCoinsMap. (by about a factor of 2000).

@sangaman
Copy link
Contributor Author

Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for CCoinsMap. (by about a factor of 2000).

Thanks! I'll try that branch out and see if resolves the underlying issue without this env var, in which case I'd agree this PR is probably not necessary.

@fanquake
Copy link
Member

Note that internally we already set M_ARENA_MAX to 1, but that is currently restricted to 32-bit systems. I would assume everyone reporting issues (#24542) is using on 64-bit. We may be at the point where we could just drop that code entirely.

@sangaman
Copy link
Contributor Author

I'm 64 bit indeed, but wouldn't the same concern be present on 32 bit systems?

I just restarted my node with the changes from #25325 and without the MALLOC_ARENA_MAX setting, I'll give it a few days and report back on how the memory usage is going.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 8, 2023

There hasn't been much activity lately. What is the status here?

Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@sipa
Copy link
Member

sipa commented Sep 20, 2023

Just because it's desirable for the purpose of running on low-memory systems does not mean it should be a default for all systemd users of bitcoind? If it's useful always, it should probably be recommended outside a doc about reducing memory usage. Alternatively, it sounds like maybe we need to document that increasing rpcthreads may increase memory usage.

@real-or-random
Copy link
Contributor

I just restarted my node with the changes from #25325 and without the MALLOC_ARENA_MAX setting, I'll give it a few days and report back on how the memory usage is going.

Any insights here? Has the issue been resolved for you?

Note that https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html says:

The default value of this tunable is 0, meaning that the limit on the number of arenas is determined by the number of CPU cores online. For 32-bit systems the limit is twice the number of cores online and on 64-bit systems, it is 8 times the number of cores online.

This means what we say in https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific is wrong (i.e., only true for 32 bits):

By default, glibc will create up to two heap arenas per core.

real-or-random added a commit to real-or-random/bitcoin that referenced this pull request Sep 20, 2023
Before this commit, we claim that glibc's malloc implementation uses 2
arenas by default. But that's true only on 32-bit systems, and even
there, it uses *up* to 2 arenas.

This commit fixes the wrong statement. The new statement is
intentionally vague to reduce our maintenance burden.

For details, see:
https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html#index-glibc_002emalloc_002earena_005fmax

Noticed in:
bitcoin#27642 (comment)
@achow101
Copy link
Member

This PR does not seem to have conceptual support. Please leave a comment if you would like this to be reopened.

@achow101 achow101 closed this Sep 20, 2023
fanquake added a commit to bitcoin-core/gui that referenced this pull request Sep 24, 2023
…LOC_ARENA_MAX

12f7257 doc: Be vague instead of wrong about MALLOC_ARENA_MAX (Tim Ruffing)

Pull request description:

  Before this commit, we claim that glibc's malloc implementation uses 2 arenas by default. But that's true only on 32-bit systems, and even there, it uses *up* to 2 arenas.

  This commit fixes the wrong statement. The new statement is intentionally vague to reduce our maintenance burden.

  For details, see:
  https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html#index-glibc_002emalloc_002earena_005fmax

  Noticed in:
  bitcoin/bitcoin#27642 (comment)

ACKs for top commit:
  fanquake:
    ACK 12f7257

Tree-SHA512: c0ff1e35b682a841e366a1cad26e18ff79a93d97103529be35a972c7dcbb95f5354e7a7b98a86731f491434d64685bb58cc3cc9100f0577d8f75db05e951b09a
Retropex pushed a commit to Retropex/bitcoin that referenced this pull request Oct 4, 2023
Before this commit, we claim that glibc's malloc implementation uses 2
arenas by default. But that's true only on 32-bit systems, and even
there, it uses *up* to 2 arenas.

This commit fixes the wrong statement. The new statement is
intentionally vague to reduce our maintenance burden.

For details, see:
https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html#index-glibc_002emalloc_002earena_005fmax

Noticed in:
bitcoin#27642 (comment)
janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Apr 1, 2024
Before this commit, we claim that glibc's malloc implementation uses 2
arenas by default. But that's true only on 32-bit systems, and even
there, it uses *up* to 2 arenas.

This commit fixes the wrong statement. The new statement is
intentionally vague to reduce our maintenance burden.

For details, see:
https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html#index-glibc_002emalloc_002earena_005fmax

Noticed in:
bitcoin/bitcoin#27642 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow memory leak in v22.0?
9 participants