coins: warn on oversized `-dbcache`

l0rinc commented at 5:26 pm on September 7, 2025: contributor

Summary

Oversized allocations can cause out-of-memory errors or heavy swapping, grinding the system to a halt.

Fix

Added a minimal system helper to query total physical RAM on Linux/macOS/Windows (on unsupported platforms we just disable this warning completely). The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GB (as a simple property test checking if the unit size is correct, e.g. doesn’t return megabytes).

Details

LogOversizedDbCache() now emits a startup warning if the configured -dbcache exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via CalculateDbCacheBytes(). This isn’t meant as a recommended setting, rather a likely upper limit.

Note that we’re not modifying the set value, just issuing a warning. Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn’t stay inside size_t on 32-bit systems - and this was simpler than casting back and forth.

We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.

Cap

If total RAM < 2 GiB, cap is DEFAULT_DB_CACHE (450 MiB), otherwise it’s 75% of total RAM. The threshold is chosen to be close to values commonly used in raspiblitz for common setups:

Total RAM	`dbcache` (MiB)	raspiblitz %	proposed cap (MiB)
1 GiB	512	50.0%	450*
2 GiB	1536	75.0%	1536
4 GiB	2560	62.5%	3072
8 GiB	4096	50.0%	6144
16 GiB	4096	25.0%	12288
32 GiB	4096	12.5%	24576

Umbrel issues also mention 75% being the upper limit.

Reproducer

Starting bitcoind on an 8 GiB rpi4b with a dbcache of 7 GiB:

./build/bin/bitcoind -dbcache=7000

warns now as follows:

02025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
12025-09-07T17:24:29Z Cache configuration:
22025-09-07T17:24:29Z * Using 2.0 MiB for block index database
32025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
42025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)

The warning should now be available via RPC as wel, run any of these against the running bitcoind:

0./build/bin/bitcoin-cli -rpcwait getblockchaininfo | grep dbcache
1./build/bin/bitcoin-cli -rpcwait getnetworkinfo | grep dbcache

which should show:

“A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.”,

Manual testing

Besides the godbolt reproducers for the new total memory method, we also tested the warnings manually on:

Apple M4 Max, macOS 15.6.1
Intel Core i9-9900K, Ubuntu 24.04.2 LTS
Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351

DrahtBot commented at 5:26 pm on September 7, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33333.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Sjors, TheCharlatan, willcl-ark, stickies-v
Stale ACK	w0xlt, hodlinator, danielabrozzoni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32380 (Modernize use of UTF-8 in Windows code by hebasto)
#29365 (Extend signetchallenge to set target block spacing by starius)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

“A %zu MiB dbcache may be too large for a system memory of only %zu MiB.” -> “A %zu MiB dbcache may be too large for a system with only %zu MiB of memory.” [original phrase “for a system memory of” is ungrammatical/awkward; the replacement is grammatical and preserves meaning]

No other typos were found.

drahtbot_id_5_m

l0rinc force-pushed on Sep 7, 2025

l0rinc force-pushed on Sep 8, 2025

Sjors commented at 6:37 am on September 8, 2025: member

Concept ACK

Tested on my M4 macOS machine that is shows the warning. It gets buried in the log, but I think it’s a good start, and at least it will help people figure out why their node crashed after the fact.

TheCharlatan commented at 8:23 am on September 8, 2025: contributor

Concept ACK

w0xlt commented at 0:13 am on September 10, 2025: contributor

Approach ACK

I’d suggest: . Logging the else condition as well, for consistency. . Showing the percentage of RAM being used.

Example:

 0@@ -1736,9 +1736,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 1     const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size());
 2     if (auto total_ram{GetTotalRAM()}) {
 3         size_t cap{(*total_ram < 2048_MiB) ? DEFAULT_KERNEL_CACHE : (*total_ram / 100) * 75};
 4+        size_t actual_percentage = (kernel_cache_sizes.coins * 100) / *total_ram;
 5         if (kernel_cache_sizes.coins > cap) {
 6-            LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB.", kernel_cache_sizes.coins >> 20, *total_ram >> 20);
 7-        }
 8+            LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB (%zu%% of system memory).",
 9+                    kernel_cache_sizes.coins >> 20, *total_ram >> 20, actual_percentage);
