Add warning on first startup if free disk space is less than necessary #25315

Empact · 2022-06-08T21:08:17Z

This reworks/revives #15848 to add a check for low disk space on first startup and issue a warning if disk space is below the expected space required to accommodate the blocks.

This PR was fashioned by a team of developers at the bitcoin++ conference workshop: "Let's contribute to Bitcoin Core"

Fixes #15813

litch · 2022-06-09T11:44:34Z

Confirmed this in an underpowered VM. I know it was discussed to use a warning instead of an error. I think that a warning, however, should probably not terminate the process. So perhaps it would be better to change this function to return true after emitting the warning.

2022-06-09T11:42:03Z init message: Verifying blocks…
2022-06-09T11:42:03Z  block index             463ms
2022-06-09T11:42:03Z Warning: Disk space for "/home/litch/.bitcoin/blocks" may not accommodate the block files
Warning: Disk space for "/home/litch/.bitcoin/blocks" may not accommodate the block files
2022-06-09T11:42:03Z Shutdown: In progress...
2022-06-09T11:42:03Z scheduler thread exit
2022-06-09T11:42:03Z Shutdown: done
litch@bitcoind-test:~/bitcoin$

Empact · 2022-06-09T13:58:15Z

So perhaps it would be better to change this function to return true after emitting the warning.

Well thanks for testing - in this case we don't need to return at all, as the intention is to note the issue and carry on. Removed the return statement.

src/init.cpp

litch · 2022-06-09T15:48:48Z

...
2022-06-09T15:43:48Z Initializing databases...
2022-06-09T15:43:48Z Opening LevelDB in /home/litch/.bitcoin/chainstate
2022-06-09T15:43:49Z Opened LevelDB successfully
2022-06-09T15:43:49Z Wrote new obfuscate key for /home/litch/.bitcoin/chainstate: 5da37d724c848cd8
2022-06-09T15:43:49Z Using obfuscation key for /home/litch/.bitcoin/chainstate: 5da37d724c848cd8
2022-06-09T15:43:49Z init message: Verifying blocks…
2022-06-09T15:43:49Z  block index            1014ms
2022-06-09T15:43:49Z Warning: Disk space for "/home/litch/.bitcoin/blocks" may not accommodate the block files. Approximately 460 GB of data will be stored in this directory.
Warning: Disk space for "/home/litch/.bitcoin/blocks" may not accommodate the block files. Approximately 460 GB of data will be stored in this directory.
2022-06-09T15:43:49Z loadblk thread start
2022-06-09T15:43:49Z UpdateTip: new best=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 version=0x00000001 log2_work=32.000022 tx=1 date='2009-01-03T18:15:05Z' progr
ess=0.000000 cache=0.0MiB(0txo)
2022-06-09T15:43:49Z Failed to open mempool file from disk. Continuing anyway.
2022-06-09T15:43:49Z loadblk thread exit
2022-06-09T15:43:49Z block tree size = 1
...

When running on a VM with a small HDD, this prints the warning as expected.

After downloading a number of blocks, then re-starting bitcoind, the warning is not re-emitted.

src/init.cpp

Empact · 2022-06-16T05:46:08Z

Disabled the warning on fReindexChainState, and reworked it to avoid duplicative check in the cases where additional_bytes_needed would be 0.

Empact · 2022-06-16T06:00:41Z

Moved the check further down initialization, to the point where chain_active_height was already being read, so we don't need to do any new locking - a question to test is: will the height be the appropriate value at this time in the startup?

luke-jr · 2022-06-18T17:37:23Z

Concept ACK (as a warning only).

achow101 · 2022-06-20T20:52:30Z

I don't think the current location in initialization is correct. ~50 lines above, we start the thread for importing blocks from an external file. This thread will be copying the blocks from the external file to our datadir and then put them all into the block index. It additionally does not do ActivateBestChain until after all of the blocks have been loaded, so we could end up in a similar scenario as a reindex - all the blocks are written but the height still remains 0.

