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

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #34641 (node: scale default -dbcache with system RAM by l0rinc)
• #34435 (refactor: use _MiB/_GiB consistently for byte conversions by l0rinc)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

🚧 At least one of the CI tasks failed.
_{Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/22490076628/job/65149651058
LLM reason (✨ experimental): C++ compile error: constexpr evaluation fails due to throwing in a constexpr context (MiB value too large for size_t).}

Concept ACK 6b0210e

I agree with Ajtowns, I think it makes more sense to have a simple bump in cache size instead of auto scaling.

We might want to add a release note since this will affect any nodes that currently do not have -dbcache set running on systems with greater than 4 GB RAM

🚧 At least one of the CI tasks failed.
_{Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/22523590140/job/65252087815
LLM reason (✨ experimental): Build failed during the Qt target (src/qt/optionsmodel.cpp) with a non-zero exit status from the cmake/make step.}

Thanks @ajtowns I took all your suggestions.
Thanks @kevkevinpal added release notes.

ACK 4ae9a10

Checked it gives 450 on a system with less than 4GB and 1024 on a system with more.

If you want to avoid changing kernel behavior in this PR, it would be good to state so in the PR-description. "This change only changes bitcoind behavior, while kernel still defaults to 450 MiB."

Please also use the correct "GiB" unit in PR title. (Edit: and correct all units in the PR description).

Concept ~0 because i think it's fine if this PR is merged as is, but i reject the premise that we should be supporting systems with very low memory through the default configuration.

I think the goal of supporting systems with low resources is better achieved with configuration presets (as was suggested in the meeting where this issue was discussed) than with complicated and surprising system resource detection baked into our code.

Put this on the milestone, to gather any more opinons/make a decision for v31.

Concept ACK

Concept ACK - Prefer incremental improvement

weigh the argument for making this something that's configurable?

@yancyribbens do you mean making -dbcache configurable? If so it is already a configuration option. This PR and discussion is about changing the default value for it.

@yancyribbens do you mean making -dbcache configurable? If so it is already a configuration option. This PR and discussion is about changing the default value for it.

Oh, great! Then, I think if this breaks the default compatibility for some users, then they would simply need to adjust their configuration..

@yancyribbens It really helps to read PRs before commenting on them.

@yancyribbens It really helps to read PRs before commenting on them.

Leave it up to @sipa to put me in my place, again. Fair point.

reACK 4ae9a10

This looks good to me, I think it makes sense to continue to support low RAM systems and not do a simple bump.

This is good to get into the v31.0 release without being too complicated and revisit the autoscaling RAM in a later release if desired.

changes from last ack:
- Release notes added
- ajtowns suggestions

Concept ~0 because i think it's fine if this PR is merged as is, but i reject the premise that we should be supporting systems with very low memory through the default configuration.

Been thinking about this some more. I still think it's preferable this PR be merged as-is for 31.0, but a different approach could be:

• choose defaults that give respectable performance on easy to obtain hardware; eg 1GB dbcache on an 8GB RAM machine
• on startup, if the configuration settings have been left as defaults, check if they're obviously unreasonable (dbcache is >25% of memory, prune is unset on mainnet and there's only 20GB of diskspace, eg) and provide a startup error if so.
• you can avoid the startup error by explicitly configuring the settings, even just setting to the same values as the defaults. ie saying "I know what I'm doing, shut up about it"

That lets us set the defaults to a single value that achieve reasonable performance on reasonable hardware, and leaves achieving great performance or running on unreasonable hardware with poor performance up to people to configure.

I feel like simple good-enough defaults and flexible configuration options is a better approach than trying to make the defaults automatically optimise themselves for the hardware, a la #34641. Having a check and an early error when we can detect that the good-enough defaults are probably not appropriate for the hardware would be a win on top of that, I think, while also not making things much more complex. For people who specifically want automatic optimisation of their config, then a third party tool (lopp's bitcoin.conf generator?) or AI consultation would let them achieve that.

One edge case to consider is how GetTotalRAM() determines available memory. On Unix systems it uses sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGE_SIZE), which reports the host’s physical RAM, not the memory limit of the container.

For example, if a Docker container is started with --memory=1g on a host with 32 GB of RAM, this call will still return about 32 GB. As a result, the 4 GiB check passes and dbcache may be set to 1024 MiB, which can cause the container to run out of memory.

However, the container’s memory limit is exposed through cgroups. In this setup, /sys/fs/cgroup/memory.max correctly reports the container limit (for example, 1 GiB). Taking the minimum between GetTotalRAM() and the cgroup memory limit, when the latter is available, would address this issue for common container environments such as Docker.

ACK 4ae9a10

Reviewed the code, built and ran it. Can attest that it defaults to 1024 without the dbcache set, and still to 450 with dbcache=450.

Since I have no machine with less than 4GB available, tried to patch the GetTotalRAM function to set it to 2GB, but it did not work, since the dynamic linker never resolves it.