10+        } else {
11+            LogInfo("Total system memory is %zu MiB; setting dbcache to %zu MiB (%zu%% of system memory)",
12+                    *total_ram >> 20, kernel_cache_sizes.coins >> 20, actual_percentage);
13+    }
14     }
15
16     LogInfo("Cache configuration:");

achow101 added this to the milestone 30.0 on Sep 11, 2025

l0rinc renamed this:
~~RFC: coins: warn on oversized `-dbcache`~~
coins: warn on oversized `-dbcache`
on Sep 11, 2025

DrahtBot added the label UTXO Db and Indexes on Sep 11, 2025

l0rinc marked this as ready for review on Sep 11, 2025

willcl-ark commented at 5:49 pm on September 11, 2025: member

Concept ACK

I think warning when someone is likely going to clobber their system performance is useful.

This change introduces a warning on lower limit of free RAM remaining; would it make sense to also include an upper bound to suppress/skip the warning? The cap is set to warn when < 1.5GB is left free in the case RAM is below 2GB, but will then warn when I have 16GB of ram free:

0src/core/bitcoin on  pr-33333 [$] via △ v3.31.6 via 🐍 v3.13.3 via ❄️  impure (nix-shell-env) took 3s
1❯ build/bin/bitcoind -regtest -daemon=0 -dbcache=48140 | rg warning
22025-09-11T17:39:13Z [warning] A 48130 MiB dbcache may be too large for a system memory of only 64172 MiB.

Although the warning can just be ignored it feels a shame to introduce new false positives if we can avoid it?

l0rinc commented at 6:04 pm on September 11, 2025: contributor

would it make sense to also include an upper bound to suppress/skip the warning

I’m afraid it would be interpreted as a recommendation in that case - all we can claim is that this is likely too high, we can’t actually know the optimal size

feels a shame to introduce new false positives if we can avoid it?

You mean that the uxto size is less than 48 GiB therefore the warning isn’t relevant?

in src/common/system.cpp:118 in 080a21cb12 outdated

110@@ -105,6 +111,21 @@ int GetNumCores()
111     return std::thread::hardware_concurrency();
112 }
113 
114+std::optional<size_t> GetTotalRAM()
115+{
116+#ifdef WIN32
117+    if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) {
118+        return size_t(std::clamp<uint64_t>(m.ullTotalPhys, 0, SIZE_MAX));

stickies-v commented at 10:54 am on September 12, 2025:

nit: using std::numeric_limits<size_t>::max() would avoid using a macro, and has my preference

l0rinc commented at 2:08 pm on September 12, 2025:

I was deliberately trying to avoid that, I find that completely unreadable

stickies-v commented at 2:43 pm on September 14, 2025:

It’s a bit longer, but I find using a commonly used std lib function over a one-off macro more readable and more safe, but no big deal either way.

l0rinc commented at 5:28 pm on September 14, 2025:

If any of you feel strongly, I can change it, but it was a deliberate choice from my part. See https://godbolt.org/z/o3sKqefT5 for an ugly error on Windows that also convinced me to just use the shorter version. I could of course hack it with:

0#ifdef _WIN32
1  #include <windows.h>
2  #undef max
3#else

or just hope that https://github.com/bitcoin/bitcoin/blob/9f744fffc39d93c9966dec1d61f113a7521983ad/CMakeLists.txt#L261 fixes it, but SIZE_MAX seemed simpler and shorter in this case :/

Regardless, if we deduplicate the two branches, we’d have a single clamp/min call and it wouldn’t be as noisy, something like:

0std::optional<size_t> GetTotalRAM()
1{
2    uint64_t mem{0};
3#ifdef WIN32
4    if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) mem = m.ullTotalPhys;
5#elif defined(__linux__) || defined(__APPLE__)
6    if (long p{sysconf(_SC_PHYS_PAGES)}, s{sysconf(_SC_PAGESIZE)}; p > 0 && s > 0) mem = 1ULL * p * s;
7#endif
8    return mem ? size_t(std::min(mem, uint64_t{std::numeric_limits<size_t>::max()})) : std::optional<size_t>{};
9}

would reviewers find that to be safer or more readable?

hodlinator commented at 8:06 pm on September 15, 2025:

