Skip to content

Commit 7320db9

Browse files
committed
kernel: Add flushError method to notifications
This is done in addition with the following commit. Both have the goal of getting rid of direct calls to AbortNode from kernel code. This extra flushError method is added to notify specifically about errors that arrise when flushing (syncing) block data to disk. Unlike other instances, the current calls to AbortNode in the blockstorage flush functions do not report an error to their callers. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure.
1 parent 3fa9094 commit 7320db9

10 files changed

+35
-3
lines changed

src/bitcoin-chainstate.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ int main(int argc, char* argv[])
9797
{
9898
std::cout << "Warning: " << warning.original << std::endl;
9999
}
100+
void flushError(const std::string& debug_message) override
101+
{
102+
std::cerr << "Error flushing block data to disk: " << debug_message << std::endl;
103+
}
104+
100105
};
101106
auto notifications = std::make_unique<KernelNotifications>();
102107

@@ -112,6 +117,7 @@ int main(int argc, char* argv[])
112117
const node::BlockManager::Options blockman_opts{
113118
.chainparams = chainman_opts.chainparams,
114119
.blocks_dir = abs_datadir / "blocks",
120+
.notifications = chainman_opts.notifications,
115121
};
116122
ChainstateManager chainman{kernel_context.interrupt, chainman_opts, blockman_opts};
117123

src/init.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,7 @@ bool AppInitParameterInteraction(const ArgsManager& args)
999999
BlockManager::Options blockman_opts_dummy{
10001000
.chainparams = chainman_opts_dummy.chainparams,
10011001
.blocks_dir = args.GetBlocksDirPath(),
1002+
.notifications = chainman_opts_dummy.notifications,
10021003
};
10031004
auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)};
10041005
if (!blockman_result) {
@@ -1423,6 +1424,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14231424
BlockManager::Options blockman_opts{
14241425
.chainparams = chainman_opts.chainparams,
14251426
.blocks_dir = args.GetBlocksDirPath(),
1427+
.notifications = chainman_opts.notifications,
14261428
};
14271429
Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
14281430

src/kernel/blockmanager_opts.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
66
#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
77

8+
#include <kernel/notifications_interface.h>
89
#include <util/fs.h>
910

1011
#include <cstdint>
@@ -25,6 +26,7 @@ struct BlockManagerOpts {
2526
bool fast_prune{false};
2627
bool stop_after_block_import{DEFAULT_STOPAFTERBLOCKIMPORT};
2728
const fs::path blocks_dir;
29+
Notifications& notifications;
2830
};
2931

3032
} // namespace kernel

src/kernel/notifications_interface.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ class Notifications
2727
virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {}
2828
virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {}
2929
virtual void warning(const bilingual_str& warning) {}
30+
31+
//! The flush error notification is sent to notify the user that an error
32+
//! occurred while flushing block data to disk. Kernel code may ignore flush
33+
//! errors that don't affect the immediate operation it is trying to
34+
//! perform. Applications can choose to handle the flush error notification
35+
//! by logging the error, or notifying the user, or triggering an early
36+
//! shutdown as a precaution against causing more errors.
37+
virtual void flushError(const std::string& debug_message) {}
3038
};
3139
} // namespace kernel
3240

src/node/blockstorage.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ void BlockManager::FlushUndoFile(int block_file, bool finalize)
528528
{
529529
FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize);
530530
if (!UndoFileSeq().Flush(undo_pos_old, finalize)) {
531-
AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.");
531+
m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error.");
532532
}
533533
}
534534

@@ -547,7 +547,7 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)
547547

548548
FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize);
549549
if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) {
550-
AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");
550+
m_opts.notifications.flushError("Flushing block file to disk failed. This is likely the result of an I/O error.");
551551
}
552552
// we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks,
553553
// e.g. during IBD or a sync after a node going offline

src/node/kernel_notifications.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <common/args.h>
1212
#include <common/system.h>
1313
#include <node/interface_ui.h>
14+
#include <shutdown.h>
1415
#include <util/strencodings.h>
1516
#include <util/string.h>
1617
#include <util/translation.h>
@@ -72,4 +73,9 @@ void KernelNotifications::warning(const bilingual_str& warning)
7273
DoWarning(warning);
7374
}
7475

76+
void KernelNotifications::flushError(const std::string& debug_message)
77+
{
78+
AbortNode(debug_message);
79+
}
80+
7581
} // namespace node

src/node/kernel_notifications.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class KernelNotifications : public kernel::Notifications
2525
void progress(const bilingual_str& title, int progress_percent, bool resume_possible) override;
2626

2727
void warning(const bilingual_str& warning) override;
28+
29+
void flushError(const std::string& debug_message) override;
2830
};
2931
} // namespace node
3032

src/test/blockmanager_tests.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@
55
#include <chainparams.h>
66
#include <node/blockstorage.h>
77
#include <node/context.h>
8+
#include <node/kernel_notifications.h>
89
#include <util/chaintype.h>
910
#include <validation.h>
1011

1112
#include <boost/test/unit_test.hpp>
1213
#include <test/util/setup_common.h>
1314

14-
using node::BlockManager;
1515
using node::BLOCK_SERIALIZATION_HEADER_SIZE;
16+
using node::BlockManager;
17+
using node::KernelNotifications;
1618
using node::MAX_BLOCKFILE_SIZE;
1719

1820
// use BasicTestingSetup here for the data directory configuration, setup, and cleanup
@@ -21,9 +23,11 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
2123
BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
2224
{
2325
const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)};
26+
KernelNotifications notifications{};
2427
const BlockManager::Options blockman_opts{
2528
.chainparams = *params,
2629
.blocks_dir = m_args.GetBlocksDirPath(),
30+
.notifications = notifications,
2731
};
2832
BlockManager blockman{m_node.kernel->interrupt, blockman_opts};
2933
CChain chain {};

src/test/util/setup_common.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
196196
const BlockManager::Options blockman_opts{
197197
.chainparams = chainman_opts.chainparams,
198198
.blocks_dir = m_args.GetBlocksDirPath(),
199+
.notifications = chainman_opts.notifications,
199200
};
200201
m_node.chainman = std::make_unique<ChainstateManager>(m_node.kernel->interrupt, chainman_opts, blockman_opts);
201202
m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(DBParams{

src/test/validation_chainstatemanager_tests.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ struct SnapshotTestSetup : TestChain100Setup {
389389
const BlockManager::Options blockman_opts{
390390
.chainparams = chainman_opts.chainparams,
391391
.blocks_dir = m_args.GetBlocksDirPath(),
392+
.notifications = chainman_opts.notifications,
392393
};
393394
// For robustness, ensure the old manager is destroyed before creating a
394395
// new one.

0 commit comments

Comments
 (0)