HomePhabricator

blockstorage: add return codes on flush functions, force callers to handle them

Description

blockstorage: add return codes on flush functions, force callers to handle them

Summary:

blockstorage: Mark FindBlockPos as nodiscard

A false return value indicates a fatal error (disk space being too low),
so make sure we always consume this error code.

This commit is part of an ongoing process for making the handling of
fatal errors more transparent and easier to understand.

blockstorage: Log on fatal block file flush error

By returning an error code if FlushBlockFile fails, the caller now has
to explicitly handle block file flushing errors. Before this change
such errors were non-explicitly ignored without a clear rationale.

Prior to this patch FlushBlockFile may have failed silently in
Chainstate::FlushStateToDisk. Improve this with a log line. Also add a
TODO comment to flesh out whether returning early in the case of an
error is appropriate or not. Returning early might be appropriate to
prohibit WriteBlockIndexDB from writing a block index entry that does
not refer to a fully flushed block.

Besides Chainstate::FlushStateToDisk, FlushBlockFile is also called
by FindBlockPos. Don't change the abort behavior there, since we don't
want to fail the function if the flushing of already written blocks
fails. Instead, just document it.

blockstorage: Log on fatal undo file flush error

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 does not change the way flush errors are handled other than by adding log prints and comments to callsites. This makes ignoring the Flush errors more explicit, and will force potential future callsites to think about how to handle these errors.

This is a backport of core#27866

Test Plan: ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D17851

Details

Provenance
TheCharlatan <seb.kung@gmail.com>Authored on Jul 25 2023, 09:12
PiRKCommitted on Mon, Mar 24, 19:27
PiRKPushed on Mon, Mar 24, 19:27
Reviewer
Restricted Project
Differential Revision
D17851: blockstorage: add return codes on flush functions, force callers to handle them
Parents
rABC85e069a96145: [e.cash] Add Kryptex mining pool to e.cash/mining page
Branches
Unknown
Tags
Unknown