Think we should rely on the NOMINMAX define you found in bitcoin/CMakeLists.txt, also prefer numeric_limits. std::clamp<uint64_t>( with the lower bound of 0 wasn’t doing much so agree with getting rid of that.

~0 on switching from separate if-blocks to mutating mem-variable.

l0rinc commented at 0:19 am on September 16, 2025:

Ok, I have applied something similar and updated the godbolt reproducer, thanks for the feedback

willcl-ark commented at 12:34 pm on September 12, 2025: member

You mean that the uxto size is less than 48 GiB therefore the warning isn’t relevant?

What I mean is, we are warning when system ram is below <various_sizes> with the %-based scheme. Why should <1.5GB free emit a warning on some systems, and < 16GB free emit a warning on others? I get that it’s a best-effort algorithm approach, but just not convinced that it makes the most sense.

For example, did you consider just warning if it would leave <1.5GB free on all systems, and if so why choose to opt for the %-based approach over that?

Still concept ACK btw!

in src/init.cpp:1740 in 080a21cb12 outdated

1733@@ -1734,6 +1734,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1734 
1735     // cache size calculations
1736     const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size());
1737+    if (auto total_ram{GetTotalRAM()}) {
1738+        size_t cap{(*total_ram < 2048_MiB) ? DEFAULT_KERNEL_CACHE : (*total_ram / 100) * 75};
1739+        if (kernel_cache_sizes.coins > cap) {
1740+            LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB.", kernel_cache_sizes.coins >> 20, *total_ram >> 20);

stickies-v commented at 1:06 pm on September 12, 2025:

To make this more visible than just the log line, you can use node::warnings to expose it through RPC (e.g. getnetworkinfo["warnings"] and GUI:

 0diff --git a/src/init.cpp b/src/init.cpp
 1index caf1515804..782cb7f6c4 100644
 2--- a/src/init.cpp
 3+++ b/src/init.cpp
 4@@ -56,6 +56,7 @@
 5 #include <node/mempool_persist.h>
 6 #include <node/mempool_persist_args.h>
 7 #include <node/miner.h>
 8+#include <node/warnings.h>
 9 #include <node/peerman_args.h>
10 #include <policy/feerate.h>
11 #include <policy/fees.h>
12@@ -70,6 +71,7 @@
13 #include <scheduler.h>
14 #include <script/sigcache.h>
15 #include <sync.h>
16+#include <tinyformat.h>
17 #include <torcontrol.h>
18 #include <txdb.h>
19 #include <txmempool.h>
20@@ -1737,7 +1739,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
21     if (auto total_ram{GetTotalRAM()}) {
22         size_t cap{(*total_ram < 2048_MiB) ? DEFAULT_KERNEL_CACHE : (*total_ram / 100) * 75};
23         if (kernel_cache_sizes.coins > cap) {
24-            LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB.", kernel_cache_sizes.coins >> 20, *total_ram >> 20);
25+            bilingual_str msg{tfm::format(
26+                _("A %zu MiB dbcache may be too large for a system memory of only %zu MiB."),
27+                kernel_cache_sizes.coins >> 20,
28+                *total_ram >> 20)};
29+            LogWarning("%s", msg.original);
30+            if (Assume(node.warnings)) {
31+                node.warnings->Set(node::Warning::OVERSIZED_DBCACHE, msg);
32+            }
33         }
34     }
35 
36diff --git a/src/node/warnings.h b/src/node/warnings.h
37index 24aeb8a922..b0fc737324 100644
38--- a/src/node/warnings.h
39+++ b/src/node/warnings.h
40@@ -24,6 +24,7 @@ enum class Warning {
41     CLOCK_OUT_OF_SYNC,
42     PRE_RELEASE_TEST_BUILD,
43     FATAL_INTERNAL_ERROR,
44+    OVERSIZED_DBCACHE,
45 };
46 
47 /**

l0rinc commented at 2:11 pm on September 12, 2025:

Good idea, thanks. Do you think we should also add the reason for the enable/disable (we’d store an enum instead of the optional bool which would provide more info on why signature verification was enabled or disabled)?

stickies-v commented at 2:48 pm on September 12, 2025:

This PR is about dbcache

l0rinc commented at 5:49 pm on September 12, 2025:

sorry, woke up in the middle of the night and replied from my phone :))

l0rinc commented at 8:27 pm on September 14, 2025:

Added this in a separate commit (I’m not on my phone anymore :p), reviewers can check the updated PR description or commit message for instructions for how to reproduce it

stickies-v commented at 1:07 pm on September 12, 2025: contributor

Concept ACK

hodlinator commented at 10:16 pm on September 12, 2025: contributor

Concept ACK

Confirmed to work on Windows:

0$ ./build/bin/Release/bitcoind.exe -regtest -dbcache=64534
12025-09-12T22:14:09Z Bitcoin Core version v29.99.0-080a21cb127f-dirty (release build)
2...
32025-09-12T22:14:09Z [warning] A 64524 MiB dbcache may be too large for a system memory of only 65413 MiB.

Edit: Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351 Edit2: The above was a Windows -> Windows build, going to try Guix cross build also, stay posted. Edit3: Guix Linux -> Windows cross build also works:

0C:\Program Files\Bitcoin>bitcoin node -regtest -dbcache=65000
1C:\Program Files\Bitcoin>2025-09-13T18:53:35Z Bitcoin Core version v29.99.0-g080a21cb127f901d59b8478404db513a0ce0ed07 (release build)
2...
32025-09-13T18:53:35Z [warning] A 64990 MiB dbcache may be too large for a system memory of only 65413 MiB.

in src/init.cpp:1737 in 080a21cb12 outdated

1733@@ -1734,6 +1734,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1734 
1735     // cache size calculations
1736     const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size());
1737+    if (auto total_ram{GetTotalRAM()}) {

stickies-v commented at 2:42 pm on September 14, 2025:

Is there a reason you’re using kernel_cache_sizes.coins instead of just inspecting -dbcache? Using the latter is more in line with the what we’re telling the user we’re doing, and makes the log in line with the value the user actually provided.

Also, would prefer carving this out into separate function to minimize bloating an already long init sequence.

Finally, nit: using DEFAULT_DB_CACHE instead of DEFAULT_KERNEL_CACHE seems more appropriate, even if they currently have the same value

 0diff --git a/src/init.cpp b/src/init.cpp
 1index caf1515804..15465093ac 100644
 2--- a/src/init.cpp
 3+++ b/src/init.cpp
 4@@ -1733,13 +1733,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 5     // ********************************************************* Step 7: load block chain
 6 
 7     // cache size calculations
 8+    node::CheckOversizedDbCache(args);
 9     const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size());
10-    if (auto total_ram{GetTotalRAM()}) {
11-        size_t cap{(*total_ram < 2048_MiB) ? DEFAULT_KERNEL_CACHE : (*total_ram / 100) * 75};
12-        if (kernel_cache_sizes.coins > cap) {
13-            LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB.", kernel_cache_sizes.coins >> 20, *total_ram >> 20);
14-        }
15-    }
16 
17     LogInfo("Cache configuration:");
18     LogInfo("* Using %.1f MiB for block index database", kernel_cache_sizes.block_tree_db * (1.0 / 1024 / 1024));
19diff --git a/src/node/caches.cpp b/src/node/caches.cpp
20index d5d69fc204..3b79d5454c 100644
21--- a/src/node/caches.cpp
22+++ b/src/node/caches.cpp
23@@ -5,6 +5,7 @@
24 #include <node/caches.h>
25 
26 #include <common/args.h>
27+#include <common/system.h>
28 #include <index/txindex.h>
29 #include <kernel/caches.h>
30 #include <logging.h>
31@@ -44,4 +45,19 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
32     }
33     return {index_sizes, kernel::CacheSizes{total_cache}};
34 }
35+
36+void CheckOversizedDbCache(const ArgsManager& args)
37+{
38+    auto total_ram{GetTotalRAM()};
39+    const auto db_cache{args.GetIntArg("-dbcache")};
40+    if (!total_ram || !db_cache) return;
41+    *total_ram = std::max<size_t>(*total_ram, 1 << 20);
42+
43+    const size_t cap{(*total_ram < 2048_MiB) ? DEFAULT_DB_CACHE : (*total_ram / 100) * 75};
44+    if (size_t(*db_cache) > cap >> 20) {
45+        LogWarning("A %zu MiB dbcache may be too large for a system memory of only %zu MiB.",
46+                   *db_cache,
47+                   *total_ram >> 20);
48+    }
49+}
50 } // namespace node
51diff --git a/src/node/caches.h b/src/node/caches.h
52index f24e9cc910..ff1e26816a 100644
53--- a/src/node/caches.h
54+++ b/src/node/caches.h
55@@ -27,6 +27,7 @@ struct CacheSizes {
56     kernel::CacheSizes kernel;
57 };
58 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0);
59+void CheckOversizedDbCache(const ArgsManager& args);
60 } // namespace node
61 
62 #endif // BITCOIN_NODE_CACHES_H

l0rinc commented at 5:26 pm on September 14, 2025:

using kernel_cache_sizes.coins instead of just inspecting -dbcache

dbcache isn’t always set (we’d duplicate work by setting the default explicitly). But we already have a duplicate strategy for very small memory environments, so this should be fine.
dbcache is split to other indexes, this is the final value that is actually being used for dbcache. But you could argue that for total memory that’s irrelevant.

prefer carving this out into separate function

Agree

using DEFAULT_DB_CACHE instead of DEFAULT_KERNEL_CACHE seems more appropriate

Good point, added you as coauthor, thanks for the hints.

if (!total_ram || !db_cache) return;

I don’t think we should behave differently if dbcache is set or if it’s explicitly set to 450, so I unified this.

*total_ram = std::max<size_t>(*total_ram, 1 « 20);

Are you anticipating an error here or is this just a way to signal that we don’t really support systems with less than 1 GiB? I left this out

stickies-v commented at 1:12 pm on September 15, 2025:

Are you anticipating an error here or is this just a way to signal that we don’t really support systems with less than 1 GiB? I left this out

In my diff, I’m left shifting total_ram later so I wanted to ensure no UB happened. 1<<20 is 1 MiB so I don’t think we need to handle that with anything more complex than an assert.

l0rinc force-pushed on Sep 14, 2025

l0rinc commented at 8:36 pm on September 14, 2025: contributor

Thanks a lot for the comments and reproducers, added a @hodlinator and @stickies-v as coauthors.

I have split the PR into 3 commits, explained the descisions in the messages (cc: @willcl-ark), added unit tests for whatever I could extract, extracted the warning message to a method and exposed the warning for RPC (thanks @stickies-v).

l0rinc force-pushed on Sep 14, 2025

in src/node/caches.cpp:55 in 099968e8ed outdated

47@@ -44,4 +48,16 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
48     }
49     return {index_sizes, kernel::CacheSizes{total_cache}};
50 }
51+
52+void LogOversizedDbCache(const ArgsManager& args, Warnings& warnings) noexcept
53+{
54+    if (const auto total_ram_bytes{GetTotalRAM()}) {
55+        const size_t cache_bytes{size_t(args.GetIntArg("-dbcache", DEFAULT_DB_CACHE >> 20)) * 1_MiB};

w0xlt commented at 6:23 am on September 15, 2025:

Can this operation overflow size_t ? What happens if someone passes a negative value to -dbcache?

Maybe worth pulling the below snippet into a helper so it can be reused here.

https://github.com/bitcoin/bitcoin/blob/d20f10affba83601f1855bc87d0f47e9dfd5caae/src/node/caches.cpp#L31-L34

stickies-v commented at 1:09 pm on September 15, 2025:

Maybe worth pulling the below snippet into a helper so it can be reused here.

Agreed, a carved out function that just calculates and clamps db_cache_bytes makes most sense, and I think we should get rid of ShouldWarnOversizedDbCache.

l0rinc commented at 6:16 pm on September 15, 2025:

Excellent, added @w0xlt as a coauthor. I kept ShouldWarnOversizedDbCache since this makes it unit-testable.

l0rinc force-pushed on Sep 15, 2025

in src/node/caches.cpp:44 in 9f69c646d7 outdated

43+    return total_cache;
44+}
45+
46+CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
47+{
48+    // Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.

hodlinator commented at 7:48 pm on September 15, 2025:

nit: Comment might fit better above the CalculateDbCacheBytes function definition?

l0rinc commented at 9:00 pm on September 15, 2025:

In that case I’d just delete it since the code already states all of this

in src/test/caches_tests.cpp:38 in 9f69c646d7 outdated

33+
34+        // 32 GiB RAM - cap is 75%
35+        BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/20'000_MiB, /*total_ram=*/32768_MiB)); // Under cap
36+        BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/30'000_MiB, /*total_ram=*/32768_MiB)); // Over cap
37+    }
38+}

