diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2024-05-16 11:15:56 -0400 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2024-05-16 11:16:08 -0400 |
commit | 2f53f2273da020d7fabd7c65a1bc7e69a31249b2 (patch) | |
tree | 7054487407d21b6a503665dbcaf32dcdaf67cb28 /test | |
parent | 75118a608fc22a57567743000d636bc1f969f748 (diff) | |
parent | e41667b720372dae8438ea86e9819027e62b54e0 (diff) | |
download | bitcoin-2f53f2273da020d7fabd7c65a1bc7e69a31249b2.tar.xz |
Merge bitcoin/bitcoin#29975: blockstorage: Separate reindexing from saving new blocks
e41667b720372dae8438ea86e9819027e62b54e0 blockstorage: Don't move cursor backwards in UpdateBlockInfo (Ryan Ofsky)
17103637c6fa2dfcf5374ebb0cd715e540dd4ce1 blockstorage: Rename FindBlockPos and have it return a FlatFilePos (Martin Zumsande)
d9e477c4dc39d9623ed66c35c06e28f94ae62ad5 validation, blockstorage: Separate code paths for reindex and saving new blocks (Martin Zumsande)
064859bbad6984a6ec85c744064abdf757807c58 blockstorage: split up FindBlockPos function (Martin Zumsande)
fdae638e83522c28a1222e65c43d1cbca3e34cba doc: Improve doc for functions involved in saving blocks to disk (Martin Zumsande)
0d114e3cb20cb9e03fc9ba8daf3d03436b491742 blockstorage: Add Assume for fKnown / snapshot chainstate (Martin Zumsande)
Pull request description:
`SaveBlockToDisk` / `FindBlockPos` are used for two purposes, depending on whether they are called during reindexing (`dbp` set, `fKnown = true`) or in the "normal" case when adding new blocks (`dbp == nullptr`, `fKnown = false`).
The actual tasks are quite different
- In normal mode, preparations for saving a new block are made, which is then saved: find the correct position on disk (maybe skipping to a new blk file), check for available disk space, update the blockfile info db, save the block.
- during reindex, most of this is not necessary (the block is already on disk after all), only the blockfile info needs to rebuilt because reindex wiped the leveldb it's saved in.
Using one function with many conditional statements for this leads to code that is hard to read / understand and bug-prone:
- many code paths in `FindBlockPos` are conditional on `fKnown` or `!fKnown`
- It's not really clear what actually needs to be done during reindex (we don't need to "save a block to disk" or "find a block pos" as the function names suggest)
- logic that should be applied to only one of the two modes is sometimes applied to both (see first commit, or #27039)
#24858 and #27039 were recent bugs directly related to the differences between reindexing and normal mode, and in both cases the simple fix took a long time to be reviewed and merged.
This PR proposes to clean this code up by splitting out the reindex logic into a separate function (`UpdateBlockInfo`) which will be called directly from validation. As a result, `SaveBlockToDisk` and `FindBlockPos` only need to cover the non-reindex logic.
ACKs for top commit:
paplorinc:
ACK e41667b720372dae8438ea86e9819027e62b54e0
TheCharlatan:
Re-ACK e41667b720372dae8438ea86e9819027e62b54e0
ryanofsky:
Code review ACK e41667b720372dae8438ea86e9819027e62b54e0. Just improvements to comments since last review.
Tree-SHA512: a14ff9a0facf6b1e3c1cd724a2d19a79a25d4b48de64398fdd172671532a472bc10a20cbb64ac3a3e55814dcc877d0597a3e1699cabc4f9d9a86b439b6eaba20
Diffstat (limited to 'test')
0 files changed, 0 insertions, 0 deletions