During IBD, prune as much as possible until we get close to where we will eventually keep blocks #20827

luke-jr · 2021-01-02T01:49:09Z

This should reduce pruning flushes even more, speeding up IBD with pruning on systems that have a sufficient dbcache.

Assumes 1 MB per block between tip and best header chain. Simply adds this to the buffer pruning is trying to leave available, which results in pruning almost everything up until we get close to where we need to be keeping blocks.

DrahtBot · 2021-01-02T02:01:42Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	andrewtoth, fjahr, achow101
Concept ACK	jonasschnelli, theStack, 0xB10C, jonatack, kristapsk

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

Conflicts

No conflicts as of last run.

jonatack · 2021-01-04T00:16:09Z

Interesting, will have a look.

jonasschnelli · 2021-01-04T08:26:06Z

Nice!
Concept ACK.
Could we further reduce flushes by allowing a user parameter (-pruneflushbuffer [or similar])? Some users are probably okay with providing a few GBs of extra space for speeding up pruning (assume hight dbcache) (but wan't the extra space be freed after IBD).
Or we could automatically use he dbcache size as the buffer size.

Sjors · 2021-01-08T17:50:03Z

I also like the idea of temporally assigning more RAM for IBD. This could even be done by default for GUI users (with opt-out in the intro screen).

luke-jr · 2021-01-08T18:33:07Z

dbcache adjustments are ultimately an unrelated feature. I see #19873 as the next step in that area.

DrahtBot · 2021-01-13T11:45:55Z

🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file.

luke-jr · 2021-07-06T23:28:45Z

src/validation.cpp

+        // So when pruning in IBD, increase the buffer to avoid a re-prune too soon.
+        if (is_ibd && target_sync_height > (uint64_t)chain_tip_height) {
+            // Since this is only relevant during IBD, we assume blocks are at least 1 MB on average
+            static constexpr uint64_t average_block_size = 1000000;  /* 1 MB */


AFAIK we don't have access to future block sizes at this point.

stickies-v · 2021-12-16T14:15:35Z

Without rebase, compiling fails for me:

$ make clean && ./configure CXXFLAGS="-O0 -g" CFLAGS="-O0 -g" && make -j 9
...
touch src/config/bitcoin-config.h.in
  CXX      util/libbitcoin_util_a-moneystr.o
  CXX      util/libbitcoin_util_a-readwritefile.o
  CXX      util/libbitcoin_util_a-settings.o
  CXX      util/libbitcoin_util_a-serfloat.o
  CXX      util/libbitcoin_util_a-spanparsing.o
  CXX      util/libbitcoin_util_a-strencodings.o
  CXX      util/libbitcoin_util_a-string.o
  CXX      util/libbitcoin_util_a-url.o
make[3]: *** No rule to make target `libunivalue.la'.  Stop.
make[2]: *** [univalue/libunivalue.la] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1

After git rebase master, everything runs smoothly again. Will report back shortly with my review results!

maflcko · 2021-12-16T14:21:13Z

@stickies-v If you want to compile older commits of master, you'll need to make distclean first.

luke-jr · 2021-12-16T16:44:43Z

Without rebase, compiling fails for me:

Note: It's usually best to merge PRs into master for testing. Sometimes (bugfixes) they can be based on very old commits (when the bug was introduced); and if there's a silent problem with the merge, you'd want to notice that too.

@stickies-v If you want to compile older commits of master, you'll need to make distclean first.

Can we fix this build system bug?

theStack · 2021-12-20T01:15:59Z

Concept ACK

0xB10C · 2021-12-20T11:36:53Z

Concept ACK

We'll be doing a Bitcoin Core PR Review club covering this PR on the 29th: bitcoincore.reviews/20827

0xB10C · 2021-12-26T12:03:27Z

#12404 (not merged) may also be interesting to reviewers. @Sjors did run a few benchmarks back then and found #11658 (which this PR overrides) to be the better option at that time.

I'm planing on benchmarking this change too.

2e42050 doc: fix undo data filename (s/undo???.dat/rev???.dat/) (Sebastian Falbesoner) Pull request description: This typo was discovered in the course of a review club to #20827, see https://bitcoincore.reviews/20827#l-31. ACKs for top commit: shaavan: ACK 2e42050 Tree-SHA512: 0c7a00dce24c03bee6d37265d5b4bc97e976c3f3910af1113f967f6298940f892d6fb517f7b154f32ccedb365060314d4d78d5eb2a9c68b25f0859a628209cd3

…?.dat/) 2e42050 doc: fix undo data filename (s/undo???.dat/rev???.dat/) (Sebastian Falbesoner) Pull request description: This typo was discovered in the course of a review club to bitcoin#20827, see https://bitcoincore.reviews/20827#l-31. ACKs for top commit: shaavan: ACK 2e42050 Tree-SHA512: 0c7a00dce24c03bee6d37265d5b4bc97e976c3f3910af1113f967f6298940f892d6fb517f7b154f32ccedb365060314d4d78d5eb2a9c68b25f0859a628209cd3

fjahr · 2022-01-09T18:36:48Z

Concept ACK, I read the PR Review Club and will review after rebase.

0xB10C · 2022-01-10T15:52:33Z

fwiw: #23581 moved BlockManager to node/blockstorage, that's where the conflict with master happens.

I've rebased this PR in https://github.com/0xB10C/bitcoin/tree/2022-01-lukejr-ibd-prune-max-rebased and started benchmarking 0xB10C@db2a9e7 (PR) vs 0xB10C@2e01b69 (mergebase; MB).

I have 8 different prune and dbcache configurations set-up. Each configuration runs three times per binary (PR and MB).
That's 8 configs * 3 runs * 2 binaries = 48. I expect this to take about 6 days. Configurations:

1. -dbcache=300 -prune=550
2. -dbcache=4000 -prune=550

3. -dbcache=300 -prune=1100
4. -dbcache=4000 -prune=1100

5. -dbcache=300 -prune=2200
6. -dbcache=4000 -prune=2200

7. -dbcache=300 -prune=4400
8. -dbcache=4000 -prune=4400

Benchmarks run on a dedicated, properly cooled, and otherwise idle machine with two HDDs in a RAID0. The node syncs from a local node on the same machine and disks (connected via addnode and otherwise connect=0). The sync starts at block height 500_000 and ends at height 600_000. Between each run, the machine idles 5 minutes. The debug.log, with extra debug=coindb debug=prune logging, of each run is saved. I'll post results and graphs next week.

0xB10C · 2022-01-15T10:29:16Z

With a prune just before IBD, there is more blk and undo data available when running master than with this change. One potential issue that was mentioned during the PR review club:

On non-mainnet networks, the 1MB per block assumption doesn't hold. When we prune with a large nBuffer just before IBD ends, the prune might cause problems on larger reorgs as not enough undo data is present to revert back to the last shared block. Testnet has seen large reorgs in the past.

Might be worth thinking about potential alternatives to increasing the nBuffer by 1MB * remaining_blocks. There might be cleaner ways to implement "prune everything as long as we are not close to IBD being done". Haven't come up with a practical threshold for "IBD being close to done" yet that works for all networks.

luke-jr · 2022-01-15T19:09:03Z

The 1 MB per block assumption would mean at least 550 blocks. Testnet has deeper reorgs than that?

luke-jr · 2023-07-19T01:25:45Z

Rebased

jonatack · 2023-07-19T01:33:59Z

Concept ACK

kristapsk · 2023-07-19T09:42:28Z

Concept ACK

BenWestgate · 2023-08-04T04:15:23Z

I have a project that syncs bitcoin core on an external drive and this would really help a lot of people testing it with smaller flash drives and higher RAM. So much so that I've made scripts to do manual pruning (back to 550mib) only when disk space is getting low to emulate this.

achow101 · 2023-09-20T16:22:26Z

Are you still working on this?

…will eventually keep blocks

andrewtoth · 2023-12-27T23:52:26Z

src/node/blockstorage.cpp

@@ -298,6 +298,7 @@ void BlockManager::FindFilesToPrune(
    // Distribute our -prune budget over all chainstates.
    const auto target = std::max(
        MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / chainman.GetAll().size());
+    const uint64_t target_sync_height = chainman.m_best_header->nHeight;


nit: Any reason this declaration is up here and not directly above where it is used below?

andrewtoth · 2023-12-27T23:53:01Z

src/node/blockstorage.cpp

-            // Since this is only relevant during IBD, we use a fixed 10%
-            nBuffer += target / 10;
+        // So when pruning in IBD, increase the buffer to avoid a re-prune too soon.
+        const auto chain_tip_height = chain.m_chain.Height();


nit: could remove the cast in the next line with

Suggested change

const auto chain_tip_height = chain.m_chain.Height();

const uint64_t chain_tip_height = chain.m_chain.Height();

andrewtoth · 2023-12-30T15:11:33Z

ACK d298ff8

Ran benchmark with hyperfine:

hyperfine --show-output --parameter-list commit 96ec3b67a7a7f968d002e13d6fc227f69b7f07d7,d298ff8b62b2624ed390c8a2f905c888ffc956ff --setup 'git checkout {commit} && make -j$(nproc) src/bitcoind' --prepare 'sync; sudo /sbin/sysctl vm.drop_caches=3; rm -r /home/user/.bitcoin/blocks /home/user/.bitcoin/chainstate' -M 3 './src/bitcoind -dbcache=2000 -prune=2000 -printtoconsole=0 -stopatheight=800000'

Results were 21% faster on this branch 🚀

Summary
  ./src/bitcoind -dbcache=2000 -prune=2000 -printtoconsole=0 -stopatheight=800000 (commit = d298ff8b62b2624ed390c8a2f905c888ffc956ff) ran
    1.21 ± 0.00 times faster than ./src/bitcoind -dbcache=2000 -prune=2000 -printtoconsole=0 -stopatheight=800000 (commit = 96ec3b67a7a7f968d002e13d6fc227f69b7f07d7)

fjahr · 2024-01-12T20:20:40Z

utACK d298ff8

While there may be slightly better configurations possible, as @0xB10C pointed out above, I think the 1MB assumption is alright and this is a clear improvement already, so this can be merged and potentially improved later.

achow101 · 2024-01-25T20:07:25Z

ACK d298ff8

vostrnad · 2024-02-28T19:18:25Z

15% improvement in IBD performance in some configurations seems significant enough for a release note, no?

luke-jr force-pushed the ibd_prune_max branch from 6b8a4b3 to 2c35ce7 Compare January 2, 2021 02:03

DrahtBot added the Validation label Jan 2, 2021

DrahtBot mentioned this pull request Jan 2, 2021

Allow maintaining the blockfilterindex when using prune #15946

Merged

1 task

DrahtBot added the Needs rebase label Feb 18, 2021

luke-jr commented Jul 6, 2021

View reviewed changes

luke-jr force-pushed the ibd_prune_max branch from 2c35ce7 to 24f3936 Compare August 31, 2021 03:50

DrahtBot removed the Needs rebase label Aug 31, 2021

DrahtBot mentioned this pull request Sep 1, 2021

assumeutxo #15606

Closed

18 tasks

DrahtBot mentioned this pull request Sep 15, 2021

refactor: Move mutable globals cleared in ::UnloadBlockIndex to BlockManager #22564

Merged

DrahtBot mentioned this pull request Nov 24, 2021

Move BlockManager to node/blockstorage #23581

Merged

theStack mentioned this pull request Jan 1, 2022

doc: fix undo data filename (s/undo???.dat/rev???.dat/) #23929

Merged

DrahtBot added the Needs rebase label Jan 4, 2022

luke-jr marked this pull request as ready for review July 19, 2023 01:25

DrahtBot removed the Needs rebase label Jul 19, 2023

maflcko added Block storage and removed Validation labels Jul 19, 2023

This was referenced Aug 5, 2023

refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate #28218

Merged

assumeutxo (2) #27596

Merged

DrahtBot added the Needs rebase label Aug 21, 2023

0xB10C mentioned this pull request Aug 30, 2023

Don't empty dbcache on prune flushes: >30% faster IBD #28280

Merged

During IBD, prune as much as possible until we get close to where we …

d298ff8

…will eventually keep blocks

luke-jr force-pushed the ibd_prune_max branch from 1d23d95 to d298ff8 Compare December 27, 2023 22:41

DrahtBot removed the Needs rebase label Dec 27, 2023

andrewtoth reviewed Dec 27, 2023

View reviewed changes

DrahtBot requested review from jonatack, fjahr, 0xB10C and theStack December 30, 2023 15:11

DrahtBot removed the request for review from fjahr January 12, 2024 20:20

DrahtBot added the CI failed label Jan 24, 2024

achow101 merged commit 3672099 into bitcoin:master Jan 25, 2024

Sjors mentioned this pull request Mar 11, 2024

Current default settings are broken, some fix is needed #29603

Closed

bitcoin locked and limited conversation to collaborators Feb 27, 2025

	const auto chain_tip_height = chain.m_chain.Height();
	const uint64_t chain_tip_height = chain.m_chain.Height();

During IBD, prune as much as possible until we get close to where we will eventually keep blocks #20827

During IBD, prune as much as possible until we get close to where we will eventually keep blocks #20827

Uh oh!

Conversation

luke-jr commented Jan 2, 2021

Uh oh!

DrahtBot commented Jan 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

jonatack commented Jan 4, 2021

Uh oh!

jonasschnelli commented Jan 4, 2021

Uh oh!

Sjors commented Jan 8, 2021

Uh oh!

luke-jr commented Jan 8, 2021

Uh oh!

DrahtBot commented Jan 13, 2021

Uh oh!

luke-jr Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

stickies-v commented Dec 16, 2021

Uh oh!

maflcko commented Dec 16, 2021

Uh oh!

luke-jr commented Dec 16, 2021

Uh oh!

theStack commented Dec 20, 2021

Uh oh!

0xB10C commented Dec 20, 2021

Uh oh!

0xB10C commented Dec 26, 2021

Uh oh!

fjahr commented Jan 9, 2022

Uh oh!

0xB10C commented Jan 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xB10C commented Jan 15, 2022

Uh oh!

luke-jr commented Jan 15, 2022

Uh oh!

luke-jr commented Jul 19, 2023

Uh oh!

jonatack commented Jul 19, 2023

Uh oh!

kristapsk commented Jul 19, 2023

Uh oh!

BenWestgate commented Aug 4, 2023

Uh oh!

achow101 commented Sep 20, 2023

Uh oh!

andrewtoth Dec 27, 2023

Choose a reason for hiding this comment

Uh oh!

andrewtoth Dec 27, 2023

Choose a reason for hiding this comment

Uh oh!

andrewtoth commented Dec 30, 2023

Uh oh!

fjahr commented Jan 12, 2024

Uh oh!

achow101 commented Jan 25, 2024

Uh oh!

vostrnad commented Feb 28, 2024

Uh oh!

Uh oh!

DrahtBot commented Jan 2, 2021 •

edited

Loading

0xB10C commented Jan 10, 2022 •

edited

Loading