hodlinator commented at 8:09 pm on September 15, 2025:

(nit: as stated previously I prefer making simple functions like ShouldWarnOversizedDbCache constexpr and using static_assert instead of runtime checks, #31645 (review)).

l0rinc commented at 0:38 am on September 16, 2025:

I don’t like burdening the compiler this - and it makes errors and debugging uniform if we do it in tests - but I did make it constexpr, thanks for the hint

in src/node/caches.cpp:32 in 9f69c646d7 outdated

26@@ -23,16 +27,22 @@ static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB};
27 static constexpr size_t MAX_32BIT_DBCACHE{1024_MiB};
28 
29 namespace node {
30-CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
31+size_t CalculateDbCacheBytes(const ArgsManager& args)
32 {
33-    // Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
34-    size_t total_cache{DEFAULT_DB_CACHE};
35+    size_t total_cache = {DEFAULT_DB_CACHE};

hodlinator commented at 8:24 pm on September 15, 2025:

Stray addition of = in last commit.

Suggestion: Could remove total_cache completely (the name is more fitting for the original copy which is still initialized by the result of this function anyway):

 0--- a/src/node/caches.cpp
 1+++ b/src/node/caches.cpp
 2@@ -29,14 +29,14 @@ static constexpr size_t MAX_32BIT_DBCACHE{1024_MiB};
 3 namespace node {
 4 size_t CalculateDbCacheBytes(const ArgsManager& args)
 5 {
 6-    size_t total_cache = {DEFAULT_DB_CACHE};
 7     if (std::optional<int64_t> db_cache = args.GetIntArg("-dbcache")) {
 8         if (*db_cache < 0) db_cache = 0;
 9         const uint64_t db_cache_bytes{SaturatingLeftShift<uint64_t>(*db_cache, 20)};
10         constexpr auto max_db_cache{sizeof(void*) == 4 ? MAX_32BIT_DBCACHE : std::numeric_limits<size_t>::max()};
11-        total_cache = std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));
12+        return std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));
13+    } else {
14+        return DEFAULT_DB_CACHE;
15     }
16-    return total_cache;
17 }
18 
19 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)

l0rinc commented at 0:20 am on September 16, 2025:

Done

in src/node/caches.cpp:66 in 9f69c646d7 outdated

61+        const size_t db_cache_bytes{CalculateDbCacheBytes(args)};
62+        if (ShouldWarnOversizedDbCache(db_cache_bytes, *total_ram_bytes)) {
63+            const auto msg{tfm::format(_("A %zu MiB dbcache may be too large for a system memory of only %zu MiB."),
64+                db_cache_bytes >> 20, *total_ram_bytes >> 20)};
65+            LogWarning("%s", msg.original);
66+            Assert(warnings.Set(kernel::Warning::OVERSIZED_DBCACHE, msg));

hodlinator commented at 8:43 pm on September 15, 2025:

Might be overly harsh with Assert()? Could use Assume(), and/or maybe add a comment why this is worth guarding against. Noticed the original suggestion didn’t even check the return value (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344204033).

l0rinc commented at 0:20 am on September 16, 2025:

Changed to Assume, but I don’t find comments useful here.

hodlinator commented at 11:48 am on September 16, 2025:

Yeah, now that you use InitWarning() adjacently, it’s clearer why we would expect true.

hodlinator commented at 8:48 pm on September 15, 2025: contributor

Reviewed 9f69c646d7eb8790ec9fd6d1d5607f554cc2f4ff

system: add helper for fetching total system memory

Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on other platforms we just return an empty optional).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 1 TiB.

https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex
> ullTotalPhys The amount of actual physical memory, in bytes.

https://man7.org/linux/man-pages/man3/sysconf.3.html:
> _SC_PHYS_PAGES The number of pages of physical memory. Note that it is possible for the product of this value and the value of _SC_PAGESIZE to overflow.
> _SC_PAGESIZE Size of a page in bytes. Must not be less than 1.

See https://godbolt.org/z/ec81Tjvrj for further details

278584f852

l0rinc force-pushed on Sep 16, 2025

l0rinc commented at 0:42 am on September 16, 2025: contributor

Thanks for the reviews, keep them coming :)

achow101 commented at 0:59 am on September 16, 2025: member

I think it would be better to use an InitWarning rather than just logging since people are unlikely notice the warning when it’s only in the log.

l0rinc force-pushed on Sep 16, 2025

l0rinc commented at 2:20 am on September 16, 2025: contributor

Changed it to InitWarning, so now the warnings look like:

02025-09-16T02:17:00Z [warning] A 60000 MiB dbcache may be too large for a system memory of only 65536 MiB.
1Warning: A 60000 MiB dbcache may be too large for a system memory of only 65536 MiB.

Thanks for the hint!

w0xlt commented at 6:22 am on September 16, 2025: contributor

ACK https://github.com/bitcoin/bitcoin/pull/33333/commits/13fa0d0a433722f294854001b0561c079db12dbc

