aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTheCharlatan <seb.kung@gmail.com>2023-06-15 23:09:37 +0200
committerTheCharlatan <seb.kung@gmail.com>2023-06-28 09:52:32 +0200
commit7320db96f8d2aeff0bc5dc67d8b7b37f5f808990 (patch)
tree6a15f7fac466f6c0d4dc75ff05a9b52205c1102f /src
parent3fa9094b92c5d37f486b0f8265062d3456796a50 (diff)
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.
Diffstat (limited to 'src')
-rw-r--r--src/bitcoin-chainstate.cpp6
-rw-r--r--src/init.cpp2
-rw-r--r--src/kernel/blockmanager_opts.h2
-rw-r--r--src/kernel/notifications_interface.h8
-rw-r--r--src/node/blockstorage.cpp4
-rw-r--r--src/node/kernel_notifications.cpp6
-rw-r--r--src/node/kernel_notifications.h2
-rw-r--r--src/test/blockmanager_tests.cpp6
-rw-r--r--src/test/util/setup_common.cpp1
-rw-r--r--src/test/validation_chainstatemanager_tests.cpp1
10 files changed, 35 insertions, 3 deletions
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index da42372a6a..cb391e30de 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -97,6 +97,11 @@ int main(int argc, char* argv[])
{
std::cout << "Warning: " << warning.original << std::endl;
}
+ void flushError(const std::string& debug_message) override
+ {
+ std::cerr << "Error flushing block data to disk: " << debug_message << std::endl;
+ }
+
};
auto notifications = std::make_unique<KernelNotifications>();
@@ -112,6 +117,7 @@ int main(int argc, char* argv[])
const node::BlockManager::Options blockman_opts{
.chainparams = chainman_opts.chainparams,
.blocks_dir = abs_datadir / "blocks",
+ .notifications = chainman_opts.notifications,
};
ChainstateManager chainman{kernel_context.interrupt, chainman_opts, blockman_opts};
diff --git a/src/init.cpp b/src/init.cpp
index ac81ebab82..a1b210f201 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -999,6 +999,7 @@ bool AppInitParameterInteraction(const ArgsManager& args)
BlockManager::Options blockman_opts_dummy{
.chainparams = chainman_opts_dummy.chainparams,
.blocks_dir = args.GetBlocksDirPath(),
+ .notifications = chainman_opts_dummy.notifications,
};
auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)};
if (!blockman_result) {
@@ -1423,6 +1424,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
BlockManager::Options blockman_opts{
.chainparams = chainman_opts.chainparams,
.blocks_dir = args.GetBlocksDirPath(),
+ .notifications = chainman_opts.notifications,
};
Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h
index 8f26422f72..0251bbb10a 100644
--- a/src/kernel/blockmanager_opts.h
+++ b/src/kernel/blockmanager_opts.h
@@ -5,6 +5,7 @@
#ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
+#include <kernel/notifications_interface.h>
#include <util/fs.h>
#include <cstdint>
@@ -25,6 +26,7 @@ struct BlockManagerOpts {
bool fast_prune{false};
bool stop_after_block_import{DEFAULT_STOPAFTERBLOCKIMPORT};
const fs::path blocks_dir;
+ Notifications& notifications;
};
} // namespace kernel
diff --git a/src/kernel/notifications_interface.h b/src/kernel/notifications_interface.h
index 48248e9aa0..e009810473 100644
--- a/src/kernel/notifications_interface.h
+++ b/src/kernel/notifications_interface.h
@@ -27,6 +27,14 @@ public:
virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {}
virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {}
virtual void warning(const bilingual_str& warning) {}
+
+ //! The flush error notification is sent to notify the user that an error
+ //! occurred while flushing block data to disk. Kernel code may ignore flush
+ //! errors that don't affect the immediate operation it is trying to
+ //! perform. Applications can choose to handle the flush error notification
+ //! by logging the error, or notifying the user, or triggering an early
+ //! shutdown as a precaution against causing more errors.
+ virtual void flushError(const std::string& debug_message) {}
};
} // namespace kernel
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index a27a7a5466..729ac8850a 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -528,7 +528,7 @@ void 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)) {
- AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.");
+ m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error.");
}
}
@@ -547,7 +547,7 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)
FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize);
if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) {
- AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");
+ m_opts.notifications.flushError("Flushing block file to disk failed. This is likely the result of an I/O error.");
}
// 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
diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp
index 926b157f3b..7c7db26e9b 100644
--- a/src/node/kernel_notifications.cpp
+++ b/src/node/kernel_notifications.cpp
@@ -11,6 +11,7 @@
#include <common/args.h>
#include <common/system.h>
#include <node/interface_ui.h>
+#include <shutdown.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/translation.h>
@@ -72,4 +73,9 @@ void KernelNotifications::warning(const bilingual_str& warning)
DoWarning(warning);
}
+void KernelNotifications::flushError(const std::string& debug_message)
+{
+ AbortNode(debug_message);
+}
+
} // namespace node
diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h
index 3e665bbf14..fa80af1120 100644
--- a/src/node/kernel_notifications.h
+++ b/src/node/kernel_notifications.h
@@ -25,6 +25,8 @@ public:
void progress(const bilingual_str& title, int progress_percent, bool resume_possible) override;
void warning(const bilingual_str& warning) override;
+
+ void flushError(const std::string& debug_message) override;
};
} // namespace node
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
index 631f85908e..3e8cbb218d 100644
--- a/src/test/blockmanager_tests.cpp
+++ b/src/test/blockmanager_tests.cpp
@@ -5,14 +5,16 @@
#include <chainparams.h>
#include <node/blockstorage.h>
#include <node/context.h>
+#include <node/kernel_notifications.h>
#include <util/chaintype.h>
#include <validation.h>
#include <boost/test/unit_test.hpp>
#include <test/util/setup_common.h>
-using node::BlockManager;
using node::BLOCK_SERIALIZATION_HEADER_SIZE;
+using node::BlockManager;
+using node::KernelNotifications;
using node::MAX_BLOCKFILE_SIZE;
// use BasicTestingSetup here for the data directory configuration, setup, and cleanup
@@ -21,9 +23,11 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
{
const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)};
+ KernelNotifications notifications{};
const BlockManager::Options blockman_opts{
.chainparams = *params,
.blocks_dir = m_args.GetBlocksDirPath(),
+ .notifications = notifications,
};
BlockManager blockman{m_node.kernel->interrupt, blockman_opts};
CChain chain {};
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 514df73922..629b0cdcb8 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -196,6 +196,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
const BlockManager::Options blockman_opts{
.chainparams = chainman_opts.chainparams,
.blocks_dir = m_args.GetBlocksDirPath(),
+ .notifications = chainman_opts.notifications,
};
m_node.chainman = std::make_unique<ChainstateManager>(m_node.kernel->interrupt, chainman_opts, blockman_opts);
m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(DBParams{
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index f918ed6f95..00d33114dd 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -389,6 +389,7 @@ struct SnapshotTestSetup : TestChain100Setup {
const BlockManager::Options blockman_opts{
.chainparams = chainman_opts.chainparams,
.blocks_dir = m_args.GetBlocksDirPath(),
+ .notifications = chainman_opts.notifications,
};
// For robustness, ensure the old manager is destroyed before creating a
// new one.