Change Details

> 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 is a backport of [[https://github.com/bitcoin/bitcoin/pull/27866 | core#27866]]

> 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 [[https://github.com/bitcoin/bitcoin/pull/27866 | core#27866]]

> 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 [[https://github.com/bitcoin/bitcoin/pull/27866 | core#27866]]