Autoprune #4701

pull rdponticelli wants to merge 9 commits into bitcoin:master from Criptomonedas:autoprune changing 3 files +256 −42
  1. rdponticelli commented at 8:09 pm on August 14, 2014: contributor

    This pull implements a new mode of operation which automatically removes old block files trying to maintain at most a maximum amount of disk space used by the node. This amount is configured by the user with the -prune switch.

    There’s also a lightweight sanity check which executes periodically during runtime to make sure the minimum block files required for the node to be operative are present.

    This should allow to lower the amount of resources needed to run a node.

    See the individual commits, about all the changes introduced.

  2. gmaxwell commented at 10:06 pm on August 14, 2014: contributor
    Very cool. I went to go make some random suggestions but found you implemented them already. I’ll give this more review soon.
  3. laanwj added the label Feature on Aug 18, 2014
  4. in src/init.cpp: in 73ed2bd6c2 outdated 
    224@@ -225,6 +225,7 @@ std::string HelpMessage(HelpMessageMode mode)
225     strUsage += "  -maxorphanblocks=<n>   " + strprintf(_("Keep at most <n> unconnectable blocks in memory (default: %u)"), DEFAULT_MAX_ORPHAN_BLOCKS) + "\n";
226     strUsage += "  -par=<n>               " + strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -(int)boost::thread::hardware_concurrency(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS) + "\n";
227     strUsage += "  -pid=<file>            " + _("Specify pid file (default: bitcoind.pid)") + "\n";
228+    strUsage += "  -pruned                " + _("Run in a pruned state") + "\n";
    sipa commented at 11:28 am on August 23, 2014:
    Nit: “Prune old blocks” may be an easier explanation for the flag.
  5. in src/main.cpp: in 73ed2bd6c2 outdated 
    3540-                                    pfrom->PushMessage("tx", block.vtx[pair.first]);
3541+                    if (!ReadBlockFromDisk(block, (*mi).second)) {
3542+                        if (fPruned) {
3543+                            // Ignore requests which ask us for blocks we don't have any more
3544+                            // Peers shouldn't ask, anyway, as we unset NODE_NETWORK on this mode.
3545+                            LogPrintf("cannot load block from disk, ignoring request from peer=%d\n", pfrom->id);
    sipa commented at 11:31 am on August 23, 2014:
    I think you should disconnect such peers (for their sake) rather than just ignoring.
    rdponticelli commented at 10:07 pm on August 24, 2014:
    Added rejection and disconnection.
  6. in src/init.cpp: in 73ed2bd6c2 outdated 
    588+        if (SoftSetBoolArg("-disablewallet", true))
589+            LogPrintf("AppInit2 : parameter interaction: -pruned=1 -> setting -disablewallet=1\n");
590+        else
591+            return InitError(_("Can't run with a wallet in pruned mode."));
592+    }
593+#endif
    TheBlueMatt commented at 11:00 pm on August 23, 2014:
    Actually, I might just always InitError here. Its not obvious that -pruned will disable wallet from the explanation given and it may be overly confusing.
    rdponticelli commented at 10:07 pm on August 24, 2014:
    A proper explanation has been added.
  7. in src/init.cpp: in 73ed2bd6c2 outdated 
    960@@ -950,6 +961,14 @@ bool AppInit2(boost::thread_group& threadGroup)
961                     break;
962                 }
963 
964+                if (!CheckAndPruneBlockFiles()) {
965+                    if (!fPruned)
966+                        strLoadError = _("Error checking required block files. You might try to run in -pruned mode, or try to rebuild the block database");
    luke-jr commented at 11:05 pm on August 23, 2014:
    Should probably warn of the destructive nature of -pruned?
  8. in src/main.cpp: in 73ed2bd6c2 outdated 
    3001+}
3002 
3003+bool CheckAndPruneBlockFiles()
3004+{
3005+    // Check presence of essential data
3006+    int nKeepBlksFromHeight = fPruned ? (max((int)(chainActive.Height() - MIN_BLOCKS_TO_KEEP), 0)) : 0;
    luke-jr commented at 11:08 pm on August 23, 2014:
    Maybe this should be configurable? -pruned 500 to keep all blocks in the main chain at height 500+ vs -pruned -500 to keep the last 500?
    rdponticelli commented at 1:56 am on September 2, 2014:
    I’m working on something like this. The code is still messy, but I would wait for it to be ready before merging this. The default would be -pruned=0 which means do now autoprune, just allow running pruned.
  9. in src/init.cpp: in 73ed2bd6c2 outdated 
    582@@ -582,6 +583,15 @@ bool AppInit2(boost::thread_group& threadGroup)
583     if (nFD - MIN_CORE_FILEDESCRIPTORS < nMaxConnections)
584         nMaxConnections = nFD - MIN_CORE_FILEDESCRIPTORS;
585 
586+#ifdef ENABLE_WALLET
587+    if ((GetBoolArg("-pruned", false)) && !GetBoolArg("-disablewallet", false)) {
588+        if (SoftSetBoolArg("-disablewallet", true))
589+            LogPrintf("AppInit2 : parameter interaction: -pruned=1 -> setting -disablewallet=1\n");
590+        else
591+            return InitError(_("Can't run with a wallet in pruned mode."));
    luke-jr commented at 11:09 pm on August 23, 2014:
    Why not?
    rdponticelli commented at 8:59 pm on August 24, 2014:
    Because otherwise people can very easily shoot themselves in the foot by plugging in an old wallet which silently fails rescanning, getting in an inconsistent state.
    luke-jr commented at 9:11 pm on August 24, 2014:
    Could just fail if it needs a rescan…
    rdponticelli commented at 10:05 pm on August 24, 2014:
    Yeah, but I would better leave that for another pull, when such a case has been more extensively tested.
  10. gmaxwell commented at 0:30 am on August 24, 2014: contributor
    luke: wrt pruning depth, probably what would be good eventually is a size target and then the software can make use of the size target usefully… but I don’t know that it makes sense at this point since we don’t yet have a good way to make use of a sparse blockchain.
  11. luke-jr commented at 1:00 am on August 24, 2014: member
    We have to use it for reorgs. Setting a default prune depth is probably dangerous enough to becoming an (inconsistent) consensus rule already.
  12. gmaxwell commented at 1:12 am on August 24, 2014: contributor
    By sparse I mean containing any blocks other than the last N. If you’ll note, the number 288 above comes from my comments on the prior PR as a minimum number I’d consider acceptable as an absolute minimum for the purpose of reorgs.
  13. rdponticelli force-pushed on Aug 24, 2014
  14. rdponticelli force-pushed on Aug 25, 2014
  15. rdponticelli force-pushed on Aug 26, 2014
  16. in src/main.cpp: in 8ba326b852 outdated 
    2883@@ -2881,11 +2884,17 @@ bool CheckDiskSpace(uint64_t nAdditionalBytes)
2884     return true;
2885 }
2886 
2887+boost::filesystem::path GetBlockFilePath(const CDiskBlockPos &pos, const char *prefix)
2888+{
2889+    boost::filesystem::path path = GetDataDir() / "blocks" / strprintf("%s%05u.dat", prefix, pos.nFile);
    sipa commented at 2:19 pm on August 26, 2014:
    Nit: no need for the intermediate variable.
  17. in src/main.cpp: in 8ba326b852 outdated 
    2920@@ -2912,6 +2921,14 @@ FILE* OpenUndoFile(const CDiskBlockPos &pos, bool fReadOnly) {
2921     return OpenDiskFile(pos, "rev", fReadOnly);
2922 }
2923 
2924+bool RemoveBlockFile(const CDiskBlockPos &pos) {
2925+    return boost::filesystem::remove(GetBlockFilePath(pos, "blk")) ? true : false;
    sipa commented at 2:21 pm on August 26, 2014:
    No need for the ? true : false. boost::filesystem::remove already returns a bool.
  18. in src/main.cpp: in 8ba326b852 outdated 
    2920@@ -2912,6 +2921,14 @@ FILE* OpenUndoFile(const CDiskBlockPos &pos, bool fReadOnly) {
2921     return OpenDiskFile(pos, "rev", fReadOnly);
2922 }
2923 
2924+bool RemoveBlockFile(const CDiskBlockPos &pos) {
2925+    return boost::filesystem::remove(GetBlockFilePath(pos, "blk")) ? true : false;
2926+}
2927+
2928+bool RemoveUndoFile(const CDiskBlockPos &pos) {
2929+    return boost::filesystem::remove(GetBlockFilePath(pos, "rev")) ? true : false;
    sipa commented at 2:21 pm on August 26, 2014:
    Same.
  19. in src/main.cpp: in 8ba326b852 outdated 
    3546-                                    pfrom->PushMessage("tx", block.vtx[pair.first]);
3547+                    if (!ReadBlockFromDisk(block, (*mi).second)) {
3548+                        if (fPruned) {
3549+                            // Reject requests and disconnect peers asking us for blocks we don't have.
3550+                            // Doing so we avoid stalling their IBD but they shouldn't ask as we unset NODE_NETWORK on this mode.
3551+                            LogPrintf("cannot load block from disk, rejecting request and disconnectiong peer:%d\n", pfrom->id);
    sipa commented at 2:27 pm on August 26, 2014:
    disconnecting
  20. in src/main.cpp: in 8ba326b852 outdated 
    3021+                if (CAutoFile(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION)) {
3022+                    pblocktree->ReadBlockFileInfo(pindex->nFile, info);
3023+                    if (chainActive.Height() > AUTOPRUNE_AFTER_HEIGHT && (int)info.nHeightLast < nKeepBlksFromHeight) {
3024+                        if (RemoveBlockFile(pos)) {
3025+                            LogPrintf("File blk%05u.dat removed\n", pindex->nFile);
3026+                            mapBlkDataFileReadable.insert(make_pair(pindex->nFile, false));
    sipa commented at 2:28 pm on August 26, 2014:

    mapBlkDataFileReadable[pindex->nFile] = false;

    etc

  21. in src/main.cpp: in 8ba326b852 outdated 
    3547+                    if (!ReadBlockFromDisk(block, (*mi).second)) {
3548+                        if (fPruned) {
3549+                            // Reject requests and disconnect peers asking us for blocks we don't have.
3550+                            // Doing so we avoid stalling their IBD but they shouldn't ask as we unset NODE_NETWORK on this mode.
3551+                            LogPrintf("cannot load block from disk, rejecting request and disconnectiong peer:%d\n", pfrom->id);
3552+                            pfrom->PushMessage("reject", string("getdata"), REJECT_NOTFOUND, string("Not found"));
    sipa commented at 2:29 pm on August 26, 2014:
    notfound message instead of reject?
  22. rdponticelli force-pushed on Aug 26, 2014
  23. rdponticelli force-pushed on Aug 26, 2014
  24. sipa commented at 11:39 pm on August 26, 2014: member

    When a block is being disconnected due to a reorg, and its data cannot be loaded from disk, there is currently just a state.Abort with “Failed to read block”. Exceedingly unlikely, but we need to be able to deal with such situations. I wonder whether crashing with some extra help/debug output may be enough, or whether we need to retry downloading the missing data…

    EDIT: Downloading the missing data might work for block data, but not for undo data, so it will be unlikely to be useful.

  25. gmaxwell commented at 11:49 pm on August 26, 2014: contributor

    I’d like to re-download, and thought that would be interesting to explore with headers first in place— but the problem is that if the undo data is deleted we cannot usefully redownload.

    Edit: ah, you noticed that. Yea, well— we could have different retention policies for undo date. I considered that future work. If ever we make the undo data normative we could just store hashes of it and fetch it from peers too.

  26. sipa commented at 11:53 pm on August 26, 2014: member
    Of course, we could for example delete blocks data at depth N, but only delete undo data at depth N_3 or so (undo data is 7-10 times smaller than block data). Of course, that just moves the problem further to what to do when an N_3 deep reorg is encountered.
  27. sipa commented at 0:49 am on August 27, 2014: member
    Untested ACK. I guess I’m fine with resolving the missing-block/undo problem for reorgs later.
  28. rdponticelli force-pushed on Aug 27, 2014
  29. rdponticelli force-pushed on Aug 28, 2014
  30. rdponticelli force-pushed on Sep 2, 2014
  31. sipa commented at 5:38 pm on September 12, 2014: member
    Needs rebase.
  32. rdponticelli force-pushed on Sep 12, 2014
  33. rdponticelli force-pushed on Sep 16, 2014
  34. rdponticelli force-pushed on Sep 17, 2014
  35. rdponticelli commented at 5:46 pm on September 17, 2014: contributor
    Added configure options, refactored a little to improve readability, rebased and fixed some small formatting nits remaining.
  36. in src/init.cpp: in df41b2d88c outdated 
    228@@ -229,6 +229,7 @@ std::string HelpMessage(HelpMessageMode mode)
229     strUsage += "  -maxorphantx=<n>       " + strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS) + "\n";
230     strUsage += "  -par=<n>               " + strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -(int)boost::thread::hardware_concurrency(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS) + "\n";
231     strUsage += "  -pid=<file>            " + _("Specify pid file (default: bitcoind.pid)") + "\n";
232+    strUsage += "  -pruned=<n>            " + _("Run in a pruned state (default: 0 = allow running pruned but do not autoprune; <n> > 0 = automatically prune old block files up to height <n>; <n> < 0 = automatically prune up to last <n> blocks. Warning: going back to be a full node after pruning requires a very expensive resync process. This mode also disables wallet support and is incompatible with txindex)") + "\n";
    Diapolo commented at 5:08 am on September 18, 2014:
    May I suggest calling this -prune or -autoprune and also rephrase the help message for this? It IMHO sounds weird to have an option in that grammar.
    laanwj commented at 9:39 am on September 18, 2014:

    Let’s not assume that people know what you mean with “prune” at the beginning of the message. Also the warning should not be in the (…), and I’d just say “re-downloading” instead of “a very expensive resync process”.

    Instead of

    Run in a pruned state (default: 0 = allow running pruned but do not autoprune; > 0 = automatically prune old block files up to height ; < 0 = automatically prune up to last blocks. Warning: going back to be a full node after pruning requires a very expensive resync process. This mode also disables wallet support and is incompatible with txindex)

    I’d propose:

    Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex. Warning: going back to full storage after pruning requires re-downloading the entire blockchain. (default: 0 = allow running with missing blocks, but do not delete anything; > 0 = delete up to block height <n>; < 0 = delete all but last <n> blocks)

  37. in src/init.cpp: in df41b2d88c outdated 
    978@@ -962,6 +979,14 @@ bool AppInit2(boost::thread_group& threadGroup)
979                     break;
980                 }
981 
982+                if (!CheckAndPruneBlockFiles()) {
983+                    if (!fPruned)
984+                        strLoadError = _("Error checking required block files. You might try to run in -pruned mode, or try to rebuild the block database (Warning: please check carefully the implications of running pruned before doing so)");
    rdponticelli commented at 5:24 pm on September 18, 2014:
    Any suggestion about how to make this user facing text better?
    laanwj commented at 7:10 am on September 19, 2014:
    I’d just say “Error checking required block files” in both cases. I wouldn’t suggest using -pruned as a way to get around errors caused by random disk corruption (for example). Someone that wants to run a pruned node will make a conscious choice to do so.
  38. rdponticelli force-pushed on Sep 19, 2014
  39. in src/init.cpp: in 8e9d6a36b4 outdated 
    228@@ -229,6 +229,7 @@ std::string HelpMessage(HelpMessageMode mode)
229     strUsage += "  -maxorphantx=<n>       " + strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS) + "\n";
230     strUsage += "  -par=<n>               " + strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -(int)boost::thread::hardware_concurrency(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS) + "\n";
231     strUsage += "  -pid=<file>            " + _("Specify pid file (default: bitcoind.pid)") + "\n";
232+    strUsage += "  -prune=<n>            " + _("Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex. Warning: going back to full storage after pruning requires re-downloading the entire blockchain. (default: 0 = allow running with missing blocks, but do not delete anything; > 0 = delete up to block height <n>; < 0 = delete all but last <n> blocks)") + "\n";
    Diapolo commented at 2:29 pm on September 19, 2014:

    Much better, thanks :)! A little nit, you are missing a single space after -prune=<n> ;).

    May I also suggest splitting the sentences of the description, like in -debug=<category>? That makes it (IMHO) easier to change the string in the future (translations) and better looking in the output.

    While I’m at nitting: Warning: going back to full storage after pruning requires re-downloading the entire blockchain. (default: 0 = allow running with missing blocks, but do not delete anything; > 0 = delete up to block height <n>; < 0 = delete all but last <n> blocks) could read: Warning: Reverting this setting requires re-downloading the entire blockchain! (default: 0 = allow running with missing blocks, but do not delete anything, >0 = delete up to block height <n>, <0 = delete all but last <n> blocks)

    I think going back to full storage after pruning is not that easy to translate and I changed the used ; as separators with , and remove the spaces before > and <.

  40. in src/init.cpp: in 8e9d6a36b4 outdated 
    595@@ -595,6 +596,19 @@ bool AppInit2(boost::thread_group& threadGroup)
596     if (nFD - MIN_CORE_FILEDESCRIPTORS < nMaxConnections)
597         nMaxConnections = nFD - MIN_CORE_FILEDESCRIPTORS;
598 
599+    if (mapArgs.count("-prune")) {
600+        if (GetBoolArg("-txindex", false))
601+            return InitError(_("Pruned mode is incompatible with -txindex."));
602+#ifdef ENABLE_WALLET
603+        if (!GetBoolArg("-disablewallet", false)) {
    Diapolo commented at 2:39 pm on September 19, 2014:
    This should be moved to parameter interactions in Step 2: parameter interactions in init.cpp?
    rdponticelli commented at 4:02 pm on September 19, 2014:
    That’s right were it is? :P
  41. in src/init.cpp: in 8e9d6a36b4 outdated 
    646@@ -633,6 +647,9 @@ bool AppInit2(boost::thread_group& threadGroup)
647     fPrintToConsole = GetBoolArg("-printtoconsole", false);
648     fLogTimestamps = GetBoolArg("-logtimestamps", true);
649     fLogIPs = GetBoolArg("-logips", false);
650+    fPruned = mapArgs.count("-prune") ? 1 : 0;
651+    if (fPruned)
652+        nPruned = GetArg("-prune", 0);
    Diapolo commented at 2:42 pm on September 19, 2014:
    Wouldn’t it be possible to just use GetArg("-prune", 0); to determine if pruned mode is active? If != 0 it’s active and you’ve got <n>.
    rdponticelli commented at 4:07 pm on September 19, 2014:
    But that way you lose the possibility or running pruned without autopruning.
  42. in src/init.cpp: in 8e9d6a36b4 outdated 
    978@@ -962,6 +979,11 @@ bool AppInit2(boost::thread_group& threadGroup)
979                     break;
980                 }
981 
982+                if (!CheckAndPruneBlockFiles()) {
983+                    strLoadError = _("Error checking required block files");
    Diapolo commented at 2:45 pm on September 19, 2014:

    I fear that this message is too generic? At least you are using 2 different messages for failing calls to !CheckAndPruneBlockFiles().

    Edit: This is still true, see below.

  43. in src/main.cpp: in 8e9d6a36b4 outdated 
    2825@@ -2822,6 +2826,28 @@ boost::filesystem::path GetBlockPosFilename(const CDiskBlockPos &pos, const char
2826     return GetDataDir() / "blocks" / strprintf("%s%05u.dat", prefix, pos.nFile);
2827 }
2828 
2829+bool RemoveBlockFile(int nFile)
2830+{
2831+    CDiskBlockPos pos(nFile, 0);
2832+    if (boost::filesystem::remove(GetBlockPosFilename(pos, "blk"))) {
    Diapolo commented at 2:50 pm on September 19, 2014:
    Perhaps use GetBlockPosFilename(pos, "blk") also for the log message? You could perhaps also merge these 2 functions into one by just passing a flag for undo or normal block file, as most code is a duplicate.
  44. in src/main.cpp: in 8e9d6a36b4 outdated 
    2962+            nAutoPruneUpToHeight = min(nKeepMinBlksFromHeight, nPruned);
2963+        } else if (nPruned < 0) {
2964+            LogPrintf("Autoprune configured to keep the last: %i blocks\n", -nPruned);
2965+            nAutoPruneUpToHeight = min((chainActive.Height() + nPruned), nKeepMinBlksFromHeight);
2966+        }
2967+        LogPrintf("Current height: %i.\n", chainActive.Height());
    Diapolo commented at 2:51 pm on September 19, 2014:
    Nit: . seems unneeded.
  45. gmaxwell commented at 4:13 pm on September 19, 2014: contributor
    Can someone help me understand the usecase for running pruned without autopruning? My original expectation was that you’d just tell it to prune and a disk usage limit, and it would do the right thing(tm) for the network within that limit (subject to the constraint that it won’t prune so much that its likely to get stuck).
  46. sipa commented at 5:26 pm on September 19, 2014: member
    Agree with @gmaxwell. It seems an artefact of how it got implemented to see “ability to run without all data” and “automatically remove data under some policy” as separate. Once you delete a single block file, you’re already unable to function as a full node (in the traditional NODE_NETWORK sense).
  47. ghost commented at 5:01 pm on September 22, 2014: none

    I have been able to successfully sync a non-listening x86_64 node over the network using this patch using very little disk space and the option prune=-50000. Node has been stable for 4 days without any problems so far. Was able to do unusual operations like reducing the number of retained blocks during the initial synchronization with no apparent problems.

    0Autopruning active.
1Autoprune configured to keep the last: 50000 blocks
2Current height: 182982.
3Autopruning up to height 132982
4Checking all required data for active chain is available (mandatory from height 182694 to 182982)
5Data for blocks from 1 to 131236 has been pruned
6Undo data for blocks from 1 to 131236 has been pruned

    Logged autoprune messages appear to be sane during sync.

  48. laanwj commented at 9:27 am on September 23, 2014: member
    @bitcoinpulltesterhuman thanks for testing
  49. rdponticelli force-pushed on Sep 24, 2014
  50. rdponticelli force-pushed on Sep 25, 2014
  51. in src/init.cpp: in 84529a3e52 outdated 
    608+        if (GetBoolArg("-txindex", false))
609+            return InitError(_("Prune mode is incompatible with -txindex."));
610+#ifdef ENABLE_WALLET
611+        if (!GetBoolArg("-disablewallet", false)) {
612+            if (SoftSetBoolArg("-disablewallet", true))
613+                LogPrintf("AppInit2 : parameter interaction: -prune -> setting -disablewallet=1\n");
    Diapolo commented at 6:49 am on September 26, 2014:
    You might change this to (so we don’t need to rework log strings when function names change in the future): LogPrintf("%s: parameter interaction: -prune set -> setting -disablewallet=1\n", __func__);
  52. in src/main.cpp: in 84529a3e52 outdated 
    2186@@ -2186,6 +2187,8 @@ bool FindBlockPos(CValidationState &state, CDiskBlockPos &pos, unsigned int nAdd
2187         unsigned int nOldChunks = (pos.nPos + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;
2188         unsigned int nNewChunks = (infoLastBlockFile.nSize + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;
2189         if (nNewChunks > nOldChunks) {
2190+            if (!CheckAndPruneBlockFiles())
2191+                return state.Abort("Error checking block files");
    Diapolo commented at 6:50 am on September 26, 2014:
    Talking about that message here ;).
    rdponticelli commented at 4:21 pm on September 26, 2014:

    Any suggestion?

    Anyway, take into account that in case of failure, the function itself throws a more specific log…

  53. in src/init.cpp: in 84529a3e52 outdated 
    1275@@ -1252,6 +1276,8 @@ bool AppInit2(boost::thread_group& threadGroup)
1276     LogPrintf("mapAddressBook.size() = %u\n",  pwalletMain ? pwalletMain->mapAddressBook.size() : 0);
1277 #endif
1278 
1279+    if (nPrune) // unsetting NODE_NETWORK on prune state
1280+        nLocalServices &= ~NODE_NETWORK;
    Diapolo commented at 6:51 am on September 26, 2014:
    @sipa Should such a thing be logged so that it’s clear we are not a full node anymore?
    gmaxwell commented at 6:57 am on September 26, 2014:
    It is a full node— its validation and security model is the same; its not node-network though. Service bits are show in the rpc info output, no harm in logging something. Just please don’t say that it’s not a full node.
    Diapolo commented at 6:58 am on September 26, 2014:
    I’m sorry, I didn’t know the term full node is still correct here!
  54. rdponticelli force-pushed on Sep 26, 2014
  55. BitcoinPullTester commented at 6:41 pm on September 26, 2014: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4701_8893254172c8efd807418218dea7a7a4022d3f26/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
  56. sipa commented at 10:05 pm on September 30, 2014: member
    I’ve submitted a patch (#4988) that keeps data about all block files in memory, which should simplify making a version of this that functions as gmaxwell’s suggestion (have a disk usage limit instead of a block number limit).
  57. laanwj added this to the milestone 0.10.0 on Oct 8, 2014
  58. rdponticelli referenced this in commit 3eaf3d4cfc on Oct 9, 2014
  59. rdponticelli force-pushed on Oct 10, 2014
  60. ghost commented at 5:05 am on October 11, 2014: none

    @rdponticelli I’m not convinced 98daccf is quite right. @gmaxwell’s suggestion that the limit should be in megabytes of blocks is sound, but defining them as a negative of the amount of disk space is not the right direction to go. Depending how I use my disks I could end up ridiculously overpruning my blocks, which requires an unfriendly resync to rectify. I believe it would be a lot more useful for the limit to be defined as a maximum for the block chain size on disk, or at least give an option between the two.

    I have tested this latest commit and can’t see any troubles yet.

  61. laanwj commented at 10:17 am on October 11, 2014: member
    I agree with @BitcoinPullTesterHuman here. Setting a fixed quota on bitcoind’s disk usage would be most useful. Making it about keeping a certain amount of disk space free would make it depend on other factors, and potentially interact very badly with other programs that reason in the same way.
  62. rdponticelli force-pushed on Oct 11, 2014
  63. rdponticelli commented at 9:07 pm on October 11, 2014: contributor
    Better now?
  64. ghost commented at 0:47 am on October 12, 2014: none

    Seems to do what it says on the tin.

     0...
 1UpdateTip: new best=000000000000000020cb9c249f38d295031483debe1966608859a8c3576f0be9  height=314903  log2_work=80.125233  tx=44239173  date=2014-08-10 12:50:42 progress=0.800166  cache=6452  
 2UpdateTip: new best=000000000000000036e888b57e08c72d2d3c57869e24b759d8f728f87002ee41  height=314904  log2_work=80.125326  tx=44239192  date=2014-08-10 12:49:49 progress=0.800165  cache=6496  
 3Autoprune configured to use less than 10000MB on disk for block files  
 4Capacity: 80158MB; Available: 5400MB; Used for block files: 9897MB  
 5UpdateTip: new best=000000000000000014078ca51becfb85d6104a553ac32bbb9759922125b58f00  height=314905  log2_work=80.125418  tx=44240419  date=2014-08-10 13:25:07 progress=0.800240  cache=0  
 6Autoprune configured to use less than 10000MB on disk for block files  
 7Capacity: 80158MB; Available: 5393MB; Used for block files: 9897MB  
 8Pre-allocating up to position 0x700000 in rev00164.dat  
 9UpdateTip: new best=00000000000000000f34337e2854eed6d315dc8f1b2ab27ebc3d1c82abbe2fbb  height=314906  log2_work=80.125511  tx=44240870  date=2014-08-10 13:37:51 progress=0.800268  cache=1321  
10UpdateTip: new best=0000000000000000119c842e1af4e8b3507e7eb21fcdf14263d5524e5946b276  height=314907  log2_work=80.125604  tx=44241044  date=2014-08-10 13:42:24 progress=0.800278  cache=1951  
11UpdateTip: new best=0000000000000000161388f89b8d4caea72dca599bfce374e375b93786cfc651  height=314908  log2_work=80.125697  tx=44241113  date=2014-08-10 13:43:58 progress=0.800281  cache=2100  
12Autoprune configured to use less than 10000MB on disk for block files  
13Capacity: 80158MB; Available: 5392MB; Used for block files: 9898MB  
14Pre-allocating up to position 0x4000000 in blk00164.dat  
15UpdateTip: new best=000000000000000027a296aafdce13174d474c00f88cf55f37e1909304ff42d6  height=314909  log2_work=80.125789  tx=44243223  date=2014-08-10 14:46:51 progress=0.800415  cache=6913  
16Autoprune configured to use less than 10000MB on disk for block files  
17Capacity: 80158MB; Available: 5364MB; Used for block files: 9914MB  
18UpdateTip: new best=00000000000000002d62caf86485a31de7558020fe5dcbb7c3443057a5159bb9  height=314910  log2_work=80.125882  tx=44244883  date=2014-08-10 15:35:13 progress=0.800518  cache=0  
19UpdateTip: new best=00000000000000001736a28c2d8170da93ec7affc9c7bbc9f3e50abbcd392f08  height=314911  log2_work=80.125975  tx=44245395  date=2014-08-10 15:58:44 progress=0.800566  cache=1129  
20UpdateTip: new best=00000000000000001ae4b7f7a7ddc650a8f2ae2ffd3dc5ccab4acb61266f1a44  height=314912  log2_work=80.126067  tx=44245894  date=2014-08-10 15:47:21 progress=0.800550  cache=3173  
21UpdateTip: new best=00000000000000001cd15ea31d92b3704b1d74a4c6f7cdcbb51df303f6f5e640  height=314913  log2_work=80.12616  tx=44246363  date=2014-08-10 15:53:37 progress=0.800566  cache=4598  
22UpdateTip: new best=00000000000000002c34dd824eb4d46d2ab11034316136e9a5f4e3424f2f2a32  height=314914  log2_work=80.126253  tx=44247171  date=2014-08-10 16:06:07 progress=0.800597  cache=6700  
23Autoprune configured to use less than 10000MB on disk for block files  
24Capacity: 80158MB; Available: 5381MB; Used for block files: 9914MB  
25UpdateTip: new best=000000000000000000489b46754c90f3cda614f5a8df17413fd8eb85dad3c1a6  height=314915  log2_work=80.126345  tx=44247610  date=2014-08-10 16:11:11 progress=0.800610  cache=0  
26UpdateTip: new best=00000000000000001fc91469023097e3cad6822abd566f1534495e68232bfed9  height=314916  log2_work=80.126438  tx=44248002  date=2014-08-10 16:16:39 progress=0.800624  cache=1398  
27UpdateTip: new best=00000000000000001fa3937d72ae89092fc9e738fd24c6ebe5341596151f6327  height=314917  log2_work=80.12653  tx=44248456  date=2014-08-10 16:24:41 progress=0.800643  cache=2762  
28 ...

    The logging output is a little verbose but that’s a cosmetic issue rather than a functional one.

  65. in src/main.cpp: in df07387e46 outdated 
    2899+}
2900 
2901+bool BlockFileReadable(int nFile)
2902+{
2903+    CDiskBlockPos pos(nFile, 0);
2904+    return CAutoFile(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION) ? true : false;
    TheBlueMatt commented at 1:41 am on October 12, 2014:
    For sanity you may want to actually add bool operator() to CAutoFIle (the only bool operator declared there is bool operator!()).
    laanwj commented at 9:43 am on October 12, 2014:

    CAutoFile already ‘degenerates’ into a FILE*. Let’s not add more magic behavior.

    0operator FILE*()            { return file; }

    I’d prefer an explicit method ‘bool IsNull() const { return file==0; }’.

    TheBlueMatt commented at 6:18 pm on October 12, 2014:
    Sure, that’d be better. Maybe remove the bool operator!() too, since that is strange.
    Diapolo commented at 10:53 am on October 21, 2014:
    This needs to be changed when @laanwj CAutoFile pull is merged then…
  66. in src/init.cpp: in df07387e46 outdated 
    606+        if (GetBoolArg("-txindex", false))
607+            return InitError(_("Prune mode is incompatible with -txindex."));
608+#ifdef ENABLE_WALLET
609+        if (!GetBoolArg("-disablewallet", false)) {
610+            if (SoftSetBoolArg("-disablewallet", true))
611+                LogPrintf("%s : parameter interaction: -prune -> setting -disablewallet=1\n", __func__);
    rebroad commented at 8:53 am on October 12, 2014:
    What’s the point in running this if not as a wallet?
    unknown commented at 9:36 am on October 12, 2014:
    Being autopruned is incompatible with a wallet, really. If you need to rescan or load a wallet that’s older than the blocks you have on disk, you’re going to have to download them and throw them out all over again. As I understand it, it’s an implied requirement of having a wallet that you must be able to do a rescan as necessary.
    gmaxwell commented at 9:45 am on October 12, 2014:
    This isn’t so, but the extra things required to do that haven’t been implemented yet and shouldn’t be part of this patch.
    laanwj commented at 9:45 am on October 12, 2014:
    Running with wallet usefully with pruned blocks (and still allowing import/rescan) would require SPV support in the wallet.
    rebroad commented at 12:00 pm on June 15, 2015:
    @laanwj Would it be desirable to add SPV support to Bitcoin Core now that pruning is included?
    jonasschnelli commented at 12:11 pm on June 15, 2015:
    SPV and autoprune are orthogonal IMO. I’m working on a SPV mode for bitcoin-cores wallet.
    laanwj commented at 12:15 pm on June 15, 2015:

    I was wrong in my above comment, it is possible to use the wallet with pruning as of #6057. It just excludes some operations such as rescanning which need the entire chain.

    SPV mode is orthogonal to that, though. That has always been desirable, and pruning hasn’t changed that.

  67. rdponticelli force-pushed on Oct 18, 2014
  68. rdponticelli commented at 6:25 pm on October 18, 2014: contributor
    Rebased. I’m also trying to improve the commits history to make the changes less invasive, and the whole thing more review-able. But meanwhile, this is at least working .
  69. in src/init.cpp: in fb82da2b0a outdated 
    239@@ -240,6 +240,10 @@ std::string HelpMessage(HelpMessageMode mode)
240 #ifndef WIN32
241     strUsage += "  -pid=<file>            " + _("Specify pid file (default: bitcoind.pid)") + "\n";
242 #endif
243+    strUsage += "  -prune=<n>             " + _("Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex.") + "\n";
244+    strUsage += "                         " + _("Warning: Reverting this setting requires re-downloading the entire blockchain!") + "\n";
245+    strUsage += "                         " + _("(default: 0 = disable pruning blocks,") + "\n";
246+    strUsage += "                         " + _("         >0 = max size in MB to use for block files") + "\n";
    Diapolo commented at 10:47 am on October 21, 2014:
    IMHO the formatting spaces in front of this should be moved out of the translations string. Also is there a minimum defined which needs to be set for this to work?
  70. in src/init.cpp: in fb82da2b0a outdated 
    659@@ -643,6 +660,11 @@ bool AppInit2(boost::thread_group& threadGroup)
660     fPrintToConsole = GetBoolArg("-printtoconsole", false);
661     fLogTimestamps = GetBoolArg("-logtimestamps", true);
662     fLogIPs = GetBoolArg("-logips", false);
663+    if (GetArg("-prune", 0) < 0)
664+        return InitError(_("Error: Negative -prune values doesn't make sense."));
665+    nPrune = GetArg("-prune", 0) * 1024 * 1024;
666+    if (nPrune && nPrune < MIN_BLOCK_FILES_SIZE)
    Diapolo commented at 10:49 am on October 21, 2014:
    Ah, here it is… you could use that in the help message string.
  71. in src/init.cpp: in fb82da2b0a outdated 
    659@@ -643,6 +660,11 @@ bool AppInit2(boost::thread_group& threadGroup)
660     fPrintToConsole = GetBoolArg("-printtoconsole", false);
661     fLogTimestamps = GetBoolArg("-logtimestamps", true);
662     fLogIPs = GetBoolArg("-logips", false);
663+    if (GetArg("-prune", 0) < 0)
664+        return InitError(_("Error: Negative -prune values doesn't make sense."));
    Diapolo commented at 10:50 am on October 21, 2014:
    Not sure this needs to be an error, why not set the default instead? See nConnectTimeout for example.
  72. in src/main.cpp: in fb82da2b0a outdated 
    2726 
2727+bool PruneBlockFiles()
2728+{
2729+    if (!CheckBlockFiles())
2730+        return false;
2731+    if (!setDataFilePrunable.empty())
    Diapolo commented at 10:52 am on October 21, 2014:
    Could also be: if (!setDataFilePrunable.empty() && RemoveBlockFile(*setDataFilePrunable.rbegin())). Same for next if clause.
  73. in src/main.cpp: in fb82da2b0a outdated 
    2814@@ -2762,6 +2815,7 @@ boost::filesystem::path GetBlockPosFilename(const CDiskBlockPos &pos, const char
2815     return GetDataDir() / "blocks" / strprintf("%s%05u.dat", prefix, pos.nFile);
2816 }
2817 
2818+
    Diapolo commented at 10:52 am on October 21, 2014:
    Nit: I guess that’s not intended ;)?
  74. rdponticelli force-pushed on Oct 23, 2014
  75. rdponticelli force-pushed on Oct 23, 2014
  76. rdponticelli force-pushed on Oct 26, 2014
  77. Michagogo commented at 10:00 am on October 30, 2014: contributor
    Re: txindex, what does txindex track exactly? List of transaction hashes in each block? In that case, how about just saying “if a pruned transaction is requested, pull that block from a peer to find it”?
  78. rdponticelli force-pushed on Oct 30, 2014
  79. rdponticelli force-pushed on Oct 30, 2014
  80. sipa commented at 3:13 pm on November 4, 2014: member
    @Michagogo easier said than done.
  81. rdponticelli commented at 10:09 pm on November 5, 2014: contributor
    Some commits were pushed addressing the feedback. I’ll rebase and squash if there aren’t further comments, not to change commits already reviewed.
  82. rdponticelli force-pushed on Nov 7, 2014
  83. rdponticelli commented at 6:59 pm on November 7, 2014: contributor
    Rebased and squashed.
  84. rdponticelli force-pushed on Nov 9, 2014
  85. rdponticelli force-pushed on Nov 9, 2014
  86. sipa commented at 5:44 pm on November 10, 2014: member

    Just testing with -prune=1000, and I notice that it prefers deleting block files over undo files. That’s suboptimal for two reasons:

    • undo data is useless without the corresponding block data (unless we have a mechanism in place to redownload missing blocks, which is safe, while undo data can’t be reconstructed).
    • over time, a larger and larger percentage of the selected disk space will be wasted on (very old) undo data, while less and less is available for block data.

    I’d suggest deleting them simultaneously.

  87. in src/main.cpp: in ef7184713c outdated 
    2791 bool CheckDiskSpace(uint64_t nAdditionalBytes)
2792 {
2793+    if (nPrune) {
2794+        while (BlockFilesSize() + nAdditionalBytes > nPrune) {
2795+            if (!PruneBlockFiles())
2796+                return AbortNode("Can't keep block files size as low as requested", _("Can't keep block files size as low as requested"));
    TheBlueMatt commented at 8:35 am on November 12, 2014:
    I would avoid considering -prune as a mandate, I’d prefer if it was a soft goal (maybe with an option to quit if it gets too large)
    gmaxwell commented at 8:41 am on November 12, 2014:
    We already have a quit if you run out of space… which could be made user tunable, though I worry about introducing too many arguments at once. Perhaps in a follow on patch?
    TheBlueMatt commented at 9:04 am on November 12, 2014:
    Tried to test, but couldnt get a value for prune that didnt just quit here, and removing the quit resulted in an infinite loop calling CheckBlockFiles.
    rdponticelli commented at 1:54 pm on November 12, 2014:

    Did you run it on a node already pruned by a previous version of this patch, or pruned manually?

    Anyway, I’ll fix this behavior.

    TheBlueMatt commented at 8:27 pm on November 12, 2014:
    Started with a completely fresh datadir.
  88. laanwj commented at 4:41 pm on November 17, 2014: member
    I’m going to bump this to 0.11 - probably an unpopular decision, but the 0.10 split-off is coming very close IMO this hasn’t received enough testing yet. I would like to hear more reports from some people that have actually used this first. I’ve tried to solicit this on twitter and other places, without much success.
  89. laanwj added this to the milestone 0.11.0 on Nov 17, 2014
  90. laanwj removed this from the milestone 0.10.0 on Nov 17, 2014
  91. gavinandresen commented at 4:49 pm on November 17, 2014: contributor

    This could really use a written test plan (e.g. test with existing, populated datadir, test with fresh sync, test what happens if prune size is increased or decreased, test on windows, etc). See https://github.com/bitcoin/QA/

    Once a test plan is written, I’d be happy to pay (out of core dev grant funds) testers 50,000 bits to work through the tests.

  92. sipa commented at 5:17 pm on November 17, 2014: member

    Just tested with a clean datadir, with -prune=500, and seems to work as expected (block and undo files are added and deleted in sync, remaining under 500 MB.

    Starting afterwards without -prune (and before full sync is done) results in a: Error checking required block files. There must be missing or unreadable data.

    Do you want to rebuild the block database now?

  93. rdponticelli commented at 5:45 pm on November 17, 2014: contributor
    Yes. Should I allow it to run on a pruned state even if the -prune switch isn’t specified? I thought such behavior has been discouraged on previous comments…
  94. sipa commented at 5:56 pm on November 17, 2014: member
    Not a criticism; just a test report.
  95. TheBlueMatt commented at 0:52 am on November 18, 2014: member

    I was trying to test this with a new block-tester, but ran into #5267 (and some issues here, as mentioned), before I could get very far :(.

    On 11/17/14 16:49, Gavin Andresen wrote:

    This could really use a written test plan (e.g. test with existing, populated datadir, test with fresh sync, test what happens if prune size is increased or decreased, test on windows, etc). See https://github.com/bitcoin/QA/

    Once a test plan is written, I’d be happy to pay testers 50,000 bits to work through the tests.

    — Reply to this email directly or view it on GitHub #4701 (comment).

  96. sipa commented at 10:00 am on November 18, 2014: member
    If a pruned block is being connected or disconnected, the node will just abort with “Failed to read block”. Is that intentional? With a guaranteed 288-block window that is retained, I guess it’s acceptable, but we need IMHO at least some documentation mentioning how to recover from such a state.
  97. instagibbs commented at 3:13 am on November 24, 2014: member

    Ran my own tests, same setup to sipa. (500 MB)

    Everything worked as expected until I tried to run a full node again “-prune=0”, and was greeted with the missing data message. Responding with “yes” gives me:

    2014-11-24 03:06:39 : Error checking required block files. There must be missing or unreadable data.

    Do you want to rebuild the block database now? 2014-11-24 03:06:39 Aborted block database rebuild. Exiting. 2014-11-24 03:06:39 Shutdown: In progress… 2014-11-24 03:06:39 StopNode() 2014-11-24 03:06:39 Shutdown: done

    Not sure if recovery from this dialoge is supposed to be possible.

    Swapping between prune/not-pruned state is what seems to have erratic behavior.

  98. rebroad commented at 6:07 am on November 24, 2014: contributor
    Does this pull cause random blocks to be deleted or the oldest blocks? Hopefully the former!
  99. gmaxwell commented at 6:42 am on November 24, 2014: contributor
    Not hopefully the former. Random is quite pessimal, and in the current setup of the system the best behavior (that which maximizes utility given limited space) is to remove the oldest first. The behavior will change (e.g. to determinstically random segments) when there are facilities for actually make use of different behavior.
  100. rdponticelli force-pushed on Nov 26, 2014
  101. rdponticelli commented at 6:41 pm on November 26, 2014: contributor
    @instagibbs Are you sure you answered yes to the question on bitcoin-qt? Working fine here. The issue could be with bitcoind which doesn’t have a way of receiving the input. You need a -reindex there to go back to full node.
  102. instagibbs commented at 6:56 pm on November 26, 2014: member
    @rdponticelli that would explain it. I was running bitcoind only. I guess as a bitcoind user I would have expected a message telling me to reindex. But that might be out of scope for this. Thanks.
  103. rebroad commented at 1:06 am on November 27, 2014: contributor
    @gmaxwell surely if everyone pruned the oldest blocks, this would be like sharing a video on bittorrent where no-one has the first part of the video…
  104. gmaxwell commented at 1:24 am on November 27, 2014: contributor

    @rebroad sure, and there is no risk of that currently (this disables node network and the wallet). Not deleting old blocks wouldn’t help here, because without extensions to the p2p protocol sparse block possession is not useful, any missing blocks makes the node no longer node-network.

    Please go read the old discussion about this, back to pieter’s first pruning posts on bitcoin development.

  105. laanwj commented at 8:47 am on November 27, 2014: member
    This is different from a video on bittorrent. The demand is not uniform over the block chain. The recent blocks are requested most frequently, the early blocks are only used when a new node wants to catch up which happens less often. So it makes sense to have nodes that can serve recent history only. Of course, that will break if everyone does it, but let’s not wield the categorical imperative here.
  106. rdponticelli force-pushed on Dec 5, 2014
  107. luke-jr commented at 10:11 am on December 24, 2014: member
    I merged this into 0.10.0rc1 and built binaries so others can help test: http://luke.dashjr.org/programs/bitcoin/files/bitcoind/misc/0.10.x/0.10.0rc1.autoprune/
  108. Michagogo commented at 10:13 am on December 24, 2014: contributor

    Why are they named 0.10.0rc2?

    On Wed, Dec 24, 2014 at 12:12 PM, Luke-Jr notifications@github.com wrote:

    I merged this into 0.10.0rc1 and built binaries so others can help test: http://luke.dashjr.org/programs/bitcoin/files/bitcoind/misc/0.10.x/0.10.0rc1.autoprune/

    — Reply to this email directly or view it on GitHub #4701 (comment).

  109. luke-jr commented at 10:13 am on December 24, 2014: member
    Typo, will rename.
  110. TheBlueMatt commented at 4:27 pm on December 24, 2014: member
    Has anyone tried this with a low prune limit and #5422? Last I checked it crashed and burned pretty quick. Also, anything that lets you make your node unable to reorg back at least a week is entirely unacceptable as far as I’m concerned.
  111. luke-jr commented at 9:09 pm on December 24, 2014: member
    I would expect handling any reorg to be a requirement, even if the node has to begin reindexing from scratch…
  112. gmaxwell commented at 9:13 pm on December 24, 2014: contributor
    I was willing to look the other way for 0.10, but since we missed it and the headers-first work should have made it easier. I do believe that we should correctly respond to a too-long reorg by going into a rebuild.
  113. welshjf commented at 5:33 am on January 2, 2015: contributor
    I volunteer to help testing, given instructions. I’ve been running a full node for a while but I’m not familiar with the code. Linux or Win7 x64.
  114. laanwj commented at 7:39 pm on January 6, 2015: member
    I think it makes sense to move this forward soon by merging this in some form, as long as there are no regressions, it can be improved later. Or alternatively, @rdponticelli are there preparatory changes that can be merged already to make keeping this rebased easier?
  115. rdponticelli commented at 2:37 pm on January 10, 2015: contributor
    @laanwj: #4515 would certainly help. I’ll rebase this one soon.
  116. Refactor startup checks onto a separate function. 35752e58cc
  117. Split the logic to check if a block file can be opened to a separate function. 63ecce0542
  118. Add undo data to checks to be sure all required info can be opened. 972c5d72ba
  119. Scan active chain instead of map of blocks. cb883f0e4c
  120. Accept pruned blocks when loading the database. 1339b7bbbe
  121. Add a switch to allow running in a pruned state. 
    These are the main functional changes on this state:

* Do not allow running with a wallet or txindex.
* Check for data at startup is mandatory only up to the last 288 blocks.
* NODE_NETWORK flag is unset.
* Requests for pruned blocks from other peers is answered with "notfound" and they are disconnected, not to stall their IBD.
    c974c57025
  122. Add a counter to log the range of pruned blocks. 41c15691ce
  123. Implement autoprune. 
    This mode introduces a configuration parameter to keep block files at less than a fixed amount of MiB.
    c12d6e5e15
  124. Simplify CheckBlockFiles routine. 
    We can do it now that the logic to avoid opening the files several times has been
moved to their own functions and is handled mainly through variables.
    bbb769c5c1
  125. rdponticelli force-pushed on Jan 12, 2015
  126. rdponticelli commented at 7:03 pm on January 12, 2015: contributor
    Rebased.
  127. in src/main.cpp: in bbb769c5c1 
    3530-                            // however we MUST always provide at least what the remote peer needs
3531-                            typedef std::pair<unsigned int, uint256> PairType;
3532-                            BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn)
3533-                                if (!pfrom->setInventoryKnown.count(CInv(MSG_TX, pair.second)))
3534-                                    pfrom->PushMessage("tx", block.vtx[pair.first]);
3535+                    if (!ReadBlockFromDisk(block, (*mi).second)) {
    theuni commented at 9:19 pm on January 12, 2015:

    It seems this doesn’t make the distinction between missing a pruned block and a failed read. If a non-pruned block fails to read when pruning is enabled, shouldn’t we fail as before?

    Alternatively.. couldn’t the pruning check happen before ReadBlockFromDisk(), to avoid the overhead entirely for pruned nodes? If we’re comfortable with randomly answering with a notfound, why not do it constantly?

    sipa commented at 8:41 pm on February 16, 2015:
    There is a distinction in the expectation of what a node does. If you enable pruning, the node does not promise to the network to behave as a full node, so it’s fine to not answer. If a node advertizes as NODE_NETWORK, and can’t answer a request for a block, it’s buggy.
  128. shivaenigma commented at 12:47 pm on January 29, 2015: none
    Testing this from https://github.com/luke-jr/bitcoin/tree/0.10.0rc3.autoprune Just curious on what is the desired behaviour of -reindex when run on already pruned blocks ?
  129. Michagogo commented at 1:48 pm on January 29, 2015: contributor

    I would assume reindexing would force it to redownload all the blocks from scratch.

    On Thursday, January 29, 2015, shivaenigma notifications@github.com wrote:

    Testing this from https://github.com/luke-jr/bitcoin/tree/0.10.0rc3.autoprune Just curious on what is the desired behaviour of -reindex when run on already pruned blocks ?

    — Reply to this email directly or view it on GitHub #4701 (comment).

  130. mrbandrews commented at 7:59 pm on January 29, 2015: contributor

    Hi. I’ve been testing this also (building from source) and I think the latest commit may have re-introduced the issue of re-opening a block and undo file for each block in the active chain. Thus, on testnet (about 320k blocks) each call to CheckBlockFiles results in 640k calls to the file system. I know that @rdponticelli has a separate PR (#4515) which appears to still have the code which should prevent this (using setRequiredDataFilesAreOpenable) - and which autoprune may eventually be built on top of.

    It seems from the comment on the last commit that it was intended that this check was moved into a different function, but if so, it doesn’t seem to be working as intended?

  131. ghost commented at 7:35 pm on February 1, 2015: none
    This has been tagged as v0.11. What time frame is that indicative of?
  132. gmaxwell commented at 8:46 pm on February 1, 2015: contributor
    @21E14 presumably and hopefully in the next couple months. Right now much attention is focused on getting 0.10 out (as it should be), after that you should expect to see more attention on getting this merged from the rest of the contributors.
  133. laanwj commented at 8:56 pm on February 1, 2015: member

    @21E14 July 2015 is the time frame for 0.11. The tag is no guarantee that it will make it into that release though, but just a reminder. If it isn’t ready to merge well before 0.11’s release date it will be bumped to 0.12.

    You can help by testing and reviewing the code.

  134. ghost commented at 0:29 am on February 2, 2015: none

    @gmaxwell @laanwj I’m assuming a few minor releases in-between?

    This PR is looking pretty good so far. Running the daemon though, just for kicks, with the prune option set to less than 300 MiB results in the following awkward message:

    AppInit2 : parameter interaction: -prune -> setting -disablewallet=1 Autoprune configured below the minimum of 300MiB. Setting at the maximum possible of 17592186044415MiB, to avoid pruning too much. Please, check your configuration.

    More to the point, why even let a ‘misconfigured prune’ carry on?

  135. shivaenigma commented at 5:40 pm on February 4, 2015: none

    Did more testing from https://github.com/luke-jr/bitcoin/tree/0.10.0rc3.autoprune Tested using -prune=300 and blocks are getting deleted

    But sometimes size of .bitcoin/blocks is more than much more than 300 2015-02-04 17:34:59 Data for blocks from 1 to 184525 has been pruned 2015-02-04 17:34:59 Undo data for blocks from 1 to 184525 has been pruned du -sh ~/.bitcoin/blocks/ is 496MB

    Switching from pruned mode to nonpruned mode causes this: Error checking required block files. There must be missing or unreadable data. Do you want to rebuild the block database now?

  136. Michagogo commented at 6:29 pm on February 4, 2015: contributor
    @shivaenigma What’s the size of the index/ directory? Perhaps that’s the rest. I don’t know if the index shrinks with a pruned node. A couple hundred megabytes is insignificant compared to 30+ GB, but indeed, with pruned nodes like this that does become a factor. And at the end, do you mean switching to non-pruned mode? In that case, yes, of course you’re missing data – you’ve just deleted most of the blockchain!
  137. shivaenigma commented at 6:53 pm on February 4, 2015: none

    @Michagogo size of my blocks/index/ 34MB. I think the parameter -pruned=300MB is misleading, even if after pruning the size end of 496MB . How does the error scale, so if I set 2GB will it take 2.1GB or 3GB

    so I guess its checking all the blocks at startup on non pruned mode and throws an error. I think there should be way to disable this check. Because now I can never switch from pruned mode to nonpruned mode even if I dont care about missing inital blocks

  138. Michagogo commented at 7:06 pm on February 4, 2015: contributor

    Uh, what? By definition, non-pruned means that you have the entire blockchain. So yes, you do need to redownload it. It would be nice, though, if it were smart enough to make the switch gracefully and just fill in the history, with the headers-first mechanism. If you mean you don’t want to prune more, you can just set the threshold to something high (100000000 or whatever) and you’ll be covered for the foreseeable future.

    On Wed, Feb 4, 2015 at 8:54 PM, shivaenigma notifications@github.com wrote:

    @Michagogo https://github.com/Michagogo size of my blocks/index/ 34MB. I think the parameter -pruned=300MB is misleading, even if after pruning the size end of 496MB . How does the error scale, so if I set 2GB will it take 2.1GB or 3GB

    so I guess its checking all the blocks at startup on non pruned mode and throws an error. I think there should be way to disable this check. Because now I can never switch from pruned mode to nonpruned mode even if I dont care about missing inital blocks

    — Reply to this email directly or view it on GitHub #4701 (comment).

  139. shivaenigma commented at 6:07 am on February 5, 2015: none

    If you mean you don’t want to prune more, you can just set the threshold to something high (100000000 or whatever) and you’ll be covered for the foreseeable future

    Yes this is actually what I wanted . Thanks

  140. in src/init.cpp: in bbb769c5c1 
    694+    if (nPrune) {
695+        if (nPrune >= MIN_BLOCK_FILES_SIZE)
696+            LogPrintf("Autoprune configured to use less than %uMiB on disk for block files.\n", nPrune / 1024 / 1024);
697+        else {
698+            nPrune = ~0;
699+            LogPrintf("Autoprune configured below the minimum of %uMiB. Setting at the maximum possible of %uMiB, to avoid pruning too much. Please, check your configuration.\n", MIN_BLOCK_FILES_SIZE / 1024 / 1024, nPrune / 1024 / 1024);
    sipa commented at 8:45 pm on February 16, 2015:
    I think it’s more clear if you just say something like “Leaving pruned mode enabled, but not deleting any more blocks for now”.
  141. in src/main.cpp: in bbb769c5c1 
    2849+    if (setDataFilePrunable.empty() && setUndoFilePrunable.empty()) {
2850+        LogPrintf("There's nothing to prune.\n");
2851+        return false;
2852+    }
2853+    bool fFileRemoved = false;
2854+    int dataPrunable = *setDataFilePrunable.begin(), undoPrunable = *setUndoFilePrunable.begin();
    sipa commented at 8:49 pm on February 16, 2015:
    Is it guaranteed that both setDataFilePrunable and setUndoFilePrunable are both non-empty at this stage? The test above only guarantees that at least one of them is non-empty. Don’t dereference begin() of an empty set.
  142. in src/main.cpp: in bbb769c5c1 
    2978@@ -2886,7 +2979,7 @@ bool static LoadBlockIndexDB()
2979     {
2980         CBlockIndex* pindex = item.second;
2981         pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex);
2982-        if (pindex->nStatus & BLOCK_HAVE_DATA) {
2983+        if (pindex->nStatus & BLOCK_HAVE_DATA || pindex->nStatus & BLOCK_VALID_CHAIN) {
    sipa commented at 11:16 pm on February 16, 2015:
    This may lose never-fully-connected branches. I think this condition as a whole should just change to (pindex->nStatus & BLOCK_VALID_TRANSACTIONS && pindex->nTx != 0).
  143. in src/main.cpp: in bbb769c5c1 
    3118+                    pindex->nStatus &= ~BLOCK_HAVE_UNDO;
3119+                    setDirtyBlockIndex.insert(pindex);
3120+                }
3121+            }
3122+        }
3123+        if (~pindex->nStatus & BLOCK_HAVE_DATA && pindex->nStatus & BLOCK_VALID_CHAIN)
    sipa commented at 11:20 pm on February 16, 2015:
    This shouldn’t require BLOCK_VALID_CHAIN anymore; I think BLOCK_VALID_TRANSACTIONS is enough.
  144. in src/main.cpp: in bbb769c5c1 
    3087+    int nKeepMinBlksFromHeight = nPrune ? (max((int)(chainActive.Height() - MIN_BLOCKS_TO_KEEP), 0)) : 0;
3088+    LogPrintf("Checking all required data for active chain is available (mandatory from height %i to %i)\n", nKeepMinBlksFromHeight, max(chainActive.Height(), 0));
3089+    set<int> setDataPruned, setUndoPruned;
3090+    setDataFilePrunable.clear();
3091+    setUndoFilePrunable.clear();
3092+    for (CBlockIndex* pindex = chainActive.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) {
    sipa commented at 11:57 pm on February 16, 2015:

    This will not iterate over side branches of the block chain, and may perhaps miss things that way. The chances for that are really low, as this code still checks the last height in each affected file, and then deletes whole files, so unless a file consists solely of side branch blocks, it will still be processed.

    However, I think this flaw indicates a conceptual mistake: this function should iterate over files, rather than over blocks. That would be faster too. The concistency check can be done on blocks separately, but only needs to be done for the last blocks I guess?

  145. sipa commented at 12:21 pm on March 1, 2015: member
    Do you plan to work on this any more in the future? If not, I may try to maintain/update it.
  146. laanwj commented at 8:31 am on March 9, 2015: member
    Closing in favor of #5863
  147. laanwj closed this on Mar 9, 2015
  148. MarcoFalke locked this on Sep 8, 2021


rdponticelli gmaxwell sipa TheBlueMatt luke-jr Diapolo laanwj ghost BitcoinPullTester rebroad jonasschnelli Michagogo gavinandresen instagibbs welshjf theuni shivaenigma mrbandrews

Labels
Feature

Milestone
0.11.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me