We need to be checking the disk space before -loadblk is handled so that we can know whether we have enough disk space before we begin loading blocks from the external file(s).

DrahtBot · 2022-06-21T00:54:10Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	achow101, aureleoules, willcl-ark
Concept ACK	hernanmarino, luke-jr, pablomartin4btc

Conflicts

No conflicts as of last run.

hernanmarino · 2022-08-05T17:24:07Z

I agree with @achow101 I believe this should be done earlier.

pablomartin4btc

As @hernanmarino and @achow101, perhaps we could move that validation after the other 2 that also check disk space? (around line 1635)

Empact · 2022-08-08T05:32:20Z

As suggested, moved the check back to being alongside the other disk space checks.

hernanmarino

Code review ACK bba667e

Empact · 2022-08-08T15:56:27Z

Will need some accommodation for the tests - ignoring the message perhaps?:

test 2022-08-08T06:47:18.274000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main
self.run_test()
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_crosschain.py", line 53, in run_test
self.stop_nodes()
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 567, in stop_nodes
node.stop_node(wait=wait, wait_until_stopped=False)
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_node.py", line 350, in stop_node
raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
AssertionError: Unexpected stderr Warning: Disk space for "/tmp/cirrus-ci-build/ci/scratch/test_runner/test_runner_₿_🏃_20220808_063508/wallet_crosschain_80/node1/testnet3/blocks" may not accommodate the block files. Approximately 40 GB of data will be stored in this directory. !=

https://cirrus-ci.com/task/5510419358941184?logs=ci#L4359

pablomartin4btc

Core review ACK.

And it works as expected, only displaying the warning the first time before we start loading blocks.

2022-08-08T19:01:43Z Opening LevelDB in /tmp/btc/chainstate
2022-08-08T19:01:43Z Opened LevelDB successfully
2022-08-08T19:01:43Z Wrote new obfuscate key for /tmp/btc/chainstate: f4392c646d173853
2022-08-08T19:01:43Z Using obfuscation key for /tmp/btc/chainstate: f4392c646d173853
2022-08-08T19:01:43Z init message: Verifying blocks…
2022-08-08T19:01:43Z  block index              82ms
2022-08-08T19:01:43Z Warning: Disk space for "/tmp/btc/blocks" may not accommodate the block files. Approximately 460 GB of data will be stored in this directory.
2022-08-08T19:02:24Z loadblk thread start
2022-08-08T19:02:24Z UpdateTip: new best=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 version=0x00000001 log2_work=32.000022 tx=1 date='2009-01-03T18:15:05Z' progress=0.000000 cache=0.0MiB(0txo)
2022-08-08T19:02:24Z block tree size = 1
2022-08-08T19:02:24Z nBestHeight = 0
2022-08-08T19:02:24Z Failed to open mempool file from disk. Continuing anyway.
2022-08-08T19:02:24Z loadblk thread exit

Also on the UI:

pablomartin4btc · 2022-08-09T10:08:39Z

@Empact, I also get these 2 funtional tests failing when I run them locally (they pass on the master):

aureleoules

Concept and Code Review ACK bba667e.
I also verified that the warning is shown.

CI fails because the tests p2p_dos_header_tree.py and wallet_crosschain.py try to spawn a testnet chain, which would be too big for the CI and creates an output on stderr which should not happen.

This fix works on my machine but I haven't tested in CI:

diff --git a/test/functional/p2p_dos_header_tree.py b/test/functional/p2p_dos_header_tree.py
index fde1e4bfa..922e65036 100755
--- a/test/functional/p2p_dos_header_tree.py
+++ b/test/functional/p2p_dos_header_tree.py
@@ -22,6 +22,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.chain = 'testnet3'  # Use testnet chain because it has an early checkpoint
         self.num_nodes = 2
+        self.extra_args = [['-prune=550']] * self.num_nodes
 
     def add_options(self, parser):
         parser.add_argument(
@@ -62,7 +63,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
 
         self.log.info("Feed all fork headers (succeeds without checkpoint)")
         # On node 0 it succeeds because checkpoints are disabled
-        self.restart_node(0, extra_args=['-nocheckpoints'])
+        self.restart_node(0, extra_args=['-nocheckpoints', '-prune=550'])
         peer_no_checkpoint = self.nodes[0].add_p2p_connection(P2PInterface())
         peer_no_checkpoint.send_and_ping(msg_headers(self.headers_fork))
         assert {
diff --git a/test/functional/wallet_crosschain.py b/test/functional/wallet_crosschain.py
index b6d0c8798..c0047542e 100755
--- a/test/functional/wallet_crosschain.py
+++ b/test/functional/wallet_crosschain.py
@@ -21,7 +21,7 @@ class WalletCrossChain(BitcoinTestFramework):
 
         # Switch node 1 to testnet before starting it.
         self.nodes[1].chain = 'testnet3'
-        self.nodes[1].extra_args = ['-maxconnections=0'] # disable testnet sync
+        self.nodes[1].extra_args = ['-maxconnections=0', '-prune=550'] # disable testnet sync
         with open(self.nodes[1].bitcoinconf, 'r', encoding='utf8') as conf:
             conf_data = conf.read()
         with open (self.nodes[1].bitcoinconf, 'w', encoding='utf8') as conf:
@@ -51,7 +51,7 @@ class WalletCrossChain(BitcoinTestFramework):
         if not self.options.descriptors:
             self.log.info("Override cross-chain wallet load protection")
             self.stop_nodes()
-            self.start_nodes([['-walletcrosschain']] * self.num_nodes)
+            self.start_nodes([['-walletcrosschain', '-prune=550']] * self.num_nodes)
             self.nodes[0].loadwallet(node1_wallet)
             self.nodes[1].loadwallet(node0_wallet)

EDIT: it works in CI as well

To accommodate the expected blocks data. Co-authored-by: Antoine Poinsot <darosior@protonmail.com> Co-authored-by: benthecarman <benthecarman@live.com> Co-authored-by: Justin Litchfield <litch@me.com> Co-authored-by: Liran Cohen <c.liran.c@gmail.com> Co-authored-by: Ryan Loomba <ryan.loomba@gmail.com> Co-authored-by: Buck Perley <bucko.perley@gmail.com> Co-authored-by: bajjer <bajjer@bajjer.xyz> Co-authored-by: Suhail Saqan <suhail.saqan@gmail.com> Co-authored-by: Christopher Sweeney <sweeney.chris@gmail.com> Co-authored-by: Alyssa <orbitalturtle@protonmail.com> Co-authored-by: Ben Schroth <ben@styng.social> Co-authored-by: Jason Hester <mail@jason-hester.me> Co-authored-by: Matt Clough <Matt.clough@pm.me> Co-authored-by: Elise Schedler <eliseschedler@gmail.com> Co-authored-by: ghander <cen254@gmail.com> Co-authored-by: PopeLaz <btclz@fastmail.com> Co-authored-by: Aurèle Oulès <hello@aureleoules.com>

aureleoules

ACK 6630a1e

achow101 · 2022-10-12T19:31:16Z

cc @mzumsande @theStack

achow101 · 2022-10-25T22:45:38Z

ACK 6630a1e

willcl-ark · 2022-11-17T20:45:31Z

tACK 6630a1e rebased on master. Warning shows on first start but not on restart after some blocks have been downloaded.

pablomartin4btc

re-ACK 6630a1e

hernanmarino

ReACK 6630a1e

…is less…

Empact · 2022-11-18T17:54:06Z

Thanks to all the reviewers and contributors. For posterity, here's the original session in which this was written: https://youtu.be/tRXIkqM8gSI

Empact force-pushed the disk-space-check branch 3 times, most recently from 3ec10dd to 914e1e8 Compare June 8, 2022 22:40

Empact force-pushed the disk-space-check branch from 914e1e8 to 86bb6f4 Compare June 9, 2022 13:55

Empact force-pushed the disk-space-check branch from 86bb6f4 to 5fa1f76 Compare June 9, 2022 13:59

maflcko reviewed Jun 9, 2022

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

Empact force-pushed the disk-space-check branch 4 times, most recently from 735f677 to f2d7bf5 Compare June 9, 2022 15:31

mzumsande reviewed Jun 9, 2022

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

Empact force-pushed the disk-space-check branch 2 times, most recently from ce0c198 to 05ff20c Compare June 16, 2022 05:42

Empact force-pushed the disk-space-check branch 2 times, most recently from 11c10ef to 6f935fd Compare June 16, 2022 05:57

Empact force-pushed the disk-space-check branch from 6f935fd to 11fc52c Compare June 16, 2022 06:04

Empact mentioned this pull request Jun 16, 2022

refactor: Extract MIB_BYTES constant for init.cpp #25386

Closed

pablomartin4btc reviewed Aug 5, 2022

View reviewed changes

Empact force-pushed the disk-space-check branch from 11fc52c to bba667e Compare August 8, 2022 05:28

hernanmarino approved these changes Aug 8, 2022

View reviewed changes

pablomartin4btc approved these changes Aug 8, 2022

View reviewed changes

aureleoules reviewed Sep 21, 2022

View reviewed changes

Empact force-pushed the disk-space-check branch 3 times, most recently from 2845677 to 1732375 Compare October 10, 2022 20:34

Empact force-pushed the disk-space-check branch from 1732375 to 6630a1e Compare October 10, 2022 20:58

aureleoules approved these changes Oct 11, 2022

View reviewed changes

glozow requested a review from mzumsande October 12, 2022 19:31

aureleoules mentioned this pull request Nov 17, 2022

webhook_features: Trim whitespaces in comments maflcko/DrahtBot#7

Closed

pablomartin4btc reviewed Nov 18, 2022

View reviewed changes

hernanmarino approved these changes Nov 18, 2022

View reviewed changes

achow101 merged commit aeb395d into bitcoin:master Nov 18, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 18, 2022

Merge bitcoin#25315: Add warning on first startup if free disk space …

cdc7c7d

…is less…

ghost mentioned this pull request Mar 11, 2023

Permission to comment on PR 27235 #27243

Closed

bitcoin locked and limited conversation to collaborators Jun 10, 2024

Add warning on first startup if free disk space is less than necessary #25315

Add warning on first startup if free disk space is less than necessary #25315

Uh oh!

Conversation

Empact commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

litch commented Jun 9, 2022

Uh oh!

Empact commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

litch commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Empact commented Jun 16, 2022

Uh oh!

Empact commented Jun 16, 2022

Uh oh!

luke-jr commented Jun 18, 2022

Uh oh!

achow101 commented Jun 20, 2022

Uh oh!

DrahtBot commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

hernanmarino commented Aug 5, 2022

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

Empact commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

Empact commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc commented Aug 9, 2022

Uh oh!

aureleoules left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aureleoules left a comment

Choose a reason for hiding this comment

Uh oh!

achow101 commented Oct 12, 2022

Uh oh!

achow101 commented Oct 25, 2022

Uh oh!

willcl-ark commented Nov 17, 2022

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

Empact commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Empact commented Jun 8, 2022 •

edited

Loading

Empact commented Jun 9, 2022 •

edited

Loading

litch commented Jun 9, 2022 •

edited

Loading

DrahtBot commented Jun 21, 2022 •

edited

Loading

Empact commented Aug 8, 2022 •

edited

Loading

Empact commented Aug 8, 2022 •

edited

Loading

aureleoules left a comment •

edited

Loading

Empact commented Nov 18, 2022 •

edited

Loading