New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add autoprune functionality #5863
Conversation
// for the most work chain if we come across them; we can't switch | ||
// to a chain unless we have all the non-active-chain parent blocks. | ||
bool fFailedChain = pindexTest->nStatus & BLOCK_FAILED_MASK; | ||
if (fFailedChain || !(pindexTest->nStatus & BLOCK_HAVE_DATA)) { | ||
// Candidate has an invalid ancestor, remove entire chain from the set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment is outdated now.
Looks good, overall. I wouldn't worry about the inefficient mapBlockIndex iteration for now, that can be improved later. I haven't tested yet. |
@@ -200,6 +201,7 @@ class CTestNetParams : public CMainParams { | |||
nMinerThreads = 0; | |||
nTargetTimespan = 14 * 24 * 60 * 60; //! two weeks | |||
nTargetSpacing = 10 * 60; | |||
nAutopruneAfterHeight = 1000; // low since testnet can be reset in future |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use //! so it will get picked up by doxygen
We've added 6 new commits. |
Addressed some of the feedback and made a few other slight improvements. |
v0.9.0rc2-2621-g40ba177. Bootstrapping with "prune=1000", (and some minor added debug prints which shouldn't have affected anything, but never know): bitcoind: coins.cpp:150: virtual bool CCoinsViewCache::BatchWrite(CCoinsMap&, const uint256&): Assertion `it->second.flags & CCoinsCacheEntry::FRESH' failed. Unf. didn't have ulimit -c unlimited. Rerunning, it seems be gathering blocks like normal... |
Hey Rusty, You don't happen to have the debug.log laying around from when it crashed, Thanks! On Wed, Mar 25, 2015 at 7:21 AM, Rusty Russell notifications@github.com
|
Also, what version are you running? If you're running this pull, you should see a version string like this:
The "v0.9.0rc2-2621-g40ba177" that appears in your comment references a version that is quite old. Also, did you update your comment after the fact? For some reason, github never included that part in the original email. Thanks! |
https://github.com/sdaftuar/bitcoin/tree/autoprune == 40ba177 which is what I was running (and, AFAICT, what is proposed in this pull request). Unf. I didn't save debug.log :( |
Hmm, next run got killed by the OOM killer. Perhaps this first assert() was due to OOM (512M mem, 512M swap). |
I see an 0.9.0rc2 in the version string you posted. Did you back port or On Wed, Mar 25, 2015, 8:35 PM Rusty Russell notifications@github.com
|
Neither. git describe uses the previous tag it knows about. For some reason it doesn't know about more recent tags. But I'm on commit 40ba177. The good news: it happened again! bitcoind: coins.cpp:150: virtual bool CCoinsViewCache::BatchWrite(CCoinsMap&, const uint256&): Assertion `it->second.flags & CCoinsCacheEntry::FRESH' failed. Here's the last 100 lines of debug.log (can't figure out how to attach the whole thing): 2015-03-26 01:12:46 UpdateTip: new best=000000000000002a45bf369333c2daa0555e8e6f064cbd6bd1ff1a5d06980d82 height=256222 log2_work=71.679906 tx=23298335 date=2013-09-05 08:23:11 progress=0.164551 cache=48753 |
Do you have the head of this log where it prints the version string? core
|
Also, my idea of a "canonical environment" is a commodity x86_64 based PC Also, as mentioned before, core files, backtraces and full debug logs are Thanks!
|
Got a core file, but backtrace was useless without debugging info :( So I've reconfigured with --enable-debug (and --disable-wallet). Seems repeatable, so I'll expect something soon. If not, I'll erase the blockchain and try again. Hardware: Setup: The following diff was applied, but I've now unapplied it to make doubly-sure: diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 6e0f7e9..2645baf 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -523,14 +523,29 @@ void CTxMemPool::removeForBlock(const std::vector<CTransaction>& vtx, unsigned i
BOOST_FOREACH(const CTransaction& tx, vtx)
{
uint256 hash = tx.GetHash();
- if (mapTx.count(hash))
+ if (mapTx.count(hash)) {
+ // Record a TX we already knew.
+ std::cerr << nBlockHeight << ":mutual:" << tx.GetHash().ToString() << endl;
entries.push_back(mapTx[hash]);
+ } else {
+ std::cerr << nBlockHeight << ":new:" << tx.GetHash().ToString() << endl;
+ }
}
minerPolicyEstimator->seenBlock(entries, nBlockHeight, minRelayFee);
BOOST_FOREACH(const CTransaction& tx, vtx)
{
std::list<CTransaction> dummy;
remove(tx, dummy, false);
+ }
+
+ typedef std::map<uint256, CTxMemPoolEntry> maptx_type;
+ BOOST_FOREACH(const maptx_type::value_type& txpair, mapTx)
+ {
+ std::cerr << nBlockHeight << ":mempool-only:" << txpair.first.ToString() << endl;
+ }
+
+ BOOST_FOREACH(const CTransaction& tx, vtx)
+ {
removeConflicts(tx, conflicts);
ClearPrioritisation(tx.GetHash());
} |
|
I don't see any immediate problems with the diff, but I agree, better safe Thanks a lot!
|
Can this be rebased? |
$ tail bitcoind-out-4 Thread 11 (Thread 0xaef05b70 (LWP 31697)): Thread 10 (Thread 0xb611bb70 (LWP 31657)): Thread 9 (Thread 0xb466ab70 (LWP 31663)): Thread 8 (Thread 0xadf03b70 (LWP 31699)): #1 0xb6c24733 in pthread_cond_timedwait@@GLIBC_2.3.2 () ---Type to continue, or q to quit--- Thread 6 (Thread 0xaf706b70 (LWP 31696)): Thread 5 (Thread 0xaff07b70 (LWP 31695)): Thread 4 (Thread 0xb591ab70 (LWP 31658)): Thread 3 (Thread 0xb691cb70 (LWP 31656)):
Thread 2 (Thread 0xb6a956d0 (LWP 31655)): Thread 1 (Thread 0xae704b70 (LWP 31698)): |
Thanks a lot for your debugging help. I think we found the problem. If we prune inside ConnectBlock (as undo files allocate more space) then we flush pcoinsTip, which breaks the assumptions made inside flushing the child cache after ConnectBlock is finished. Will work on a solution. |
// would cause Autoprune to overstate disk usage (see related | ||
// comment in FindBlockPos). | ||
Autoprune(nNewChunks * UNDOFILE_CHUNK_SIZE - vinfoBlockFile[nFile].nUndoSize); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the meantime, you could try commenting out this call to Autoprune. It should still keep you very close to the target and ought to fix the problem.
Thanks, have commented that out and re-running now. |
@jonasschnelli , re: pruning.py failing. Were you running it on a decently equipped machine? What happens is the reorg's are so large that they take a long time, and then the rpc calls (such as to getblocks() in this case) time out. I increased the timeout from 30s to 5min, which gave some buffer on my machine, but perhaps that wasn't enough for general usage. If you wouldn't mind trying increasing that timewait variable a little bigger and seeing if it completes, it might take a very long time though. I would say the test is so cumbersome its of questionable value, except we've found it quite useful in recent weeks to test unrelated behavior because it stresses the nodes so much. The test will have to be modified anyway if #5943 gets merged, which could make at least the reorgs faster. |
@morcos The machine should be sufficient: Intel Xeon E31245 3.30GHz, 16GB RAM. |
@morcos: prune.py test still fails: https://gist.github.com/jonasschnelli/03afc81383ec537eb4d2 |
@morcos: after changing My fullnode with autoprune mode is running smooth sind 2.5days. |
Concept ACK. Code review ACK. Only lightly tested, sorry. |
This needed to be rebased, so I did so and updated the pull. I also added a commit to try to reduce surprise in the event that someone uses -reindex and -prune at the same time: now, we only will delete all the block files if block file 0 is missing: in that case, we know that reindex will not find the files anyway, so there is no harm in deleting them (and this way a pruning node isn't tying up space used by old, inaccessible files). |
Ok, I've run out of nits (see above). I think this is safe to merge - especially in case pruning itself is not enabled. |
@sipa Thanks for all the review, I've gone ahead and fixed those nits and squashed everything back down to one commit. |
This adds a -prune=N option to bitcoind, which if set to N>0 will enable block file pruning. When pruning is enabled, block and undo files will be deleted to try to keep total space used by those files to below the prune target (N, in MB) specified by the user, subject to some constraints: - The last 288 blocks on the main chain are always kept (MIN_BLOCKS_TO_KEEP), - N must be at least 550MB (chosen as a value for the target that could reasonably be met, with some assumptions about block sizes, orphan rates, etc; see comment in main.h), - No blocks are pruned until chainActive is at least 100,000 blocks long (on mainnet; defined separately for mainnet, testnet, and regtest in chainparams as nPruneAfterHeight). This unsets NODE_NETWORK if pruning is enabled. Also included is an RPC test for pruning (pruning.py). Thanks to @rdponticelli for earlier work on this feature; this is based in part off that work.
return; | ||
} | ||
|
||
unsigned int nLastBlockWeMustKeep = chainActive.Tip()->nHeight - MIN_BLOCKS_TO_KEEP; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable name is slightly confusing. It's the last height that's allowed to be pruned (the test below using it is correct).
ACK |
Works for me - did full testnet and mainnet sync, which succeeded, and disk usage remained within the configured bound. Also tried various RPCs on a pruned node and was unable to crash it. Tested ACK. |
f9ec3f0 Add block pruning functionality (mrbandrews)
Post-pull ACK; copied my -txindex=1 main net datadir, ran pruned (it told me I had to -reindex, as expected), running pruned nicely on my Mac. |
I have a question: |
We should work on that, yes.
|
I have a question also: Does the -prune=N option work from a cold start, or does it require a full sync first? I recently installed bitcoind on a 40GB droplet with -prune=30000 and it keeps running out of disk space. I have tried with the prune option as a cli argument to bitcoind, and also as an option in the config file. Cheers |
@dacox That should work. |
According to I grabbed it from the PPA, and I saw that this PR was merged in before the release was tagged. However, I do not see a reference to -prune in the help output Edit: I see the problem, the 0.10 branch was branched off of master quite some time before this PR. For some reason I thought it was in 0.10 |
Could this be improved by keeping the headers of pruned blocks and somehow marking those blocks as pruned? That would allow distinguishing the difference between pruned and unknown blocks. |
Continuing the work of @rdponticelli, this is an implementation of the autoprune functionality (see #4701). Thanks to @rdponticelli for doing a lot of prior work on this topic; we used his work and the discussion of it as our starting point (though we found it easier to base this pull off of master). @mrbandrews, @morcos, @ajweiss, and @sdaftuar collaborated on this pull.
To summarize autoprune: this adds a -prune=N option to bitcoind, which if set to N>0 will enable autoprune. When autopruning is enabled, block and undo files will be deleted to try to keep total space used by those files to below the prune target (N, in MB) specified by the user, subject to some constraints:
We’ve attempted to address the comments raised in the prior conversations; here are a few things worth noting:
(Though this pull still needs more work to address some of the above items, we thought we'd open it now to keep review momentum on this feature moving...)