Improve chainstate/blockindex disk writing policy #5241
Conversation
|
Added wallet tip updating and block/undo file flushing to the FlushStateToDisk() method. Very preliminary testing on my VPS (with very slow I/O) shows a ~100ms decrease in block processing time. |
sipa
force-pushed
the
noflush
branch
2 times, most recently
from
46cec70 to
56e8308
Compare
src/main.cpp
Outdated
There was a problem hiding this comment.
Parametrization on multiple bool parameters leads to hard-to-read code; I'd prefer to pass an enum, or bit field
There was a problem hiding this comment.
Agree, I'll add a commit for changing it.
|
Concept ACK, will test |
|
Found a small issue. When AppInit2 exits prematurely, bitcoind will crash in shutdown with the following: You need a check |
|
Fixed. |
|
What about the |
|
@laanwj Ah right, it does. That's sort of inconvienient but inevitable until there's a way to iterate the UTXO entries in a CCoinsView. Writing pcoinsTip to disk means a reference to the tip block hash in the index, so that needs to be written, which on itself refers to disk positions, so blocks need to be written too... |
|
Fixed. |
|
Tested by kill -KILL'ing a node while it was flushing. No problems. |
|
This seems to greatly improves our handling of unclean shutdowns during the IBD. Previously we'd reliably corrupt the database, requring the user to manually delete things. I've tested this with a bunch of killing while syncing and not been able to break it. |
|
I performed a thorough synchronization of a testnet node from scratch using this patch, sending term or kill signals randomly every 180-640 seconds. It worked like a charm. Such test easily triggers #5156 without it. |
|
I think we should seriously consider this for 0.10. It's a big reliablity improvement during the initial sync, beyond its performance implications. |
|
I've subjected this to some horrible crashes, and a full sync w/ DEBUG_LOCKORDER. ACK commithash 9dd92a12fe92b4a81e612acf1c6245b85fa12a73 for 0.10. |
There are 3 pieces of data that are maintained on disk. The actual block and undo data, the block index (which can refer to positions on disk), and the chainstate (which refers to the best block hash). Earlier, there was no guarantee that blocks were written to disk before block index entries referring to them were written. This commit introduces dirty flags for block index data, and delays writing entries until the actual block data is flushed. With this stricter ordering in writes, it is now safe to not always flush after every block, so there is no need for the IsInitialBlockDownload() check there - instead we just write whenever enough time has passed or the cache size grows too large. Also updating the wallet's best known block is delayed until this is done, otherwise the wallet may end up referring to an unknown block. In addition, only do a write inside the block processing loop if necessary (because of cache size exceeded). Otherwise, move the writing to a point after processing is done, after relaying.
|
Sorry, needed to rebase after BIP23 block proposals. |
|
ACK. |
4 participants
There are 3 pieces of data that are maintained on disk. The actual block and undo data, the block index (which can refer to positions on disk), and the chainstate (which refers to the best block hash).
Earlier, there was no guarantee that blocks were written to disk before block index entries referring to them were written. This commit introduces dirty flags for block index data, and delays writing it until the actual block data is flushed.
With this stricter ordering in writes, it is now safe to not always flush after every block, so there is no need for the IsInitialBlockDownload() check there - instead we just write whenever enough time has passed or the cache size grows too large.
In addition, only do a write inside the block processing loop if necessary (because of cache size exceeded). Otherwise, move the writing to a point after processing is done, after relaying.
This should improve block relay speed, by not writing chainstate and index entries before announcing the new hash.
Future work: move block and undo data writing into FlushStateToDisk, and move it to a background thread.