It seems I may be too conservative when trying not to break use cases, since most other reviewers don't seem to think it's a big issue. I suppose there are likely very few users running on <4GB that are not already configuring their defaults and reading release notes. The defaults are meant for newbies who aren't yet configuring, which are likely on higher RAM systems.

Nevertheless, this PR is meant to be a last minute sneak in to v31. There are already several ACKs, and those with suggestions explicitly said they do not mind it being merged as is. So, I'm going to leave it as is for now and hopefully a maintainer will merge before cutoff.

on startup, if the configuration settings have been left as defaults, check if they're obviously unreasonable (dbcache is >25% of memory, prune is unset on mainnet and there's only 20GB of diskspace, eg) and provide a startup error if so.

@ajtowns Since #33333 we warn on oversized dbcache. I think it would make sense to tighten that up to an error.

ACK 4ae9a10

So, I'm going to leave it as is for now and hopefully a maintainer will merge before cutoff.

Wouldn't that be surprising to merge the detection if it doesn't work on containers:

As a result, the 4 GiB check passes and dbcache may be set to 1024 MiB, which can cause the container to run out of memory.

which are at least as common a setup as VMs are, which were used as the main argument for the detection in the first place:

Currently we can support running indefinitely on 2GB VMs with default configuration. This is a popular cheap format (https://aws.amazon.com/ec2/instance-types/t2/) for running (not syncing) bitcoind.

(In any case anybody technical enough to spin up a VM or a container is able to set a configuration option, it's not like we are breaking a use case, but hey.)

Edited for clarity since apparently the condescending comment that ensued was genuine.

I opened #34763 as an alternative that only bumps the default without introducing the detection.

which are at least as common a setup, as the one which was used as the main argument for the detection in the first place:

Currently we can support running indefinitely on 2GB VMs with default configuration. This is a popular cheap format (https://aws.amazon.com/ec2/instance-types/t2/) for running (not syncing) bitcoind.

(In any case anybody technical enough to spin up a VM or a container is able to set a configuration option, it's not like we are breaking a use case, but hey.)

A VM is not the same as a container, and will present available memory differently. In the case with AWS VPSes, the machine will appear to have the specified amount of memory and not be affected by this issue.

A VM is not the same as a container

Oh, really?

Oh, really?

This actually goes back to when chroot was introduced in 1979 as a way to provide an isolated filesystem while still using the same host kernel. FreeBSD extended this idea to also include isolation for other devices in the form of Jails, which was sort of co-opted later in the form of linux containers. Containers also brought along a who ecosystem of reproducible install tools eg Docker. I'm still pretty found of jails though for their simplicity over containers, and of course less overhead than VMs. Anyway, there's a pretty interesting detail of this here: https://hypha.pub/back-to-freebsd-part-1.

I feel like simple good-enough defaults and flexible configuration options is a better approach than trying to make the defaults automatically optimise themselves for the hardware, a la #34641.

I argue with that. A large number of bitcoin-core runners will give zero thought about RAM sizes and memory limits, and expect things to work, and if things don't work optimally, they will blame the software. Even if there was concrete guidance offered, like "set dbcache to 20% of the available RAM" (there is no such guidance), most users would not comply. I think context-aware meaningful defaults are very useful. Context-aware estimated min/max values are also needed to be able to generate meaningful warnings in case user configuration is present, but nonsensical. I think context-aware defaults that try to cover a wider range of different systems (like #34641) are better overall. But for sure user configuration should be able to override everything.

• on startup, if the configuration settings have been left as defaults, check if they're obviously unreasonable (...) and provide a startup error if so.

Refusing to start on unreasonable values is an interesting idea. But I see inconsistency here: if we trust the software-decided reasonable-limits so much that it can even refuse to start, why not trust a default to be used? The logic/reliability to derive the optimal default and unreasonable limits are very similar.

To me it looks more consistent and useful to:

• Derive meaningful context-aware default/min/max values, based on the available RAM (and possibly other parameters)
• If no config is given, use the default, and state this in the logs to the user, alongside with all data points used (e.g. detected RAM amount).
• If config is given, use that. Check against min/max values, and if it's outside, warn in the logs (alongside with all data points used e.g. detected RAM amount). But attempt to run anyways.
• React to some symptoms of incorrect settings, e.g. to high swapping, with appropriate diagnostics logs.

I think this gives the best combo for:

• informed defaults for 'non-technical' users (e.g. users of node-runner solutions but on custom hardware)
• control and relevant data points for 'technical' users
• info in case of rare strange setups where the software misbehaves (extremly low or high memory, misdetected memory, etc.).

Post-merge-ACK 4ae9a10

A limited change of the single-value limit to a two-tier limit, benefiting IBD performance on (the majority?) non-low-memory systems, for v31.
For the future a more elaborate calculation of optimal default value and meaningful range, and more informative logging would be beneficial.