BSD operating system support can be a follow-up.

DrahtBot requested review from Sjors on Sep 16, 2025

DrahtBot requested review from stickies-v on Sep 16, 2025

DrahtBot requested review from hodlinator on Sep 16, 2025

DrahtBot requested review from willcl-ark on Sep 16, 2025

DrahtBot requested review from TheCharlatan on Sep 16, 2025

hodlinator approved

hodlinator commented at 11:59 am on September 16, 2025: contributor

ACK 13fa0d0a433722f294854001b0561c079db12dbc

Very helpful to warn users against unintentionally making their system start swapping insanely.

in src/test/system_tests.cpp:25 in 278584f852 outdated

17@@ -16,6 +18,15 @@
18 
19 BOOST_FIXTURE_TEST_SUITE(system_tests, BasicTestingSetup)
20 
21+BOOST_AUTO_TEST_CASE(total_ram)
22+{
23+    BOOST_CHECK_GE(GetTotalRAM(), 1000_MiB);
24+
25+    if constexpr (SIZE_MAX == UINT64_MAX) {

danielabrozzoni commented at 2:45 pm on September 16, 2025:

nit: could benefit from a comment explaining the conditional check

From my understanding you want to check that GetTotalRAM() wouldn’t return an enormous value, and in order to do so, you need to make sure that size_t can accommodate an enormous value (if size_t was UINT32_MAX, GetTotalRAM() would never return values over ~4GB)

l0rinc commented at 9:21 pm on September 16, 2025:

I have explained it in the commit message

The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 1 TiB.

The max value is only validated on 64 bits, since it’s not unreasonable for 32 bits to have max memory, but on 64 bits it’s likely an error. Do you think it needs more explanation than that?

danielabrozzoni commented at 10:17 am on September 17, 2025:

The max value is only validated on 64 bits, since it’s not unreasonable for 32 bits to have max memory, but on 64 bits it’s likely an error.

This wasn’t immediately clear to me :) Maybe add that sentence to the commit message to make it clearer.

danielabrozzoni commented at 8:08 pm on September 16, 2025: contributor

tACK 13fa0d0a433722f294854001b0561c079db12dbc

I tested locally and reproduced the warning:

 0❯ ./build/bin/bitcoind -dbcache=64000
 1...
 22025-09-16T13:57:22Z [warning] A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.
 3Warning: A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.
 4
 5❯ ./build/bin/bitcoin-cli getblockchaininfo
 6{
 7  [...]
 8  "warnings": [
 9    "A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.",
10    "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
11  ]
12}

I left a nit, feel free to ignore if you think it’s unnecessary or if you don’t need to retouch

achow101 commented at 11:53 pm on September 16, 2025: member

The warning should now be available via RPC as wel, run any of these against the running bitcoind:

I don’t think this warning belongs in those warnings. The help text for the warnings output states “any network and blockchain warnings”, and I don’t think this warning about configuration options meets any of these. There are several other similar warnings that we have around configuration options and they do not appear in those warnings fields either, nor should they.

coins: warn on oversized -dbcache

Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).

`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via CalculateDbCacheBytes(). This isn't meant as a recommended setting, rather a likely upper limit.

Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside size_t on 32-bit systems - and this was simpler than casting back and forth.

We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.

If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:

| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
|     1 GiB |             512 |        50.0% |               450* |
|     2 GiB |            1536 |        75.0% |               1536 |
|     4 GiB |            2560 |        62.5% |               3072 |
|     8 GiB |            4096 |        50.0% |               6144 |
|    16 GiB |            4096 |        25.0% |              12288 |
|    32 GiB |            4096 |        12.5% |              24576 |

[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.

Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000

warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```

Besides the [godbolt](https://godbolt.org/z/EPsaE3xTj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351

Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: w0xlt <woltx@protonmail.com>

84cf842b40

l0rinc force-pushed on Sep 16, 2025

l0rinc commented at 11:58 pm on September 16, 2025: contributor

Thanks for the feedback, removed the last commit.

coins: warn on oversized -dbcache #33333

Summary

Fix

Details

Cap

Reproducer

Manual testing

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

coins: warn on oversized `-dbcache` #33333