foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-04 13:55:23 -05:00
Code Issues Activity

blockstorage: Return on fatal undo file flush error

Browse source 
By returning an error code if either `FlushUndoFile` or `FlushBlockFile`
fail, the caller now has to explicitly handle block undo file flushing
errors. Before this change such errors were non-explicitly ignored
without a clear rationale.

Besides the call to `FlushUndoFile` in `FlushBlockFile`, ignore its
return code at its call site in `WriteUndoDataForBlock`. There, a failed
flush of the undo data should not be indicative of a failed write.

Add [[nodiscard]] annotations to `FlushUndoFile` such that its return
value is not just ignored in the future.
This commit is contained in:
TheCharlatan 2023-07-25 12:03:26 +02:00
parent f0207e0030
commit d8041d4e04
No known key found for this signature in database
GPG key ID: 9B79B45691DB4173
2 changed files with 20 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/node/blockstorage.cpp
View file

@ -538,12 +538,14 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in
return true;
}
void BlockManager::FlushUndoFile(int block_file, bool finalize)
bool BlockManager::FlushUndoFile(int block_file, bool finalize)
{
FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize);
if (!UndoFileSeq().Flush(undo_pos_old, finalize)) {
m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error.");
return false;
}
return true;
}
bool BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)
@ -567,7 +569,11 @@ bool BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)
}
// we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks,
// e.g. during IBD or a sync after a node going offline
if (!fFinalize || finalize_undo) FlushUndoFile(m_last_blockfile, finalize_undo);
if (!fFinalize || finalize_undo) {
if (!FlushUndoFile(m_last_blockfile, finalize_undo)) {
success = false;
}
}
return success;
}
@ -764,7 +770,14 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
// with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in
// the FindBlockPos function
if (_pos.nFile < m_last_blockfile && static_cast<uint32_t>(block.nHeight) == m_blockfile_info[_pos.nFile].nHeightLast) {
FlushUndoFile(_pos.nFile, true);
// Do not propagate the return code, a failed flush here should not
// be an indication for a failed write. If it were propagated here,
// the caller would assume the undo data not to be written, when in
// fact it is. Note though, that a failed flush might leave the data
// file untrimmed.
if (!FlushUndoFile(_pos.nFile, true)) {
LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", _pos.nFile);
}
} else if (_pos.nFile == m_last_blockfile && static_cast<uint32_t>(block.nHeight) > m_undo_height_in_last_blockfile) {
m_undo_height_in_last_blockfile = block.nHeight;
}

6
src/node/blockstorage.h
View file

@ -92,10 +92,12 @@ private:
bool LoadBlockIndex()
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Return false if block file flushing fails. */
/** Return false if block file or undo file flushing fails. */
[[nodiscard]] bool FlushBlockFile(bool fFinalize = false, bool finalize_undo = false);
void FlushUndoFile(int block_file, bool finalize = false);
/** Return false if undo file flushing fails. */
[[nodiscard]] bool FlushUndoFile(int block_file, bool finalize = false);
[[nodiscard]] bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown);
bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize);