From 7d8a61977db03eefdde171acc245b3edcf34285c Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 16 Jul 2020 18:09:32 +0100 Subject: [PATCH 01/23] Champ correct size --- src/ds/champ_map.h | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ds/champ_map.h b/src/ds/champ_map.h index 4e2b1eefa8f8..67fd82c0c0a8 100644 --- a/src/ds/champ_map.h +++ b/src/ds/champ_map.h @@ -243,8 +243,9 @@ namespace champ const auto& entry0 = node_as>(c_idx); if (k == entry0->key) { + auto current_size = get_size_with_padding(entry0->key, entry0->value); nodes[c_idx] = std::make_shared>(k, v); - return get_size_with_padding(entry0->key, entry0->value); + return current_size; } if (depth < (collision_depth - 1)) @@ -332,7 +333,7 @@ namespace champ { private: std::shared_ptr> root; - size_t _size = 0; + size_t size = 0; size_t serialized_size = 0; Map( @@ -340,7 +341,7 @@ namespace champ size_t size_, size_t serialized_size_) : root(std::move(root_)), - _size(size_), + size(size_), serialized_size(serialized_size_) {} @@ -371,9 +372,9 @@ namespace champ return map; } - size_t size() const + size_t get_size() const { - return _size; + return size; } size_t get_serialized_size() const @@ -383,7 +384,7 @@ namespace champ bool empty() const { - return _size == 0; + return size == 0; } std::optional get(const K& key) const @@ -404,13 +405,13 @@ namespace champ const Map put(const K& key, const V& value) const { auto r = root->put(0, H()(key), key, value); - auto size_ = _size; + auto size_ = size; if (r.second == 0) { size_++; } - int64_t size_change = get_size_with_padding(key, value) - r.second; + int64_t size_change = get_size_with_padding(key, value) - r.second; // r.second - serialized_size return Map(std::move(r.first), size_, size_change + serialized_size); } @@ -468,7 +469,7 @@ namespace champ void serialize(uint8_t* data) { std::vector ordered_state; - ordered_state.reserve(map.size()); + ordered_state.reserve(map.get_size()); size_t size = 0; map.foreach([&](auto& key, auto& value) { @@ -498,7 +499,7 @@ namespace champ "size:{}, map->size:{} ==> count:{}, vect:{}", size, map.get_serialized_size(), - map.size(), + map.get_size(), ordered_state.size()); serialized_buffer = CBuffer(data, map.get_serialized_size()); From 6dd9e0b5621245d8e8402e0d9c9e019519f61598 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 17 Jul 2020 10:25:04 +0100 Subject: [PATCH 02/23] Snapshot from raft --- src/consensus/raft/raft.h | 9 +++++++++ src/consensus/raft/raft_types.h | 16 ++++++++++++---- src/ds/champ_map.h | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index e23c45514b23..86d353184c3e 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -1137,6 +1137,8 @@ namespace raft void commit(Index idx) { + static size_t snapshot_interval = 100; + if (idx > last_idx) throw std::logic_error( "Tried to commit " + std::to_string(idx) + "but last_idx as " + @@ -1154,6 +1156,13 @@ namespace raft LOG_DEBUG_FMT("Compacting..."); store->compact(idx); ledger->commit(idx); + + if (state == Leader) + { + LOG_FAIL_FMT("Snapshotting at {}", idx); + store->snapshot(idx); + LOG_FAIL_FMT("Snapshot done"); + } LOG_DEBUG_FMT("Commit on {}: {}", local_id, idx); // Examine all configurations that are followed by a globally committed diff --git a/src/consensus/raft/raft_types.h b/src/consensus/raft/raft_types.h index 77902ee8a9b6..f25f8b33f3ce 100644 --- a/src/consensus/raft/raft_types.h +++ b/src/consensus/raft/raft_types.h @@ -29,6 +29,7 @@ namespace raft Term* term = nullptr) = 0; virtual void compact(Index v) = 0; virtual void rollback(Index v, std::optional t = std::nullopt) = 0; + virtual void snapshot(Index v) = 0; virtual void set_term(Term t) = 0; }; @@ -44,7 +45,7 @@ namespace raft S deserialise( const std::vector& data, bool public_only = false, - Term* term = nullptr) + Term* term = nullptr) override { auto p = x.lock(); if (p) @@ -52,7 +53,7 @@ namespace raft return S::FAILED; } - void compact(Index v) + void compact(Index v) override { auto p = x.lock(); if (p) @@ -61,14 +62,21 @@ namespace raft } } - void rollback(Index v, std::optional t = std::nullopt) + void rollback(Index v, std::optional t = std::nullopt) override { auto p = x.lock(); if (p) p->rollback(v, t); } - void set_term(Term t) + void snapshot(Index v) override + { + auto p = x.lock(); + if (p) + p->serialise_snapshot(v); + } + + void set_term(Term t) override { auto p = x.lock(); if (p) diff --git a/src/ds/champ_map.h b/src/ds/champ_map.h index 67fd82c0c0a8..1ee10f845a05 100644 --- a/src/ds/champ_map.h +++ b/src/ds/champ_map.h @@ -411,7 +411,7 @@ namespace champ size_++; } - int64_t size_change = get_size_with_padding(key, value) - r.second; // r.second - serialized_size + int64_t size_change = get_size_with_padding(key, value) - r.second; return Map(std::move(r.first), size_, size_change + serialized_size); } From 2b283c252be53eb330b39b86ce190b8fb76cda4e Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 17 Jul 2020 16:09:34 +0100 Subject: [PATCH 03/23] Generate snapshots and store to disk --- src/consensus/ledger_enclave.h | 6 +++ src/consensus/ledger_enclave_types.h | 5 ++ src/consensus/raft/raft.h | 24 +++++++-- src/consensus/raft/raft_types.h | 15 ++++-- src/host/main.cpp | 8 ++- src/host/node_connections.h | 1 + src/host/snapshot.h | 77 ++++++++++++++++++++++++++++ tests/reconfiguration.py | 13 +++-- 8 files changed, 136 insertions(+), 13 deletions(-) create mode 100644 src/host/snapshot.h diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 01a4595e74ed..91fa718e2cd3 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -99,5 +99,11 @@ namespace consensus { RINGBUFFER_WRITE_MESSAGE(consensus::ledger_commit, to_host, idx); } + + void put_snapshot(Index idx, const std::vector& snapshot) + { + RINGBUFFER_WRITE_MESSAGE( + consensus::ledger_snapshot, to_host, idx, snapshot); + } }; } \ No newline at end of file diff --git a/src/consensus/ledger_enclave_types.h b/src/consensus/ledger_enclave_types.h index 8955959ed722..1280a4bf8c34 100644 --- a/src/consensus/ledger_enclave_types.h +++ b/src/consensus/ledger_enclave_types.h @@ -28,6 +28,9 @@ namespace consensus DEFINE_RINGBUFFER_MSG_TYPE(ledger_append), DEFINE_RINGBUFFER_MSG_TYPE(ledger_truncate), DEFINE_RINGBUFFER_MSG_TYPE(ledger_commit), + + // Create a new snapshot. Enclave -> Host + DEFINE_RINGBUFFER_MSG_TYPE(ledger_snapshot), }; } @@ -47,3 +50,5 @@ DECLARE_RINGBUFFER_MESSAGE_PAYLOAD( DECLARE_RINGBUFFER_MESSAGE_PAYLOAD( consensus::ledger_truncate, consensus::Index); DECLARE_RINGBUFFER_MESSAGE_PAYLOAD(consensus::ledger_commit, consensus::Index); +DECLARE_RINGBUFFER_MESSAGE_PAYLOAD( + consensus::ledger_snapshot, consensus::Index, std::vector); diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 86d353184c3e..1685b154b6f0 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -1137,7 +1137,8 @@ namespace raft void commit(Index idx) { - static size_t snapshot_interval = 100; + static size_t snapshot_interval = 10; + static Index last_snapshot_idx = 0; if (idx > last_idx) throw std::logic_error( @@ -1159,9 +1160,24 @@ namespace raft if (state == Leader) { - LOG_FAIL_FMT("Snapshotting at {}", idx); - store->snapshot(idx); - LOG_FAIL_FMT("Snapshot done"); + if (idx - last_snapshot_idx > snapshot_interval) + { + LOG_FAIL_FMT("Snapshotting at {}", idx); + + auto snap = store->snapshot(idx); + if (snap.has_value()) + { + ledger->put_snapshot(idx, snap.value()); + } + else + { + LOG_FAIL_FMT( + "Error generating snapshot at {}. Continuing normal operation.", + idx); + } + last_snapshot_idx = idx; + LOG_FAIL_FMT("Snapshot done"); + } } LOG_DEBUG_FMT("Commit on {}: {}", local_id, idx); diff --git a/src/consensus/raft/raft_types.h b/src/consensus/raft/raft_types.h index f25f8b33f3ce..b0b04850d4e6 100644 --- a/src/consensus/raft/raft_types.h +++ b/src/consensus/raft/raft_types.h @@ -29,7 +29,7 @@ namespace raft Term* term = nullptr) = 0; virtual void compact(Index v) = 0; virtual void rollback(Index v, std::optional t = std::nullopt) = 0; - virtual void snapshot(Index v) = 0; + virtual std::optional> snapshot(Index v) = 0; virtual void set_term(Term t) = 0; }; @@ -49,7 +49,9 @@ namespace raft { auto p = x.lock(); if (p) + { return p->deserialise(data, public_only, term); + } return S::FAILED; } @@ -66,21 +68,28 @@ namespace raft { auto p = x.lock(); if (p) + { p->rollback(v, t); + } } - void snapshot(Index v) override + std::optional> snapshot(Index v) override { auto p = x.lock(); if (p) - p->serialise_snapshot(v); + { + return p->serialise_snapshot(v); + } + return std::nullopt; } void set_term(Term t) override { auto p = x.lock(); if (p) + { p->set_term(t); + } } }; diff --git a/src/host/main.cpp b/src/host/main.cpp index b4f669b2313a..ac7413c525f7 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -142,6 +142,10 @@ int main(int argc, char** argv) ->capture_default_str() ->transform(CLI::AsSizeValue(true)); // 1000 is kb + std::string snapshot_dir("snapshots"); + app.add_option("--snapshot-dir", snapshot_dir, "Snapshots directory") + ->capture_default_str(); + logger::Level host_log_level{logger::Level::INFO}; std::vector> level_map; for (int i = logger::TRACE; i < logger::MAX_LOG_LEVEL; i++) @@ -522,10 +526,12 @@ int main(int argc, char** argv) // graceful shutdown on sigterm asynchost::Sigterm sigterm(writer_factory); - // write to a ledger asynchost::Ledger ledger(ledger_dir, writer_factory, ledger_chunk_threshold); ledger.register_message_handlers(bp.get_dispatcher()); + asynchost::SnapshotManager snapshot(snapshot_dir); + snapshot.register_message_handlers(bp.get_dispatcher()); + // Begin listening for node-to-node and RPC messages. // This includes DNS resolution and potentially dynamic port assignment (if // requesting port 0). The hostname and port may be modified - after calling diff --git a/src/host/node_connections.h b/src/host/node_connections.h index a40b5d98b375..90b4aa1236fc 100644 --- a/src/host/node_connections.h +++ b/src/host/node_connections.h @@ -7,6 +7,7 @@ #include "consensus/raft/raft_types.h" #include "host/timer.h" #include "ledger.h" +#include "snapshot.h" #include "node/nodetypes.h" #include "tcp.h" diff --git a/src/host/snapshot.h b/src/host/snapshot.h new file mode 100644 index 000000000000..2fdb0cc0fb05 --- /dev/null +++ b/src/host/snapshot.h @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "consensus/ledger_enclave_types.h" + +#include +#include +#include + +namespace fs = std::filesystem; + +namespace asynchost +{ + class SnapshotManager + { + private: + const std::string snapshot_dir; + static constexpr auto snapshot_file_prefix = "snapshot"; + + void write_snapshot( + size_t idx, const uint8_t* snapshot_data, size_t snapshot_size) + { + auto snapshot_file_name = fmt::format("{}.{}", snapshot_file_prefix, idx); + auto full_snapshot_path = + fs::path(snapshot_dir) / fs::path(snapshot_file_name); + + if (fs::exists(full_snapshot_path)) + { + throw std::logic_error(fmt::format( + "Cannot write snapshot at {} since file already exists: {}", + idx, + full_snapshot_path)); + } + + LOG_INFO_FMT( + "Writing new snapshot to {} [{}]", snapshot_file_name, snapshot_size); + + std::ofstream snapshot_file( + full_snapshot_path, std::ios::out | std::ios::binary); + snapshot_file.write( + reinterpret_cast(snapshot_data), snapshot_size); + } + + public: + SnapshotManager(const std::string& snapshot_dir_) : + snapshot_dir(snapshot_dir_) + { + if (fs::is_directory(snapshot_dir)) + { + throw std::logic_error(fmt::format( + "Error: Cannot create snapshot directory as it already " + "exists: {}", + snapshot_dir)); + } + + if (!fs::create_directory(snapshot_dir)) + { + throw std::logic_error(fmt::format( + "Error: Could not create snapshot directory: {}", snapshot_dir)); + } + } + + void register_message_handlers( + messaging::Dispatcher& disp) + { + DISPATCHER_SET_MESSAGE_HANDLER( + disp, + consensus::ledger_snapshot, + [this](const uint8_t* data, size_t size) { + auto idx = serialized::read(data, size); + + write_snapshot(idx, data, size); + }); + } + }; +} \ No newline at end of file diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index 72f9afb39e10..e26d51c3899e 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -5,6 +5,7 @@ import infra.proc import suite.test_requirements as reqs import time +import e2e_logging from loguru import logger as LOG @@ -93,12 +94,14 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb ) as network: network.start_and_join(args) - test_add_node_from_backup(network, args) - test_add_node(network, args) - test_add_node_untrusted_code(network, args) - test_retire_node(network, args) - test_add_as_many_pending_nodes(network, args) + for _ in range(1,100): + e2e_logging.test(network, args, verify=False) + # test_add_node_from_backup(network, args) test_add_node(network, args) + # test_add_node_untrusted_code(network, args) + # test_retire_node(network, args) + # test_add_as_many_pending_nodes(network, args) + # test_add_node(network, args) if __name__ == "__main__": From a3705e24ff7a2bcf6db0cbcde4f7587df9dd68cb Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Mon, 20 Jul 2020 10:06:35 +0100 Subject: [PATCH 04/23] Snapshot protocol WIP --- python/ccf/clients.py | 4 +- src/consensus/raft/raft.h | 75 ++++++++++++++++++++++++--------- src/consensus/raft/raft_types.h | 1 + src/host/node_connections.h | 6 ++- tests/reconfiguration.py | 2 +- 5 files changed, 64 insertions(+), 24 deletions(-) diff --git a/python/ccf/clients.py b/python/ccf/clients.py index 7740ca0462c3..9522d1505808 100644 --- a/python/ccf/clients.py +++ b/python/ccf/clients.py @@ -34,8 +34,8 @@ def truncate(string, max_len=256): # Deprecated, will be removed CCF_GLOBAL_COMMIT_HEADER = "x-ccf-global-commit" -DEFAULT_CONNECTION_TIMEOUT_SEC = 3 -DEFAULT_REQUEST_TIMEOUT_SEC = 3 +DEFAULT_CONNECTION_TIMEOUT_SEC = 3000 +DEFAULT_REQUEST_TIMEOUT_SEC = 3000 class Request: diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 1685b154b6f0..431e550356ef 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -84,22 +84,23 @@ namespace raft struct NodeState { - // the highest matching index with the node that was confirmed - Index match_idx; // the highest index sent to the node Index sent_idx; + // the highest matching index with the node that was confirmed + Index match_idx; + Configuration::NodeInfo node_info; NodeState() = default; NodeState( - Index match_idx_, + const Configuration::NodeInfo& node_info_, Index sent_idx_, - const Configuration::NodeInfo& node_info_) : - match_idx(match_idx_), + Index match_idx_ = 0) : + node_info(node_info_), sent_idx(sent_idx_), - node_info(node_info_) + match_idx(match_idx_) {} }; @@ -115,6 +116,10 @@ namespace raft Index commit_idx; TermHistory term_history; + // Snapshots + Index last_snapshot_idx; + Term last_snapshot_term; + // Volatile NodeId leader_id; std::unordered_set votes_for_me; @@ -496,6 +501,13 @@ namespace raft end_idx, commit_idx); + bool is_snapshot = start_idx == last_snapshot_idx; + + if (is_snapshot) + { + LOG_FAIL_FMT("Sending snapshot"); + } + AppendEntries ae = {raft_append_entries, local_id, end_idx, @@ -503,16 +515,21 @@ namespace raft current_term, prev_term, commit_idx, - term_of_idx}; + term_of_idx, + is_snapshot}; auto& node = nodes.at(to); - // Record the most recent index we have sent to this node. - node.sent_idx = end_idx; - // The host will append log entries to this message when it is // sent to the destination node. - channels->send_authenticated(ccf::NodeMsgType::consensus_msg, to, ae); + if (!channels->send_authenticated( + ccf::NodeMsgType::consensus_msg, to, ae)) + { + return; + } + + // Record the most recent index we have sent to this node. + node.sent_idx = end_idx; } void recv_append_entries(const uint8_t* data, size_t size) @@ -536,6 +553,15 @@ namespace raft r.term, r.idx); + if (r.is_snapshot) + { + LOG_FAIL_FMT("This is a snapshot!!"); + + // TODO: If this is a snapshot: + // - Do not check index consistency + // - + } + const auto prev_term = get_term_internal(r.prev_idx); LOG_DEBUG_FMT("Previous term for {} should be {}", r.prev_idx, prev_term); @@ -545,7 +571,7 @@ namespace raft if (r.prev_idx < commit_idx) { LOG_DEBUG_FMT( - "Recv append entries to {} from {} but prev_idex ({}) < commit_idx " + "Recv append entries to {} from {} but prev_idx ({}) < commit_idx " "({})", local_id, r.from_node, @@ -608,14 +634,17 @@ namespace raft } LOG_DEBUG_FMT( - "Recv append entries to {} from {} for index {} and previous index {}", + "Recv append entries to {} from {} for index {} and previous index {} " + "{}", local_id, r.from_node, r.idx, - r.prev_idx); + r.prev_idx, + r.is_snapshot ? "[snapshot]" : ""); for (Index i = r.prev_idx + 1; i <= r.idx; i++) { + // TODO: Skip this check if snapshot if (i <= last_idx) { // If the current entry has already been deserialised, skip the @@ -651,6 +680,7 @@ namespace raft auto deserialise_success = store->deserialise(entry, public_only, &sig_term); + // TODO: don't do this if snapshot ledger->put_entry( entry, deserialise_success == kv::DeserialiseSuccess::PASS_SIGNATURE); @@ -1138,7 +1168,6 @@ namespace raft void commit(Index idx) { static size_t snapshot_interval = 10; - static Index last_snapshot_idx = 0; if (idx > last_idx) throw std::logic_error( @@ -1176,6 +1205,7 @@ namespace raft idx); } last_snapshot_idx = idx; + last_snapshot_term = current_term; LOG_FAIL_FMT("Snapshot done"); } } @@ -1284,10 +1314,14 @@ namespace raft if (nodes.find(node_info.first) == nodes.end()) { - // A new node is sent only future entries initially. If it does not - // have prior data, it will communicate that back to the leader. - auto index = last_idx + 1; - nodes.try_emplace(node_info.first, 0, index, node_info.second); + // A new node is sent entries from the last snapshot idx. Subsequent + // entries will be caught up using normal append entries. + + // TODO: In the case of last_snapshot_idx = 0, this sends -1 as + // prev_idx :( + // auto send_idx = last_snapshot_idx; + nodes.try_emplace( + node_info.first, node_info.second, last_snapshot_idx - 1); if (state == Leader) { @@ -1295,7 +1329,8 @@ namespace raft node_info.first, node_info.second.hostname, node_info.second.port); - send_append_entries(node_info.first, index); + + send_append_entries(node_info.first, last_snapshot_idx); } LOG_INFO_FMT("Added raft node {}", node_info.first); diff --git a/src/consensus/raft/raft_types.h b/src/consensus/raft/raft_types.h index b0b04850d4e6..4649409ec24a 100644 --- a/src/consensus/raft/raft_types.h +++ b/src/consensus/raft/raft_types.h @@ -115,6 +115,7 @@ namespace raft Term prev_term; Index leader_commit_idx; Term term_of_idx; + bool is_snapshot; }; struct AppendEntriesResponse : RaftHeader diff --git a/src/host/node_connections.h b/src/host/node_connections.h index 90b4aa1236fc..13fc980db13b 100644 --- a/src/host/node_connections.h +++ b/src/host/node_connections.h @@ -7,8 +7,8 @@ #include "consensus/raft/raft_types.h" #include "host/timer.h" #include "ledger.h" -#include "snapshot.h" #include "node/nodetypes.h" +#include "snapshot.h" #include "tcp.h" #include @@ -255,6 +255,10 @@ namespace asynchost uint32_t frame = (uint32_t)size_to_send; std::optional> framed_entries = std::nullopt; + + // TODO: If (ae.prev_idx + 1) is a snapshot, append that instead + // (with length-prefix), then all other append entries after that. + framed_entries = ledger.read_framed_entries(ae.prev_idx + 1, ae.idx); if (framed_entries.has_value()) diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index e26d51c3899e..3a5e05b62fc3 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -94,7 +94,7 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb ) as network: network.start_and_join(args) - for _ in range(1,100): + for _ in range(1,1): e2e_logging.test(network, args, verify=False) # test_add_node_from_backup(network, args) test_add_node(network, args) From 0b020c9077436fe1689d183e193f75b2a6ae0378 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 10:07:56 +0100 Subject: [PATCH 05/23] ledger max chunk -> ledger min chunk --- CMakeLists.txt | 2 -- doc/operators/ledger.rst | 6 +++--- src/host/main.cpp | 8 ++++---- start_test_network.sh | 2 +- tests/infra/e2e_args.py | 2 +- tests/infra/network.py | 2 +- tests/infra/remote.py | 6 +++--- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f2cb7a8c6b0b..f37ef68010df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -485,8 +485,6 @@ if(BUILD_TESTS) NAME governance_history_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/governance_history.py CONSENSUS raft - ADDITIONAL_ARGS # https://github.com/microsoft/CCF/issues/1275 - --ledger-chunk-max-bytes 20MB ) add_e2e_test( diff --git a/doc/operators/ledger.rst b/doc/operators/ledger.rst index 323d29231d97..879050d35f89 100644 --- a/doc/operators/ledger.rst +++ b/doc/operators/ledger.rst @@ -15,10 +15,10 @@ Nodes can be configured to store their ledger under a particular directory with File layout ----------- -The ledger directory contains a series of files. File size is controlled by the ``--ledger-chunk-max-bytes`` command line option. +The ledger directory contains a series of files. File size is controlled by the ``--ledger-chunk-min-bytes`` command line option. Files containing only committed entries are named ``ledger_$STARTSEQNO-$ENDSEQNO.committed``. These files are closed and immutable, -it is safe to replicate them to backup storage. They are identical across nodes, provided ``--ledger-chunk-max-bytes`` has been set to the same value. +it is safe to replicate them to backup storage. They are identical across nodes, provided ``--ledger-chunk-min-bytes`` has been set to the same value. .. warning:: Removing files from a ledger directory may cause a node to crash. @@ -32,7 +32,7 @@ a ``.committed`` file once the size threshold is met. The listing below is an example of what a ledger directory may look like. .. code-block:: bash - + $ ./cchost --ledger-dir $LEDGER_DIR ... $ cd $LEDGER_DIR $ ls diff --git a/src/host/main.cpp b/src/host/main.cpp index 158d93f01cb3..39401df68632 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -136,11 +136,11 @@ int main(int argc, char** argv) app.add_option("--ledger-dir", ledger_dir, "Ledger directory") ->capture_default_str(); - size_t ledger_chunk_threshold = 5'000'000; + size_t ledger_min_bytes = 5'000'000; app .add_option( - "--ledger-chunk-max-bytes", - ledger_chunk_threshold, + "--ledger-chunk-min-bytes", + ledger_min_bytes, "Minimum size (bytes) at which a new ledger chunk is created.") ->capture_default_str() ->transform(CLI::AsSizeValue(true)); // 1000 is kb @@ -535,7 +535,7 @@ int main(int argc, char** argv) // graceful shutdown on sigterm asynchost::Sigterm sigterm(writer_factory); - asynchost::Ledger ledger(ledger_dir, writer_factory, ledger_chunk_threshold); + asynchost::Ledger ledger(ledger_dir, writer_factory, ledger_min_bytes); ledger.register_message_handlers(bp.get_dispatcher()); asynchost::SnapshotManager snapshot(snapshot_dir); diff --git a/start_test_network.sh b/start_test_network.sh index 771dede57877..4c56994104b4 100755 --- a/start_test_network.sh +++ b/start_test_network.sh @@ -21,5 +21,5 @@ CURL_CLIENT=ON \ python "${PATH_HERE}"/tests/start_network.py \ --gov-script "${PATH_HERE}"/src/runtime_config/gov.lua \ --label test_network \ - --ledger-chunk-max-bytes 5MB \ + --ledger-chunk-min-bytes 5MB \ "$@" \ No newline at end of file diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 5e234f89ffe2..f4d5d734c36c 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -166,7 +166,7 @@ def cli_args(add=lambda x: None, parser=None, accept_unknown=False): default=30, ) parser.add_argument( - "--ledger-chunk-max-bytes", + "--ledger-chunk-min-bytes", help="Minimum size (bytes) at which a new ledger chunk is created.", default="20KB", ) diff --git a/tests/infra/network.py b/tests/infra/network.py index 084eed8320f0..c516cb64f4f2 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -73,7 +73,7 @@ class Network: "gov_script", "join_timer", "worker_threads", - "ledger_chunk_max_bytes", + "ledger_chunk_min_bytes", "domain", ] diff --git a/tests/infra/remote.py b/tests/infra/remote.py index acf38a556d5c..fd8ffb305bc1 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -562,7 +562,7 @@ def __init__( ledger_dir=None, log_format_json=None, binary_dir=".", - ledger_chunk_max_bytes=(5 * 1024 * 1024), + ledger_chunk_min_bytes=(5 * 1024 * 1024), domain=None, ): """ @@ -629,8 +629,8 @@ def __init__( if memory_reserve_startup: cmd += [f"--memory-reserve-startup={memory_reserve_startup}"] - if ledger_chunk_max_bytes: - cmd += [f"--ledger-chunk-max-bytes={ledger_chunk_max_bytes}"] + if ledger_chunk_min_bytes: + cmd += [f"--ledger-chunk-min-bytes={ledger_chunk_min_bytes}"] if notify_server: notify_server_host, *notify_server_port = notify_server.split(":") From 21587506bb267535aa6f58573b9eabfbc291af42 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 14:36:42 +0100 Subject: [PATCH 06/23] Snapshots are written to disk --- CMakeLists.txt | 7 ++ src/consensus/ledger_enclave.h | 6 -- src/consensus/ledger_enclave_types.h | 4 +- src/consensus/pbft/pbft.h | 2 +- src/consensus/raft/raft.h | 99 +++++++++++--------------- src/consensus/raft/raft_consensus.h | 6 +- src/consensus/raft/raft_types.h | 1 - src/enclave/enclave.h | 2 +- src/enclave/interface.h | 2 + src/host/main.cpp | 10 +++ src/host/node_connections.h | 2 +- src/host/snapshot.h | 2 +- src/node/channels.h | 2 +- src/node/node_state.h | 20 ++++-- src/node/node_to_node.h | 2 +- src/node/{nodetypes.h => node_types.h} | 0 src/node/snapshotter.h | 62 ++++++++++++++++ src/node/test/channel_stub.h | 2 +- tests/infra/e2e_args.py | 7 +- tests/infra/network.py | 1 + tests/infra/remote.py | 4 ++ tests/reconfiguration.py | 2 + 22 files changed, 161 insertions(+), 84 deletions(-) rename src/node/{nodetypes.h => node_types.h} (100%) create mode 100644 src/node/snapshotter.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f37ef68010df..1ac87a5333ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -574,6 +574,13 @@ if(BUILD_TESTS) CONSENSUS raft ) + add_e2e_test( + NAME reconfiguration_snapshot_test + PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/reconfiguration.py + CONSENSUS raft + ADDITIONAL_ARGS --snapshot-min-tx 10 + ) + add_e2e_test( NAME code_update_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/code_update.py diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 91fa718e2cd3..01a4595e74ed 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -99,11 +99,5 @@ namespace consensus { RINGBUFFER_WRITE_MESSAGE(consensus::ledger_commit, to_host, idx); } - - void put_snapshot(Index idx, const std::vector& snapshot) - { - RINGBUFFER_WRITE_MESSAGE( - consensus::ledger_snapshot, to_host, idx, snapshot); - } }; } \ No newline at end of file diff --git a/src/consensus/ledger_enclave_types.h b/src/consensus/ledger_enclave_types.h index 1280a4bf8c34..d7555c760e30 100644 --- a/src/consensus/ledger_enclave_types.h +++ b/src/consensus/ledger_enclave_types.h @@ -6,7 +6,7 @@ namespace consensus { - using Index = uint64_t; + using Index = int64_t; enum LedgerRequestPurpose : uint8_t { @@ -29,7 +29,7 @@ namespace consensus DEFINE_RINGBUFFER_MSG_TYPE(ledger_truncate), DEFINE_RINGBUFFER_MSG_TYPE(ledger_commit), - // Create a new snapshot. Enclave -> Host + /// Create a new snapshot. Enclave -> Host DEFINE_RINGBUFFER_MSG_TYPE(ledger_snapshot), }; } diff --git a/src/consensus/pbft/pbft.h b/src/consensus/pbft/pbft.h index 653240ec05b9..d3505c746d3e 100644 --- a/src/consensus/pbft/pbft.h +++ b/src/consensus/pbft/pbft.h @@ -17,7 +17,7 @@ #include "enclave/rpc_map.h" #include "enclave/rpc_sessions.h" #include "kv/kv_types.h" -#include "node/nodetypes.h" +#include "node/node_types.h" #include #include diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 01079ce16620..a1e71b18080f 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -6,7 +6,7 @@ #include "ds/serialized.h" #include "ds/spin_lock.h" #include "kv/kv_types.h" -#include "node/nodetypes.h" +#include "node/node_types.h" #include "raft_types.h" #include @@ -71,7 +71,7 @@ namespace raft } }; - template + template class Raft { private: @@ -84,14 +84,14 @@ namespace raft struct NodeState { + Configuration::NodeInfo node_info; + // the highest index sent to the node Index sent_idx; // the highest matching index with the node that was confirmed Index match_idx; - Configuration::NodeInfo node_info; - NodeState() = default; NodeState( @@ -116,10 +116,6 @@ namespace raft Index commit_idx; TermHistory term_history; - // Snapshots - Index last_snapshot_idx; - Term last_snapshot_term; - // Volatile NodeId leader_id; std::unordered_set votes_for_me; @@ -156,12 +152,14 @@ namespace raft static constexpr size_t append_entries_size_limit = 20000; std::unique_ptr ledger; std::shared_ptr channels; + std::shared_ptr snapshotter; public: Raft( std::unique_ptr> store, std::unique_ptr ledger_, std::shared_ptr channels_, + std::shared_ptr snapshotter_, NodeId id, std::chrono::milliseconds request_timeout_, std::chrono::milliseconds election_timeout_, @@ -187,7 +185,8 @@ namespace raft rand((int)(uintptr_t)this), ledger(std::move(ledger_)), - channels(channels_) + channels(channels_), + snapshotter(snapshotter_) {} @@ -502,20 +501,12 @@ namespace raft end_idx, commit_idx); - bool is_snapshot = start_idx == last_snapshot_idx; - - if (is_snapshot) - { - LOG_FAIL_FMT("Sending snapshot"); - } - AppendEntries ae = {{raft_append_entries, local_id}, {end_idx, prev_idx}, current_term, prev_term, commit_idx, - term_of_idx, - is_snapshot}; + term_of_idx}; auto& node = nodes.at(to); @@ -627,17 +618,14 @@ namespace raft } LOG_DEBUG_FMT( - "Recv append entries to {} from {} for index {} and previous index {} " - "{}", + "Recv append entries to {} from {} for index {} and previous index {}", local_id, r.from_node, r.idx, - r.prev_idx, - r.is_snapshot ? "[snapshot]" : ""); + r.prev_idx); for (Index i = r.prev_idx + 1; i <= r.idx; i++) { - // TODO: Skip this check if snapshot if (i <= last_idx) { // If the current entry has already been deserialised, skip the @@ -673,7 +661,6 @@ namespace raft auto deserialise_success = store->deserialise(entry, public_only, &sig_term); - // TODO: don't do this if snapshot ledger->put_entry( entry, deserialise_success == kv::DeserialiseSuccess::PASS_SIGNATURE); @@ -1162,8 +1149,6 @@ namespace raft void commit(Index idx) { - static size_t snapshot_interval = 10; - if (idx > last_idx) throw std::logic_error( "Tried to commit " + std::to_string(idx) + "but last_idx as " + @@ -1179,31 +1164,35 @@ namespace raft commit_idx = idx; LOG_DEBUG_FMT("Compacting..."); - store->compact(idx); - ledger->commit(idx); - if (state == Leader) { - if (idx - last_snapshot_idx > snapshot_interval) - { - LOG_FAIL_FMT("Snapshotting at {}", idx); - - auto snap = store->snapshot(idx); - if (snap.has_value()) - { - ledger->put_snapshot(idx, snap.value()); - } - else - { - LOG_FAIL_FMT( - "Error generating snapshot at {}. Continuing normal operation.", - idx); - } - last_snapshot_idx = idx; - last_snapshot_term = current_term; - LOG_FAIL_FMT("Snapshot done"); - } + snapshotter->snapshot(idx); } + store->compact(idx); + ledger->commit(idx); + + // if (state == Leader) + // { + // if (idx - last_snapshot_idx > snapshot_interval) + // { + // LOG_FAIL_FMT("Snapshotting at {}", idx); + + // auto snap = store->snapshot(idx); + // if (snap.has_value()) + // { + // ledger->put_snapshot(idx, snap.value()); + // } + // else + // { + // LOG_FAIL_FMT( + // "Error generating snapshot at {}. Continuing normal + // operation.", idx); + // } + // last_snapshot_idx = idx; + // last_snapshot_term = current_term; + // LOG_FAIL_FMT("Snapshot done"); + // } + // } LOG_DEBUG_FMT("Commit on {}: {}", local_id, idx); // Examine all configurations that are followed by a globally committed @@ -1309,14 +1298,10 @@ namespace raft if (nodes.find(node_info.first) == nodes.end()) { - // A new node is sent entries from the last snapshot idx. Subsequent - // entries will be caught up using normal append entries. - - // TODO: In the case of last_snapshot_idx = 0, this sends -1 as - // prev_idx :( - // auto send_idx = last_snapshot_idx; - nodes.try_emplace( - node_info.first, node_info.second, last_snapshot_idx - 1); + // A new node is sent only future entries initially. If it does not + // have prior data, it will communicate that back to the leader. + auto index = last_idx + 1; + nodes.try_emplace(node_info.first, node_info.second, index, 0); if (state == Leader) { @@ -1325,7 +1310,7 @@ namespace raft node_info.second.hostname, node_info.second.port); - send_append_entries(node_info.first, last_snapshot_idx); + send_append_entries(node_info.first, index); } LOG_INFO_FMT("Added raft node {}", node_info.first); diff --git a/src/consensus/raft/raft_consensus.h b/src/consensus/raft/raft_consensus.h index 8e33c3f072a8..95910b2f13b4 100644 --- a/src/consensus/raft/raft_consensus.h +++ b/src/consensus/raft/raft_consensus.h @@ -13,14 +13,14 @@ namespace raft // the Raft API, allowing for a mapping between the generic consensus // terminology and the terminology that is specific to Raft - template + template class RaftConsensus : public kv::Consensus { private: - std::unique_ptr> raft; + std::unique_ptr> raft; public: - RaftConsensus(std::unique_ptr> raft_) : + RaftConsensus(std::unique_ptr> raft_) : Consensus(raft_->id()), raft(std::move(raft_)) {} diff --git a/src/consensus/raft/raft_types.h b/src/consensus/raft/raft_types.h index 4649409ec24a..b0b04850d4e6 100644 --- a/src/consensus/raft/raft_types.h +++ b/src/consensus/raft/raft_types.h @@ -115,7 +115,6 @@ namespace raft Term prev_term; Index leader_commit_idx; Term term_of_idx; - bool is_snapshot; }; struct AppendEntriesResponse : RaftHeader diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 03747feaade1..985e5f970ed9 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -11,7 +11,7 @@ #include "node/historical_queries.h" #include "node/network_state.h" #include "node/node_state.h" -#include "node/nodetypes.h" +#include "node/node_types.h" #include "node/notifier.h" #include "node/rpc/forwarder.h" #include "node/rpc/node_frontend.h" diff --git a/src/enclave/interface.h b/src/enclave/interface.h index b543566802c2..2a00a46c07df 100644 --- a/src/enclave/interface.h +++ b/src/enclave/interface.h @@ -37,6 +37,7 @@ struct CCFConfig consensus::Config consensus_config = {}; ccf::NodeInfoNetwork node_info_network = {}; std::string domain; + size_t snapshot_interval; struct SignatureIntervals { @@ -69,6 +70,7 @@ struct CCFConfig consensus_config, node_info_network, domain, + snapshot_interval, signature_intervals, genesis, joining); diff --git a/src/host/main.cpp b/src/host/main.cpp index 39401df68632..04255f085dc6 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -149,6 +149,15 @@ int main(int argc, char** argv) app.add_option("--snapshot-dir", snapshot_dir, "Snapshots directory") ->capture_default_str(); + size_t snapshot_min_tx = std::numeric_limits::max(); + app + .add_option( + "--snapshot-min-tx", + snapshot_min_tx, + "Minimum number of transactions between snapshots (experimental). " + "Defaults to no snapshot.") + ->capture_default_str(); + logger::Level host_log_level{logger::Level::INFO}; std::vector> level_map; for (int i = logger::TRACE; i < logger::MAX_LOG_LEVEL; i++) @@ -597,6 +606,7 @@ int main(int argc, char** argv) node_address.port, rpc_address.port}; ccf_config.domain = domain; + ccf_config.snapshot_interval = snapshot_min_tx; if (*start) { diff --git a/src/host/node_connections.h b/src/host/node_connections.h index 3ccb49c1a602..a4cdb7f4add0 100644 --- a/src/host/node_connections.h +++ b/src/host/node_connections.h @@ -6,7 +6,7 @@ #include "consensus/raft/raft_types.h" #include "host/timer.h" #include "ledger.h" -#include "node/nodetypes.h" +#include "node/node_types.h" #include "snapshot.h" #include "tcp.h" diff --git a/src/host/snapshot.h b/src/host/snapshot.h index 2fdb0cc0fb05..78921b745617 100644 --- a/src/host/snapshot.h +++ b/src/host/snapshot.h @@ -19,7 +19,7 @@ namespace asynchost static constexpr auto snapshot_file_prefix = "snapshot"; void write_snapshot( - size_t idx, const uint8_t* snapshot_data, size_t snapshot_size) + consensus::Index idx, const uint8_t* snapshot_data, size_t snapshot_size) { auto snapshot_file_name = fmt::format("{}.{}", snapshot_file_prefix, idx); auto full_snapshot_path = diff --git a/src/node/channels.h b/src/node/channels.h index a1a430d8ed4f..45cf3308bb41 100644 --- a/src/node/channels.h +++ b/src/node/channels.h @@ -5,7 +5,7 @@ #include "crypto/symmetric_key.h" #include "ds/logger.h" #include "entities.h" -#include "nodetypes.h" +#include "node_types.h" #include "tls/key_exchange.h" #include "tls/key_pair.h" diff --git a/src/node/node_state.h b/src/node/node_state.h index d8fe78a9cc62..64b510029162 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -27,6 +27,7 @@ #include "rpc/serialization.h" #include "secret_share.h" #include "share_manager.h" +#include "snapshotter.h" #include "timer.h" #include "tls/25519.h" #include "tls/client.h" @@ -68,8 +69,10 @@ namespace std namespace ccf { using RaftConsensusType = - raft::RaftConsensus; - using RaftType = raft::Raft; + raft::RaftConsensus; + using RaftType = + raft::Raft; + using PbftConsensusType = pbft::Pbft; template @@ -164,7 +167,8 @@ namespace ccf std::shared_ptr history; std::shared_ptr encryptor; - ShareManager share_manager; + ShareManager& share_manager; + std::shared_ptr snapshotter; // // join protocol @@ -240,6 +244,9 @@ namespace ccf create_node_cert(args.config); open_node_frontend(); + snapshotter = std::make_shared( + writer_factory, network.tables, args.config.snapshot_interval); + #ifdef GET_QUOTE if (network.consensus_type != ConsensusType::PBFT) { @@ -922,8 +929,7 @@ namespace ccf } default: - { - } + {} } } @@ -1510,6 +1516,7 @@ namespace ccf network.tables), std::make_unique(writer_factory), n2n_channels, + snapshotter, self, std::chrono::milliseconds(consensus_config.raft_request_timeout), std::chrono::milliseconds(consensus_config.raft_election_timeout), @@ -1559,8 +1566,7 @@ namespace ccf break; } default: - { - } + {} } } diff --git a/src/node/node_to_node.h b/src/node/node_to_node.h index f93821b72b09..3b946e58e387 100644 --- a/src/node/node_to_node.h +++ b/src/node/node_to_node.h @@ -5,7 +5,7 @@ #include "channels.h" #include "ds/serialized.h" #include "enclave/rpc_handler.h" -#include "nodetypes.h" +#include "node_types.h" #include #define FMT_HEADER_ONLY diff --git a/src/node/nodetypes.h b/src/node/node_types.h similarity index 100% rename from src/node/nodetypes.h rename to src/node/node_types.h diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h new file mode 100644 index 000000000000..c021cfdcd187 --- /dev/null +++ b/src/node/snapshotter.h @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "consensus/ledger_enclave_types.h" +#include "ds/logger.h" + +#include + +namespace ccf +{ + // TODO: Delete + // class AbstractSnapshotter + // { + // public: + // virtual ~AbstractSnapshotter() = default; + + // void snapshot(kv::Version version) = 0; + // }; + + // TODO: Probably need a lock on this???? + + class Snapshotter + { + private: + ringbuffer::WriterPtr to_host; + std::shared_ptr store; + + size_t last_snapshot_idx = 0; + size_t snapshot_interval; + + void record_snapshot( + kv::Version version, const std::vector& serialised_snapshot) + { + RINGBUFFER_WRITE_MESSAGE( + consensus::ledger_snapshot, to_host, version, serialised_snapshot); + } + + public: + Snapshotter( + ringbuffer::AbstractWriterFactory& writer_factory, + std::shared_ptr store_, + size_t snapshot_interval_) : + to_host(writer_factory.create_writer_to_outside()), + store(store_), + snapshot_interval(snapshot_interval_) + {} + + void snapshot(kv::Version version) + { + if ((unsigned)(version - last_snapshot_idx) > snapshot_interval) + { + LOG_FAIL_FMT("Creating snapshot at {} [NO EFFECT]", version); + auto snapshot = store->serialise_snapshot(version); + record_snapshot(version, snapshot); + + last_snapshot_idx = version; + } + LOG_FAIL_FMT("Not snapshotting..."); + } + }; +} \ No newline at end of file diff --git a/src/node/test/channel_stub.h b/src/node/test/channel_stub.h index 964105a39bf9..82c94d075a7c 100644 --- a/src/node/test/channel_stub.h +++ b/src/node/test/channel_stub.h @@ -3,7 +3,7 @@ #pragma once #include "node/entities.h" -#include "node/nodetypes.h" +#include "node/node_types.h" #include diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index f4d5d734c36c..19ef18979040 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -167,9 +167,14 @@ def cli_args(add=lambda x: None, parser=None, accept_unknown=False): ) parser.add_argument( "--ledger-chunk-min-bytes", - help="Minimum size (bytes) at which a new ledger chunk is created.", + help="Minimum size (bytes) at which a new ledger chunk is created", default="20KB", ) + parser.add_argument( + "--snapshot-min-tx", + help="Minimum number of transactions between two snapshots", + default=None, + ) add(parser) diff --git a/tests/infra/network.py b/tests/infra/network.py index c516cb64f4f2..4b6b9139f2be 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -75,6 +75,7 @@ class Network: "worker_threads", "ledger_chunk_min_bytes", "domain", + "snapshot_min_tx" ] # Maximum delay (seconds) for updates to propagate from the primary to backups diff --git a/tests/infra/remote.py b/tests/infra/remote.py index fd8ffb305bc1..0b6dc1512b73 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -564,6 +564,7 @@ def __init__( binary_dir=".", ledger_chunk_min_bytes=(5 * 1024 * 1024), domain=None, + snapshot_min_tx=None, ): """ Run a ccf binary on a remote host. @@ -649,6 +650,9 @@ def __init__( if domain: cmd += [f"--domain={domain}"] + if snapshot_min_tx: + cmd += [f"--snapshot-min-tx={snapshot_min_tx}"] + if start_type == StartType.new: cmd += [ "start", diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index 8a1e3cf14b26..67ab642d83fe 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -94,6 +94,8 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb ) as network: network.start_and_join(args) + + # TODO: Remove this for _ in range(1,1): e2e_logging.test(network, args, verify=False) # test_add_node_from_backup(network, args) From 12e01583bc6b7045bcbff75ab55450ec51393938 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 17:10:29 +0100 Subject: [PATCH 07/23] Snapshotter returns snapshot version to Raft --- src/consensus/raft/raft.h | 32 +++++++++----------------------- src/node/snapshotter.h | 18 ++++++++++++++---- tests/reconfiguration.py | 14 +++++--------- 3 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index a1e71b18080f..8d87c70dbcef 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -116,6 +116,9 @@ namespace raft Index commit_idx; TermHistory term_history; + // Snapshot + Index last_snapshot_idx; + // Volatile NodeId leader_id; std::unordered_set votes_for_me; @@ -171,6 +174,7 @@ namespace raft voted_for(NoNode), last_idx(0), commit_idx(0), + last_snapshot_idx(0), leader_id(NoNode), @@ -1166,33 +1170,15 @@ namespace raft LOG_DEBUG_FMT("Compacting..."); if (state == Leader) { - snapshotter->snapshot(idx); + auto snapshot_idx = snapshotter->snapshot(idx); + if (snapshot_idx.has_value()) + { + last_snapshot_idx = snapshot_idx.value(); + } } store->compact(idx); ledger->commit(idx); - // if (state == Leader) - // { - // if (idx - last_snapshot_idx > snapshot_interval) - // { - // LOG_FAIL_FMT("Snapshotting at {}", idx); - - // auto snap = store->snapshot(idx); - // if (snap.has_value()) - // { - // ledger->put_snapshot(idx, snap.value()); - // } - // else - // { - // LOG_FAIL_FMT( - // "Error generating snapshot at {}. Continuing normal - // operation.", idx); - // } - // last_snapshot_idx = idx; - // last_snapshot_term = current_term; - // LOG_FAIL_FMT("Snapshot done"); - // } - // } LOG_DEBUG_FMT("Commit on {}: {}", local_id, idx); // Examine all configurations that are followed by a globally committed diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index c021cfdcd187..47ecfb73b025 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -26,7 +26,7 @@ namespace ccf ringbuffer::WriterPtr to_host; std::shared_ptr store; - size_t last_snapshot_idx = 0; + kv::Version last_snapshot_idx = 0; size_t snapshot_interval; void record_snapshot( @@ -46,17 +46,27 @@ namespace ccf snapshot_interval(snapshot_interval_) {} - void snapshot(kv::Version version) + std::optional snapshot(kv::Version version) { + if (version < last_snapshot_idx) + { + LOG_FAIL_FMT( + "Cannot snapshot at {} which is earlier than last snapshot at {}", + version, + last_snapshot_idx); + return std::nullopt; + } + if ((unsigned)(version - last_snapshot_idx) > snapshot_interval) { - LOG_FAIL_FMT("Creating snapshot at {} [NO EFFECT]", version); auto snapshot = store->serialise_snapshot(version); record_snapshot(version, snapshot); last_snapshot_idx = version; + return version; } - LOG_FAIL_FMT("Not snapshotting..."); + + return std::nullopt; } }; } \ No newline at end of file diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index 67ab642d83fe..3931d4023c8c 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -94,16 +94,12 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb ) as network: network.start_and_join(args) - - # TODO: Remove this - for _ in range(1,1): - e2e_logging.test(network, args, verify=False) - # test_add_node_from_backup(network, args) + test_add_node_from_backup(network, args) + test_add_node(network, args) + test_add_node_untrusted_code(network, args) + test_retire_node(network, args) + test_add_as_many_pending_nodes(network, args) test_add_node(network, args) - # test_add_node_untrusted_code(network, args) - # test_retire_node(network, args) - # test_add_as_many_pending_nodes(network, args) - # test_add_node(network, args) if __name__ == "__main__": From 09428ee2b832a4a6051fb83fd3ba9d6508300c13 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 17:42:46 +0100 Subject: [PATCH 08/23] Fix unit tests --- src/consensus/raft/raft_types.h | 11 ----------- src/consensus/raft/test/driver.h | 4 +++- src/consensus/raft/test/logging_stub.h | 23 +++++++++++++++++++---- src/consensus/raft/test/main.cpp | 22 +++++++++++++++++++++- src/ds/champ_map.h | 6 +++--- src/host/main.cpp | 1 + src/host/node_connections.h | 4 ---- src/node/snapshotter.h | 11 ----------- tests/reconfiguration.py | 1 - 9 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/consensus/raft/raft_types.h b/src/consensus/raft/raft_types.h index b0b04850d4e6..012a40ad4fd8 100644 --- a/src/consensus/raft/raft_types.h +++ b/src/consensus/raft/raft_types.h @@ -29,7 +29,6 @@ namespace raft Term* term = nullptr) = 0; virtual void compact(Index v) = 0; virtual void rollback(Index v, std::optional t = std::nullopt) = 0; - virtual std::optional> snapshot(Index v) = 0; virtual void set_term(Term t) = 0; }; @@ -73,16 +72,6 @@ namespace raft } } - std::optional> snapshot(Index v) override - { - auto p = x.lock(); - if (p) - { - return p->serialise_snapshot(v); - } - return std::nullopt; - } - void set_term(Term t) override { auto p = x.lock(); diff --git a/src/consensus/raft/test/driver.h b/src/consensus/raft/test/driver.h index 1d85cfe2f9b4..3c9000c2d701 100644 --- a/src/consensus/raft/test/driver.h +++ b/src/consensus/raft/test/driver.h @@ -16,7 +16,8 @@ #include "logging_stub.h" using ms = std::chrono::milliseconds; -using TRaft = raft::Raft; +using TRaft = raft:: + Raft; using Store = raft::LoggingStubStore; using Adaptor = raft::Adaptor; @@ -46,6 +47,7 @@ class RaftDriver std::make_unique(kv), std::make_unique(node_id), std::make_shared(), + std::make_shared(), node_id, ms(10), ms(i * 100)); diff --git a/src/consensus/raft/test/logging_stub.h b/src/consensus/raft/test/logging_stub.h index b4dff6fb2d4a..266d99908471 100644 --- a/src/consensus/raft/test/logging_stub.h +++ b/src/consensus/raft/test/logging_stub.h @@ -6,6 +6,7 @@ #include "consensus/raft/raft_types.h" #include +#include #include namespace raft @@ -87,32 +88,36 @@ namespace raft void close_all_outgoing() {} - void send_authenticated( + bool send_authenticated( const ccf::NodeMsgType& msg_type, NodeId to, const RequestVote& data) { sent_request_vote.push_back(std::make_pair(to, data)); + return true; } - void send_authenticated( + bool send_authenticated( const ccf::NodeMsgType& msg_type, NodeId to, const AppendEntries& data) { sent_append_entries.push_back(std::make_pair(to, data)); + return true; } - void send_authenticated( + bool send_authenticated( const ccf::NodeMsgType& msg_type, NodeId to, const RequestVoteResponse& data) { sent_request_vote_response.push_back(std::make_pair(to, data)); + return true; } - void send_authenticated( + bool send_authenticated( const ccf::NodeMsgType& msg_type, NodeId to, const AppendEntriesResponse& data) { sent_append_entries_response.push_back(std::make_pair(to, data)); + return true; } size_t sent_msg_count() const @@ -184,4 +189,14 @@ namespace raft return kv::DeserialiseSuccess::PASS_SIGNATURE; } }; + + class StubSnashotter + { + public: + std::optional snapshot(kv::Version version) + { + // For now, do not test snapshots in unit tests + return std::nullopt; + } + }; } \ No newline at end of file diff --git a/src/consensus/raft/test/main.cpp b/src/consensus/raft/test/main.cpp index 444c51c9b1f3..128e71eb5263 100644 --- a/src/consensus/raft/test/main.cpp +++ b/src/consensus/raft/test/main.cpp @@ -14,7 +14,8 @@ using namespace std; using ms = std::chrono::milliseconds; -using TRaft = raft::Raft; +using TRaft = raft:: + Raft; using Store = raft::LoggingStubStore; using StoreSig = raft::LoggingStubStoreSig; using Adaptor = raft::Adaptor; @@ -29,6 +30,7 @@ DOCTEST_TEST_CASE("Single node startup" * doctest::test_suite("single")) std::make_unique(kv_store), std::make_unique(node_id), std::make_shared(), + std::make_shared(), node_id, ms(10), election_timeout); @@ -65,6 +67,7 @@ DOCTEST_TEST_CASE("Single node commit" * doctest::test_suite("single")) std::make_unique(kv_store), std::make_unique(node_id), std::make_shared(), + std::make_shared(), node_id, ms(10), election_timeout); @@ -110,6 +113,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -117,6 +121,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -124,6 +129,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -277,6 +283,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -284,6 +291,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -291,6 +299,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -402,6 +411,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -409,6 +419,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -416,6 +427,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -514,6 +526,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple")) std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -521,6 +534,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple")) std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -662,6 +676,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -669,6 +684,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -676,6 +692,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -812,6 +829,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -819,6 +837,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -826,6 +845,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); diff --git a/src/ds/champ_map.h b/src/ds/champ_map.h index a40aae120fb3..87f46b436cb3 100644 --- a/src/ds/champ_map.h +++ b/src/ds/champ_map.h @@ -373,7 +373,7 @@ namespace champ return map; } - size_t get_size() const + size_t size() const { return map_size; } @@ -470,7 +470,7 @@ namespace champ void serialize(uint8_t* data) { std::vector ordered_state; - ordered_state.reserve(map.get_size()); + ordered_state.reserve(map.size()); size_t size = 0; map.foreach([&](auto& key, auto& value) { @@ -500,7 +500,7 @@ namespace champ "size:{}, map->size:{} ==> count:{}, vect:{}", size, map.get_serialized_size(), - map.get_size(), + map.size(), ordered_state.size()); serialized_buffer = CBuffer(data, map.get_serialized_size()); diff --git a/src/host/main.cpp b/src/host/main.cpp index 04255f085dc6..66b169fc74c9 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -14,6 +14,7 @@ #include "notify_connections.h" #include "rpc_connections.h" #include "sig_term.h" +#include "snapshot.h" #include "ticker.h" #include "time_updater.h" #include "version.h" diff --git a/src/host/node_connections.h b/src/host/node_connections.h index a4cdb7f4add0..58884916bb37 100644 --- a/src/host/node_connections.h +++ b/src/host/node_connections.h @@ -7,7 +7,6 @@ #include "host/timer.h" #include "ledger.h" #include "node/node_types.h" -#include "snapshot.h" #include "tcp.h" #include @@ -254,9 +253,6 @@ namespace asynchost uint32_t frame = (uint32_t)size_to_send; std::optional> framed_entries = std::nullopt; - // TODO: If (ae.prev_idx + 1) is a snapshot, append that instead - // (with length-prefix), then all other append entries after that. - framed_entries = ledger.read_framed_entries(ae.prev_idx + 1, ae.idx); if (framed_entries.has_value()) diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 47ecfb73b025..3fdee2df4864 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -9,17 +9,6 @@ namespace ccf { - // TODO: Delete - // class AbstractSnapshotter - // { - // public: - // virtual ~AbstractSnapshotter() = default; - - // void snapshot(kv::Version version) = 0; - // }; - - // TODO: Probably need a lock on this???? - class Snapshotter { private: diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index 3931d4023c8c..95132ce511fa 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -5,7 +5,6 @@ import infra.proc import suite.test_requirements as reqs import time -import e2e_logging from loguru import logger as LOG From bcc8b3105345f9ace517ad1d3fd3048acc127589 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 17:57:01 +0100 Subject: [PATCH 09/23] Format --- src/consensus/raft/raft.h | 3 ++- src/node/node_state.h | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 8d87c70dbcef..a849f4eb00a9 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -414,7 +414,8 @@ namespace raft break; default: - {} + { + } } } diff --git a/src/node/node_state.h b/src/node/node_state.h index 64b510029162..c4673b059eaa 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -929,7 +929,8 @@ namespace ccf } default: - {} + { + } } } @@ -1566,7 +1567,8 @@ namespace ccf break; } default: - {} + { + } } } From 9a8aa43cf00de02e509e5cfd9aa4e5651dd9232b Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 14 Aug 2020 18:01:09 +0100 Subject: [PATCH 10/23] black --- tests/infra/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/infra/network.py b/tests/infra/network.py index 4b6b9139f2be..b7f72f91d42e 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -75,7 +75,7 @@ class Network: "worker_threads", "ledger_chunk_min_bytes", "domain", - "snapshot_min_tx" + "snapshot_min_tx", ] # Maximum delay (seconds) for updates to propagate from the primary to backups From f4683e037a8747f747ffff391b9be769c5a9d972 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 09:29:05 +0100 Subject: [PATCH 11/23] SnaPshotter --- src/consensus/raft/test/driver.h | 4 +-- src/consensus/raft/test/logging_stub.h | 2 +- src/consensus/raft/test/main.cpp | 40 +++++++++++++------------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/consensus/raft/test/driver.h b/src/consensus/raft/test/driver.h index 3c9000c2d701..e262a1894358 100644 --- a/src/consensus/raft/test/driver.h +++ b/src/consensus/raft/test/driver.h @@ -17,7 +17,7 @@ using ms = std::chrono::milliseconds; using TRaft = raft:: - Raft; + Raft; using Store = raft::LoggingStubStore; using Adaptor = raft::Adaptor; @@ -47,7 +47,7 @@ class RaftDriver std::make_unique(kv), std::make_unique(node_id), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id, ms(10), ms(i * 100)); diff --git a/src/consensus/raft/test/logging_stub.h b/src/consensus/raft/test/logging_stub.h index 266d99908471..d5535d7fdeaa 100644 --- a/src/consensus/raft/test/logging_stub.h +++ b/src/consensus/raft/test/logging_stub.h @@ -190,7 +190,7 @@ namespace raft } }; - class StubSnashotter + class StubSnapshotter { public: std::optional snapshot(kv::Version version) diff --git a/src/consensus/raft/test/main.cpp b/src/consensus/raft/test/main.cpp index 128e71eb5263..5cd736203bc6 100644 --- a/src/consensus/raft/test/main.cpp +++ b/src/consensus/raft/test/main.cpp @@ -15,7 +15,7 @@ using namespace std; using ms = std::chrono::milliseconds; using TRaft = raft:: - Raft; + Raft; using Store = raft::LoggingStubStore; using StoreSig = raft::LoggingStubStoreSig; using Adaptor = raft::Adaptor; @@ -30,7 +30,7 @@ DOCTEST_TEST_CASE("Single node startup" * doctest::test_suite("single")) std::make_unique(kv_store), std::make_unique(node_id), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id, ms(10), election_timeout); @@ -67,7 +67,7 @@ DOCTEST_TEST_CASE("Single node commit" * doctest::test_suite("single")) std::make_unique(kv_store), std::make_unique(node_id), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id, ms(10), election_timeout); @@ -113,7 +113,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -121,7 +121,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -129,7 +129,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -283,7 +283,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -291,7 +291,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -299,7 +299,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -411,7 +411,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -419,7 +419,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -427,7 +427,7 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple")) std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -526,7 +526,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple")) std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -534,7 +534,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple")) std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -676,7 +676,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -684,7 +684,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -692,7 +692,7 @@ DOCTEST_TEST_CASE("Exceed append entries limit") std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); @@ -829,7 +829,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store0), std::make_unique(node_id0), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id0, request_timeout, ms(20)); @@ -837,7 +837,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store1), std::make_unique(node_id1), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id1, request_timeout, ms(100)); @@ -845,7 +845,7 @@ DOCTEST_TEST_CASE( std::make_unique(kv_store2), std::make_unique(node_id2), std::make_shared(), - std::make_shared(), + std::make_shared(), node_id2, request_timeout, ms(50)); From 432675cbc96b1539d6c2962828409668a81f392e Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 09:43:32 +0100 Subject: [PATCH 12/23] Unsigned idx --- src/consensus/ledger_enclave_types.h | 2 +- src/node/snapshotter.h | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/consensus/ledger_enclave_types.h b/src/consensus/ledger_enclave_types.h index d7555c760e30..cbcca6eb3b05 100644 --- a/src/consensus/ledger_enclave_types.h +++ b/src/consensus/ledger_enclave_types.h @@ -6,7 +6,7 @@ namespace consensus { - using Index = int64_t; + using Index = uint64_t; enum LedgerRequestPurpose : uint8_t { diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 3fdee2df4864..55ce318530c9 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -15,14 +15,14 @@ namespace ccf ringbuffer::WriterPtr to_host; std::shared_ptr store; - kv::Version last_snapshot_idx = 0; + consensus::Index last_snapshot_idx = 0; size_t snapshot_interval; void record_snapshot( - kv::Version version, const std::vector& serialised_snapshot) + consensus::Index idx, const std::vector& serialised_snapshot) { RINGBUFFER_WRITE_MESSAGE( - consensus::ledger_snapshot, to_host, version, serialised_snapshot); + consensus::ledger_snapshot, to_host, idx, serialised_snapshot); } public: @@ -35,24 +35,25 @@ namespace ccf snapshot_interval(snapshot_interval_) {} - std::optional snapshot(kv::Version version) + std::optional snapshot(consensus::Index idx) { - if (version < last_snapshot_idx) + if (idx < last_snapshot_idx) { LOG_FAIL_FMT( - "Cannot snapshot at {} which is earlier than last snapshot at {}", - version, + "Cannot snapshot at idx {} which is earlier than last snapshot idx " + "{}", + idx, last_snapshot_idx); return std::nullopt; } - if ((unsigned)(version - last_snapshot_idx) > snapshot_interval) + if (idx - last_snapshot_idx > snapshot_interval) { - auto snapshot = store->serialise_snapshot(version); - record_snapshot(version, snapshot); + auto snapshot = store->serialise_snapshot(idx); + record_snapshot(idx, snapshot); - last_snapshot_idx = version; - return version; + last_snapshot_idx = idx; + return idx; } return std::nullopt; From 33b4cb1222e1d40841f2996a2685081457ea69a3 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 14:09:51 +0100 Subject: [PATCH 13/23] snapshot_min_tx -> snapshot_max_tx --- src/consensus/raft/raft.h | 1 - src/host/main.cpp | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index a849f4eb00a9..11278d726550 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -116,7 +116,6 @@ namespace raft Index commit_idx; TermHistory term_history; - // Snapshot Index last_snapshot_idx; // Volatile diff --git a/src/host/main.cpp b/src/host/main.cpp index 66b169fc74c9..a934b98fd909 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -150,12 +150,12 @@ int main(int argc, char** argv) app.add_option("--snapshot-dir", snapshot_dir, "Snapshots directory") ->capture_default_str(); - size_t snapshot_min_tx = std::numeric_limits::max(); + size_t snapshot_max_tx = std::numeric_limits::max(); app .add_option( - "--snapshot-min-tx", - snapshot_min_tx, - "Minimum number of transactions between snapshots (experimental). " + "--snapshot-max-tx", + snapshot_max_tx, + "Maximum number of transactions between snapshots (experimental). " "Defaults to no snapshot.") ->capture_default_str(); @@ -607,7 +607,7 @@ int main(int argc, char** argv) node_address.port, rpc_address.port}; ccf_config.domain = domain; - ccf_config.snapshot_interval = snapshot_min_tx; + ccf_config.snapshot_interval = snapshot_max_tx; if (*start) { From 8920fe278ae6649f8d6e8e331e758a134eed6523 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 14:34:28 +0100 Subject: [PATCH 14/23] Remove type of Tmsg when adding task --- src/consensus/pbft/libbyz/client_proxy.h | 4 ++-- src/consensus/pbft/libbyz/replica.cpp | 7 +++---- src/consensus/pbft/pbft.h | 20 +++++++++----------- src/consensus/pbft/pbft_config.h | 4 ++-- src/enclave/enclave.h | 2 +- src/enclave/tls_endpoint.h | 16 ++++++---------- src/http/http_endpoint.h | 2 +- 7 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/consensus/pbft/libbyz/client_proxy.h b/src/consensus/pbft/libbyz/client_proxy.h index d4e89c04ed41..4e2c02a9176c 100644 --- a/src/consensus/pbft/libbyz/client_proxy.h +++ b/src/consensus/pbft/libbyz/client_proxy.h @@ -281,7 +281,7 @@ bool ClientProxy::send_request( if (threading::ThreadMessaging::thread_count > 1) { - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( threading::ThreadMessaging::main_thread, std::move(msg)); } else @@ -427,7 +427,7 @@ void ClientProxy::recv_reply(Reply* reply) if (threading::ThreadMessaging::thread_count > 1) { - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( ctx->reply_thread, std::move(msg)); } else diff --git a/src/consensus/pbft/libbyz/replica.cpp b/src/consensus/pbft/libbyz/replica.cpp index cfb4dbe4abe8..cdcfae2e01c3 100644 --- a/src/consensus/pbft/libbyz/replica.cpp +++ b/src/consensus/pbft/libbyz/replica.cpp @@ -3,8 +3,6 @@ // Copyright (c) 2000, 2001 Miguel Castro, Rodrigo Rodrigues, Barbara Liskov. // Licensed under the MIT license. -#include "replica.h" - #include "append_entries.h" #include "checkpoint.h" #include "commit.h" @@ -28,6 +26,7 @@ #include "prepared_cert.h" #include "principal.h" #include "query_stable.h" +#include "replica.h" #include "reply.h" #include "reply_stable.h" #include "request.h" @@ -224,7 +223,7 @@ static void pre_verify_cb(std::unique_ptr> req) resp->data.self = self; resp->data.result = self->pre_verify(m); - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( threading::ThreadMessaging::main_thread, std::move(resp)); } @@ -353,7 +352,7 @@ void Replica::receive_message(const uint8_t* data, uint32_t size) msg->data.m = m; msg->data.self = this; - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( target_thread, std::move(msg)); } else diff --git a/src/consensus/pbft/pbft.h b/src/consensus/pbft/pbft.h index d3505c746d3e..dd4de77d664b 100644 --- a/src/consensus/pbft/pbft.h +++ b/src/consensus/pbft/pbft.h @@ -129,8 +129,8 @@ namespace pbft if (threading::ThreadMessaging::thread_count > 1) { uint16_t tid = threading::ThreadMessaging::get_execution_thread(to); - threading::ThreadMessaging::thread_messaging - .add_task(tid, std::move(tmsg)); + threading::ThreadMessaging::thread_messaging.add_task( + tid, std::move(tmsg)); } else { @@ -297,8 +297,8 @@ namespace pbft { uint16_t tid = threading::ThreadMessaging::get_execution_thread( ++execution_thread_counter); - threading::ThreadMessaging::thread_messaging - .add_task(tid, std::move(tmsg)); + threading::ThreadMessaging::thread_messaging.add_task( + tid, std::move(tmsg)); } else { @@ -702,9 +702,8 @@ namespace pbft msg, &recv_authenticated_msg_process_cb); if (threading::ThreadMessaging::thread_count > 1) { - threading::ThreadMessaging::thread_messaging - .add_task( - threading::ThreadMessaging::main_thread, std::move(msg)); + threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::main_thread, std::move(msg)); } else { @@ -749,10 +748,9 @@ namespace pbft if (threading::ThreadMessaging::thread_count > 1) { - threading::ThreadMessaging::thread_messaging - .add_task( - recv_nonce.tid % threading::ThreadMessaging::thread_count, - std::move(tmsg)); + threading::ThreadMessaging::thread_messaging.add_task( + recv_nonce.tid % threading::ThreadMessaging::thread_count, + std::move(tmsg)); } else { diff --git a/src/consensus/pbft/pbft_config.h b/src/consensus/pbft/pbft_config.h index 509bf4f9eee6..8501e1fad254 100644 --- a/src/consensus/pbft/pbft_config.h +++ b/src/consensus/pbft/pbft_config.h @@ -243,7 +243,7 @@ namespace pbft { threading::ThreadMessaging::thread_messaging .ChangeTmsgCallback(c, &ExecuteCb); - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( threading::ThreadMessaging::main_thread, std::move(c)); } else @@ -282,7 +282,7 @@ namespace pbft { tid = (threading::ThreadMessaging::thread_count - 1); } - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( tid, std::move(execution_ctx)); } else diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 985e5f970ed9..ff092406e50a 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -397,7 +397,7 @@ namespace enclave { auto msg = std::make_unique>(&init_thread_cb); msg->data.tid = threading::get_current_thread_id(); - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( msg->data.tid, std::move(msg)); threading::ThreadMessaging::thread_messaging.run(); diff --git a/src/enclave/tls_endpoint.h b/src/enclave/tls_endpoint.h index c6c3da286ade..ae13bf7298e5 100644 --- a/src/enclave/tls_endpoint.h +++ b/src/enclave/tls_endpoint.h @@ -180,8 +180,7 @@ namespace enclave } default: - { - } + {} } if (r < 0) @@ -235,7 +234,7 @@ namespace enclave msg->data.self = this->shared_from_this(); msg->data.data = data; - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( execution_thread, std::move(msg)); } @@ -323,7 +322,7 @@ namespace enclave auto msg = std::make_unique>(&close_cb); msg->data.self = this->shared_from_this(); - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( execution_thread, std::move(msg)); } @@ -373,8 +372,7 @@ namespace enclave } default: - { - } + {} } } @@ -473,8 +471,7 @@ namespace enclave return; default: - { - } + {} } status = status_; @@ -504,8 +501,7 @@ namespace enclave } default: - { - } + {} } } diff --git a/src/http/http_endpoint.h b/src/http/http_endpoint.h index 5fbe2576347a..caa060e1a033 100644 --- a/src/http/http_endpoint.h +++ b/src/http/http_endpoint.h @@ -45,7 +45,7 @@ namespace http msg->data.self = this->shared_from_this(); msg->data.data.assign(data, data + size); - threading::ThreadMessaging::thread_messaging.add_task( + threading::ThreadMessaging::thread_messaging.add_task( execution_thread, std::move(msg)); } From 07e3b76baf1d8216cd6f2aec148e1474d0c104df Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 14:35:49 +0100 Subject: [PATCH 15/23] And the other half... --- CMakeLists.txt | 2 +- tests/infra/e2e_args.py | 4 ++-- tests/infra/network.py | 2 +- tests/infra/remote.py | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1ac87a5333ba..63d6eb16a3dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -578,7 +578,7 @@ if(BUILD_TESTS) NAME reconfiguration_snapshot_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/reconfiguration.py CONSENSUS raft - ADDITIONAL_ARGS --snapshot-min-tx 10 + ADDITIONAL_ARGS --snapshot-max-tx 10 ) add_e2e_test( diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 19ef18979040..8aeee65e1526 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -171,8 +171,8 @@ def cli_args(add=lambda x: None, parser=None, accept_unknown=False): default="20KB", ) parser.add_argument( - "--snapshot-min-tx", - help="Minimum number of transactions between two snapshots", + "--snapshot-max-tx", + help="Maximum number of transactions between two snapshots", default=None, ) diff --git a/tests/infra/network.py b/tests/infra/network.py index b7f72f91d42e..8f5249ed9671 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -75,7 +75,7 @@ class Network: "worker_threads", "ledger_chunk_min_bytes", "domain", - "snapshot_min_tx", + "snapshot_max_tx", ] # Maximum delay (seconds) for updates to propagate from the primary to backups diff --git a/tests/infra/remote.py b/tests/infra/remote.py index 0b6dc1512b73..4f6d1e06792e 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -564,7 +564,7 @@ def __init__( binary_dir=".", ledger_chunk_min_bytes=(5 * 1024 * 1024), domain=None, - snapshot_min_tx=None, + snapshot_max_tx=None, ): """ Run a ccf binary on a remote host. @@ -650,8 +650,8 @@ def __init__( if domain: cmd += [f"--domain={domain}"] - if snapshot_min_tx: - cmd += [f"--snapshot-min-tx={snapshot_min_tx}"] + if snapshot_max_tx: + cmd += [f"--snapshot-max-tx={snapshot_max_tx}"] if start_type == StartType.new: cmd += [ From 7e24cc2df8aac712a1afb5c020aaad5c89171881 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 18 Aug 2020 17:23:51 +0100 Subject: [PATCH 16/23] Snapshot generation is async --- src/consensus/raft/raft.h | 10 ++---- src/node/snapshotter.h | 70 ++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 11278d726550..920f76d6a2cb 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -173,7 +173,6 @@ namespace raft voted_for(NoNode), last_idx(0), commit_idx(0), - last_snapshot_idx(0), leader_id(NoNode), @@ -413,8 +412,7 @@ namespace raft break; default: - { - } + {} } } @@ -1170,11 +1168,7 @@ namespace raft LOG_DEBUG_FMT("Compacting..."); if (state == Leader) { - auto snapshot_idx = snapshotter->snapshot(idx); - if (snapshot_idx.has_value()) - { - last_snapshot_idx = snapshot_idx.value(); - } + snapshotter->snapshot(idx); } store->compact(idx); ledger->commit(idx); diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 55ce318530c9..120c2ac46277 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -3,16 +3,21 @@ #pragma once #include "consensus/ledger_enclave_types.h" -#include "ds/logger.h" +#include "ds/ccf_assert.h" +#include "ds/spin_lock.h" +#include "ds/thread_messaging.h" #include namespace ccf { - class Snapshotter + class Snapshotter : public std::enable_shared_from_this { private: ringbuffer::WriterPtr to_host; + SpinLock lock; + size_t execution_thread; + std::shared_ptr store; consensus::Index last_snapshot_idx = 0; @@ -25,6 +30,29 @@ namespace ccf consensus::ledger_snapshot, to_host, idx, serialised_snapshot); } + struct SnapshotMsg + { + std::shared_ptr self; + consensus::Index idx; + }; + + static void snapshot_cb(std::unique_ptr> msg) + { + msg->data.self->snapshot_(msg->data.idx); + } + + void snapshot_(consensus::Index idx) + { + std::lock_guard guard(lock); + + auto snapshot = store->serialise_snapshot(idx); + record_snapshot(idx, snapshot); + + LOG_DEBUG_FMT("Snapshot successfully generated at idx {}", idx); + + last_snapshot_idx = idx; + } + public: Snapshotter( ringbuffer::AbstractWriterFactory& writer_factory, @@ -33,30 +61,34 @@ namespace ccf to_host(writer_factory.create_writer_to_outside()), store(store_), snapshot_interval(snapshot_interval_) - {} + { + // For now, always generate snapshots on first worker thread if there are + // more than one thread + // Warning: With 1+ worker threads, this still executes on the main thread + // as the worker threads are initialised after the Snapshotter is created + execution_thread = (threading::ThreadMessaging::thread_count > 1) ? 1 : 0; + } - std::optional snapshot(consensus::Index idx) + void snapshot(consensus::Index idx) { - if (idx < last_snapshot_idx) - { - LOG_FAIL_FMT( - "Cannot snapshot at idx {} which is earlier than last snapshot idx " - "{}", - idx, - last_snapshot_idx); - return std::nullopt; - } + std::lock_guard guard(lock); + + CCF_ASSERT_FMT( + idx >= last_snapshot_idx, + "Cannot snapshot at idx {} which is earlier than last snapshot idx " + "{}", + idx, + last_snapshot_idx); if (idx - last_snapshot_idx > snapshot_interval) { - auto snapshot = store->serialise_snapshot(idx); - record_snapshot(idx, snapshot); + auto msg = std::make_unique>(&snapshot_cb); + msg->data.self = shared_from_this(); + msg->data.idx = idx; - last_snapshot_idx = idx; - return idx; + threading::ThreadMessaging::thread_messaging.add_task( + execution_thread, std::move(msg)); } - - return std::nullopt; } }; } \ No newline at end of file From 143f3a82522a31bf816a12fe019320718b4eed5c Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 09:50:57 +0100 Subject: [PATCH 17/23] Commit snapshot evidence --- src/consensus/raft/test/logging_stub.h | 4 +- src/ds/thread_ids.h | 4 +- src/ds/thread_messaging.h | 4 +- src/enclave/main.cpp | 2 +- src/enclave/tls_endpoint.h | 2 +- src/node/entities.h | 1 + src/node/network_tables.h | 4 ++ src/node/node_state.h | 8 ++-- src/node/snapshot_evidence.h | 22 +++++++++++ src/node/snapshotter.h | 55 +++++++++++++++++++------- 10 files changed, 79 insertions(+), 27 deletions(-) create mode 100644 src/node/snapshot_evidence.h diff --git a/src/consensus/raft/test/logging_stub.h b/src/consensus/raft/test/logging_stub.h index d5535d7fdeaa..d9bbed0552c3 100644 --- a/src/consensus/raft/test/logging_stub.h +++ b/src/consensus/raft/test/logging_stub.h @@ -193,10 +193,10 @@ namespace raft class StubSnapshotter { public: - std::optional snapshot(kv::Version version) + void snapshot(Index) { // For now, do not test snapshots in unit tests - return std::nullopt; + return; } }; } \ No newline at end of file diff --git a/src/ds/thread_ids.h b/src/ds/thread_ids.h index 308e5366f7b5..391bf216872d 100644 --- a/src/ds/thread_ids.h +++ b/src/ds/thread_ids.h @@ -10,13 +10,15 @@ namespace threading { + static constexpr size_t MAIN_THREAD_ID = 0; + extern std::map thread_ids; static inline uint16_t get_current_thread_id() { if (thread_ids.empty()) { - return 0; + return MAIN_THREAD_ID; } const auto tid = std::this_thread::get_id(); diff --git a/src/ds/thread_messaging.h b/src/ds/thread_messaging.h index 293f042445f1..3251a2a421db 100644 --- a/src/ds/thread_messaging.h +++ b/src/ds/thread_messaging.h @@ -183,7 +183,7 @@ namespace threading public: static ThreadMessaging thread_messaging; static std::atomic thread_count; - static const uint16_t main_thread = 0; + static const uint16_t main_thread = MAIN_THREAD_ID; static const uint16_t max_num_threads = 24; @@ -238,7 +238,7 @@ namespace threading static uint16_t get_execution_thread(uint32_t i) { - uint16_t tid = 0; + uint16_t tid = MAIN_THREAD_ID; if (thread_count > 1) { tid = (i % (thread_count - 1)); diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp index aa19ee2b4bb6..97733f37606f 100644 --- a/src/enclave/main.cpp +++ b/src/enclave/main.cpp @@ -161,7 +161,7 @@ extern "C" LOG_INFO_FMT("All threads are ready!"); - if (tid == 0) + if (tid == threading::MAIN_THREAD_ID) { auto s = e.load()->run_main(); while (num_complete_threads != diff --git a/src/enclave/tls_endpoint.h b/src/enclave/tls_endpoint.h index ae13bf7298e5..e8d4d2139c7a 100644 --- a/src/enclave/tls_endpoint.h +++ b/src/enclave/tls_endpoint.h @@ -73,7 +73,7 @@ namespace enclave } else { - execution_thread = 0; + execution_thread = threading::MAIN_THREAD_ID; } ctx->set_bio(this, send_callback, recv_callback, dbg_callback); } diff --git a/src/node/entities.h b/src/node/entities.h index f8edabc20fdc..a68a5fec46af 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -85,6 +85,7 @@ namespace ccf static constexpr auto USER_CODE_IDS = "ccf.users.code_ids"; static constexpr auto CONFIGURATION = "ccf.config"; static constexpr auto SUBMITTED_SHARES = "ccf.submitted_shares"; + static constexpr auto SNAPSHOT_EVIDENCES = "ccf.snapshot_evidences"; }; } diff --git a/src/node/network_tables.h b/src/node/network_tables.h index 16782a8c95b9..f58c0230f58f 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -25,6 +25,7 @@ #include "service.h" #include "shares.h" #include "signatures.h" +#include "snapshot_evidence.h" #include "submitted_shares.h" #include "users.h" #include "values.h" @@ -86,6 +87,7 @@ namespace ccf Secrets& secrets; Signatures& signatures; ConsensusTable& consensus; + SnapshotEvidence& snapshot_evidences; // // Pbft related tables @@ -147,6 +149,8 @@ namespace ccf Tables::SIGNATURES, kv::SecurityDomain::PUBLIC)), consensus(tables->create( Tables::CONSENSUS, kv::SecurityDomain::PUBLIC)), + snapshot_evidences(tables->create( + Tables::SNAPSHOT_EVIDENCES, kv::SecurityDomain::PUBLIC)), pbft_requests_map( tables->create(pbft::Tables::PBFT_REQUESTS)), pbft_pre_prepares_map( diff --git a/src/node/node_state.h b/src/node/node_state.h index c4673b059eaa..1bb3c68f3c86 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -245,7 +245,7 @@ namespace ccf open_node_frontend(); snapshotter = std::make_shared( - writer_factory, network.tables, args.config.snapshot_interval); + writer_factory, network, args.config.snapshot_interval); #ifdef GET_QUOTE if (network.consensus_type != ConsensusType::PBFT) @@ -929,8 +929,7 @@ namespace ccf } default: - { - } + {} } } @@ -1567,8 +1566,7 @@ namespace ccf break; } default: - { - } + {} } } diff --git a/src/node/snapshot_evidence.h b/src/node/snapshot_evidence.h new file mode 100644 index 000000000000..d35506e92212 --- /dev/null +++ b/src/node/snapshot_evidence.h @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "crypto/hash.h" +#include "kv/map.h" + +#include + +namespace ccf +{ + struct SnapshotHash + { + crypto::Sha256Hash hash; + + MSGPACK_DEFINE(hash); + }; + + // As we only keep track of the latest snapshot, the key for the + // SnapshotEvidence table is always 0. + using SnapshotEvidence = kv::Map; +} \ No newline at end of file diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 120c2ac46277..4ec051931686 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -3,9 +3,11 @@ #pragma once #include "consensus/ledger_enclave_types.h" +#include "crypto/hash.h" #include "ds/ccf_assert.h" #include "ds/spin_lock.h" #include "ds/thread_messaging.h" +#include "node/snapshot_evidence.h" #include @@ -16,13 +18,28 @@ namespace ccf private: ringbuffer::WriterPtr to_host; SpinLock lock; - size_t execution_thread; - std::shared_ptr store; + NetworkState& network; consensus::Index last_snapshot_idx = 0; size_t snapshot_interval; + size_t get_execution_thread() + { + // For now, always generate snapshots on first worker thread if there are + // more than one thread. Otherwise, round robin on worker threads. + if (threading::ThreadMessaging::thread_count > 1) + { + static size_t generation_count = 0; + return (generation_count++ % threading::ThreadMessaging::thread_count) + + 1; + } + else + { + return threading::MAIN_THREAD_ID; + } + } + void record_snapshot( consensus::Index idx, const std::vector& serialised_snapshot) { @@ -45,29 +62,37 @@ namespace ccf { std::lock_guard guard(lock); - auto snapshot = store->serialise_snapshot(idx); - record_snapshot(idx, snapshot); + auto snapshot = network.tables->serialise_snapshot(idx); - LOG_DEBUG_FMT("Snapshot successfully generated at idx {}", idx); + kv::Tx tx; + auto view = tx.get_view(network.snapshot_evidences); + auto snapshot_hash = crypto::Sha256Hash(snapshot); + view->put(0, {snapshot_hash}); + auto rc = tx.commit(); + if (rc != kv::CommitSuccess::OK) + { + LOG_FAIL_FMT( + "Could not commit snapshot evidence for idx {}: {}", idx, rc); + return; + } + + record_snapshot(idx, snapshot); last_snapshot_idx = idx; + + LOG_DEBUG_FMT( + "Snapshot successfully generated for idx {}: {}", idx, snapshot_hash); } public: Snapshotter( ringbuffer::AbstractWriterFactory& writer_factory, - std::shared_ptr store_, + NetworkState& network_, size_t snapshot_interval_) : to_host(writer_factory.create_writer_to_outside()), - store(store_), + network(network_), snapshot_interval(snapshot_interval_) - { - // For now, always generate snapshots on first worker thread if there are - // more than one thread - // Warning: With 1+ worker threads, this still executes on the main thread - // as the worker threads are initialised after the Snapshotter is created - execution_thread = (threading::ThreadMessaging::thread_count > 1) ? 1 : 0; - } + {} void snapshot(consensus::Index idx) { @@ -87,7 +112,7 @@ namespace ccf msg->data.idx = idx; threading::ThreadMessaging::thread_messaging.add_task( - execution_thread, std::move(msg)); + get_execution_thread(), std::move(msg)); } } }; From a07e6b0cf45dc0e48c05a41786af0e350293ac29 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 10:31:40 +0100 Subject: [PATCH 18/23] Add snapshot idx to evidence table --- src/node/snapshot_evidence.h | 4 +++- src/node/snapshotter.h | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/node/snapshot_evidence.h b/src/node/snapshot_evidence.h index d35506e92212..7a15914dde42 100644 --- a/src/node/snapshot_evidence.h +++ b/src/node/snapshot_evidence.h @@ -3,6 +3,7 @@ #pragma once #include "crypto/hash.h" +#include "entities.h" #include "kv/map.h" #include @@ -12,8 +13,9 @@ namespace ccf struct SnapshotHash { crypto::Sha256Hash hash; + ObjectId seqno; - MSGPACK_DEFINE(hash); + MSGPACK_DEFINE(hash, seqno); }; // As we only keep track of the latest snapshot, the key for the diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 459f129cdd97..3c65d5c1a866 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -27,8 +27,8 @@ namespace ccf size_t get_execution_thread() { - // For now, always generate snapshots on first worker thread if there are - // more than one thread. Otherwise, round robin on worker threads. + // Generate on main thread if there are no worker threads. Otherwise, + // round robin on worker threads. if (threading::ThreadMessaging::thread_count > 1) { static size_t generation_count = 0; @@ -68,7 +68,7 @@ namespace ccf kv::Tx tx; auto view = tx.get_view(network.snapshot_evidences); auto snapshot_hash = crypto::Sha256Hash(snapshot); - view->put(0, {snapshot_hash}); + view->put(0, {snapshot_hash, idx}); auto rc = tx.commit(); if (rc != kv::CommitSuccess::OK) From e33694c2df0212d90300d44e2721f4ad69b6d82e Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 11:22:09 +0100 Subject: [PATCH 19/23] Split snapshot generation and serialisation --- src/kv/kv_types.h | 10 ++++++---- src/kv/snapshot.h | 23 ++++++++++++++++------- src/kv/store.h | 18 +++++++++++------- src/kv/test/kv_bench.cpp | 8 +++++--- src/kv/test/kv_snapshot.cpp | 18 ++++++++++++------ src/node/snapshot_evidence.h | 5 +++-- src/node/snapshotter.h | 33 ++++++++++++++++++++------------- src/node/test/snapshot.cpp | 13 +++++++++---- 8 files changed, 82 insertions(+), 46 deletions(-) diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 8de1309a7ccd..d021c9157073 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -408,9 +408,9 @@ namespace kv { public: virtual ~AbstractSnapshot() = default; - virtual void add_map_snapshot( - std::unique_ptr snapshot) = 0; - virtual std::vector serialise(KvStoreSerialiser& s) = 0; + virtual Version get_version() const = 0; + virtual std::vector serialise( + std::shared_ptr encryptor) = 0; }; virtual ~AbstractStore() {} @@ -436,7 +436,9 @@ namespace kv virtual CommitSuccess commit( const TxID& txid, PendingTx&& pending_tx, bool globally_committable) = 0; - virtual std::vector serialise_snapshot(Version v) = 0; + virtual std::unique_ptr snapshot(Version v) = 0; + virtual std::vector serialise_snapshot( + std::unique_ptr snapshot) = 0; virtual DeserialiseSuccess deserialise_snapshot( const std::vector& data) = 0; diff --git a/src/kv/snapshot.h b/src/kv/snapshot.h index 944b6d343d1e..3c5f7fa66312 100644 --- a/src/kv/snapshot.h +++ b/src/kv/snapshot.h @@ -9,14 +9,15 @@ namespace kv class StoreSnapshot : public AbstractStore::AbstractSnapshot { private: + Version version; + std::vector> snapshots; std::optional> hash_at_snapshot = std::nullopt; public: - StoreSnapshot() = default; + StoreSnapshot(Version version_) : version(version_) {} - void add_map_snapshot( - std::unique_ptr snapshot) override + void add_map_snapshot(std::unique_ptr snapshot) { snapshots.push_back(std::move(snapshot)); } @@ -26,11 +27,19 @@ namespace kv hash_at_snapshot = std::move(hash_at_snapshot_); } - std::vector serialise(KvStoreSerialiser& s) override + Version get_version() const { + return version; + } + + std::vector serialise( + std::shared_ptr encryptor) + { + KvStoreSerialiser serialiser(encryptor, version, true); + if (hash_at_snapshot.has_value()) { - s.serialise_raw(hash_at_snapshot.value()); + serialiser.serialise_raw(hash_at_snapshot.value()); } for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) @@ -39,12 +48,12 @@ namespace kv { if (it->get_security_domain() == domain) { - it->serialise(s); + it->serialise(serialiser); } } } - return s.get_raw_data(); + return serialiser.get_raw_data(); } }; } \ No newline at end of file diff --git a/src/kv/store.h b/src/kv/store.h index cc62f1b874ae..fa1cfd4fcc8e 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -220,7 +220,7 @@ namespace kv return *result; } - std::vector serialise_snapshot(Version v) override + std::unique_ptr snapshot(Version v) override { CCF_ASSERT_FMT( v >= commit_version(), @@ -236,7 +236,7 @@ namespace kv v, current_version()); - StoreSnapshot snapshot; + auto snapshot = std::make_unique(v); { std::lock_guard mguard(maps_lock); @@ -248,13 +248,13 @@ namespace kv for (auto& map : maps) { - snapshot.add_map_snapshot(map.second->snapshot(v)); + snapshot->add_map_snapshot(map.second->snapshot(v)); } auto h = get_history(); if (h) { - snapshot.add_hash_at_snapshot(h->get_raw_leaf(v)); + snapshot->add_hash_at_snapshot(h->get_raw_leaf(v)); } for (auto& map : maps) @@ -263,10 +263,14 @@ namespace kv } } - auto e = get_encryptor(); - KvStoreSerialiser serialiser(e, v, true); + return snapshot; + } - return snapshot.serialise(serialiser); + std::vector serialise_snapshot( + std::unique_ptr snapshot) override + { + auto e = get_encryptor(); + return snapshot->serialise(e); } DeserialiseSuccess deserialise_snapshot( diff --git a/src/kv/test/kv_bench.cpp b/src/kv/test/kv_bench.cpp index d190dbe2e6d0..d621d8155d15 100644 --- a/src/kv/test/kv_bench.cpp +++ b/src/kv/test/kv_bench.cpp @@ -192,7 +192,8 @@ static void ser_snap(picobench::state& s) throw std::logic_error("Transaction commit failed: " + std::to_string(rc)); s.start_timer(); - kv_store.serialise_snapshot(tx.commit_version()); + auto snap = kv_store.snapshot(tx.commit_version()); + kv_store.serialise_snapshot(std::move(snap)); s.stop_timer(); } @@ -234,10 +235,11 @@ static void des_snap(picobench::state& s) if (rc != kv::CommitSuccess::OK) throw std::logic_error("Transaction commit failed: " + std::to_string(rc)); - auto snapshot = kv_store.serialise_snapshot(tx.commit_version()); + auto snap = kv_store.snapshot(tx.commit_version()); + auto serialised_snap = kv_store.serialise_snapshot(std::move(snap)); s.start_timer(); - kv_store2.deserialise_snapshot(snapshot); + kv_store2.deserialise_snapshot(serialised_snap); s.stop_timer(); } diff --git a/src/kv/test/kv_snapshot.cpp b/src/kv/test/kv_snapshot.cpp index 59b585a8ab34..7376213e9591 100644 --- a/src/kv/test/kv_snapshot.cpp +++ b/src/kv/test/kv_snapshot.cpp @@ -45,7 +45,9 @@ TEST_CASE("Simple snapshot" * doctest::test_suite("snapshot")) // Do not commit tx3 } - auto first_snapshot = store.serialise_snapshot(first_snapshot_version); + auto first_snapshot = store.snapshot(first_snapshot_version); + auto first_serialised_snapshot = + store.serialise_snapshot(std::move(first_snapshot)); INFO("Apply snapshot at 1 to new store"); { @@ -53,7 +55,7 @@ TEST_CASE("Simple snapshot" * doctest::test_suite("snapshot")) new_store.clone_schema(store); REQUIRE_EQ( - new_store.deserialise_snapshot(first_snapshot), + new_store.deserialise_snapshot(first_serialised_snapshot), kv::DeserialiseSuccess::PASS); REQUIRE_EQ(new_store.current_version(), 1); @@ -75,12 +77,15 @@ TEST_CASE("Simple snapshot" * doctest::test_suite("snapshot")) REQUIRE(!v.has_value()); } - auto second_snapshot = store.serialise_snapshot(second_snapshot_version); + auto second_snapshot = store.snapshot(second_snapshot_version); + auto second_serialised_snapshot = + store.serialise_snapshot(std::move(second_snapshot)); + INFO("Apply snapshot at 2 to new store"); { kv::Store new_store; new_store.clone_schema(store); - new_store.deserialise_snapshot(second_snapshot); + new_store.deserialise_snapshot(second_serialised_snapshot); REQUIRE_EQ(new_store.current_version(), 2); auto new_string_map = new_store.get("string_map"); @@ -127,7 +132,8 @@ TEST_CASE( snapshot_version = tx2.commit_version(); } - auto snapshot = store.serialise_snapshot(snapshot_version); + auto snapshot = store.snapshot(snapshot_version); + auto serialised_snapshot = store.serialise_snapshot(std::move(snapshot)); INFO("Apply snapshot while committing a transaction"); { @@ -140,7 +146,7 @@ TEST_CASE( view->put("in", "flight"); // tx is not committed until the snapshot is deserialised - new_store.deserialise_snapshot(snapshot); + new_store.deserialise_snapshot(serialised_snapshot); // Transaction conflicts as snapshot was applied while transaction was in // flight diff --git a/src/node/snapshot_evidence.h b/src/node/snapshot_evidence.h index 7a15914dde42..52d0bbb8f823 100644 --- a/src/node/snapshot_evidence.h +++ b/src/node/snapshot_evidence.h @@ -4,6 +4,7 @@ #include "crypto/hash.h" #include "entities.h" +#include "kv/kv_types.h" #include "kv/map.h" #include @@ -13,9 +14,9 @@ namespace ccf struct SnapshotHash { crypto::Sha256Hash hash; - ObjectId seqno; + kv::Version version; - MSGPACK_DEFINE(hash, seqno); + MSGPACK_DEFINE(hash, version); }; // As we only keep track of the latest snapshot, the key for the diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 3c65d5c1a866..39a99c34f055 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -8,10 +8,9 @@ #include "ds/logger.h" #include "ds/spin_lock.h" #include "ds/thread_messaging.h" +#include "kv/kv_types.h" #include "node/snapshot_evidence.h" -#include - namespace ccf { class Snapshotter : public std::enable_shared_from_this @@ -51,38 +50,46 @@ namespace ccf struct SnapshotMsg { std::shared_ptr self; - consensus::Index idx; + std::unique_ptr snapshot; }; static void snapshot_cb(std::unique_ptr> msg) { - msg->data.self->snapshot_(msg->data.idx); + msg->data.self->snapshot_(std::move(msg->data.snapshot)); } - void snapshot_(consensus::Index idx) + void snapshot_( + std::unique_ptr snapshot) { std::lock_guard guard(lock); - auto snapshot = network.tables->serialise_snapshot(idx); + auto snapshot_idx = snapshot->get_version(); + + auto serialised_snapshot = + network.tables->serialise_snapshot(std::move(snapshot)); kv::Tx tx; auto view = tx.get_view(network.snapshot_evidences); - auto snapshot_hash = crypto::Sha256Hash(snapshot); - view->put(0, {snapshot_hash, idx}); + auto snapshot_hash = crypto::Sha256Hash(serialised_snapshot); + view->put(0, {snapshot_hash, snapshot_idx}); auto rc = tx.commit(); if (rc != kv::CommitSuccess::OK) { LOG_FAIL_FMT( - "Could not commit snapshot evidence for idx {}: {}", idx, rc); + "Could not commit snapshot evidence for idx {}: {}", + snapshot_idx, + rc); return; } - record_snapshot(idx, snapshot); - last_snapshot_idx = idx; + record_snapshot(snapshot_idx, serialised_snapshot); + last_snapshot_idx = snapshot_idx; LOG_DEBUG_FMT( - "Snapshot successfully generated for idx {}: {}", idx, snapshot_hash); + "Snapshot successfully generated for idx {}: {}", + snapshot_idx, + snapshot_hash); } public: @@ -110,7 +117,7 @@ namespace ccf { auto msg = std::make_unique>(&snapshot_cb); msg->data.self = shared_from_this(); - msg->data.idx = idx; + msg->data.snapshot = network.tables->snapshot(idx); threading::ThreadMessaging::thread_messaging.add_task( get_execution_thread(), std::move(msg)); diff --git a/src/node/test/snapshot.cpp b/src/node/test/snapshot.cpp index c4635f262b79..ef0d47214b7f 100644 --- a/src/node/test/snapshot.cpp +++ b/src/node/test/snapshot.cpp @@ -97,19 +97,24 @@ TEST_CASE("Snapshot with merkle tree" * doctest::test_suite("snapshot")) INFO("Apply snapshot taken before any signature was emitted"); { - auto snapshot = source_store.serialise_snapshot(snapshot_version - 1); + auto snapshot = source_store.snapshot(snapshot_version - 1); + auto serialised_snapshot = + source_store.serialise_snapshot(std::move(snapshot)); // There is no signature to read to seed the target history REQUIRE( - target_store.deserialise_snapshot(snapshot) == + target_store.deserialise_snapshot(serialised_snapshot) == kv::DeserialiseSuccess::FAILED); } INFO("Apply snapshot taken at signature"); { - auto snapshot = source_store.serialise_snapshot(snapshot_version); + auto snapshot = source_store.snapshot(snapshot_version); + auto serialised_snapshot = + source_store.serialise_snapshot(std::move(snapshot)); + REQUIRE( - target_store.deserialise_snapshot(snapshot) == + target_store.deserialise_snapshot(serialised_snapshot) == kv::DeserialiseSuccess::PASS); } From 426b580b9526c2a410feb0d85bc2160d4c929269 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 11:22:17 +0100 Subject: [PATCH 20/23] Format --- src/consensus/pbft/libbyz/replica.cpp | 3 ++- src/consensus/raft/raft.h | 3 ++- src/enclave/tls_endpoint.h | 12 ++++++++---- src/node/node_state.h | 6 ++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/consensus/pbft/libbyz/replica.cpp b/src/consensus/pbft/libbyz/replica.cpp index cdcfae2e01c3..8681b8aa6edf 100644 --- a/src/consensus/pbft/libbyz/replica.cpp +++ b/src/consensus/pbft/libbyz/replica.cpp @@ -3,6 +3,8 @@ // Copyright (c) 2000, 2001 Miguel Castro, Rodrigo Rodrigues, Barbara Liskov. // Licensed under the MIT license. +#include "replica.h" + #include "append_entries.h" #include "checkpoint.h" #include "commit.h" @@ -26,7 +28,6 @@ #include "prepared_cert.h" #include "principal.h" #include "query_stable.h" -#include "replica.h" #include "reply.h" #include "reply_stable.h" #include "request.h" diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 3cc1d0ced400..ff7f7f583d15 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -413,7 +413,8 @@ namespace raft break; default: - {} + { + } } } diff --git a/src/enclave/tls_endpoint.h b/src/enclave/tls_endpoint.h index e8d4d2139c7a..9e3c710bde3b 100644 --- a/src/enclave/tls_endpoint.h +++ b/src/enclave/tls_endpoint.h @@ -180,7 +180,8 @@ namespace enclave } default: - {} + { + } } if (r < 0) @@ -372,7 +373,8 @@ namespace enclave } default: - {} + { + } } } @@ -471,7 +473,8 @@ namespace enclave return; default: - {} + { + } } status = status_; @@ -501,7 +504,8 @@ namespace enclave } default: - {} + { + } } } diff --git a/src/node/node_state.h b/src/node/node_state.h index d6650a553f58..4e2387159664 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -929,7 +929,8 @@ namespace ccf } default: - {} + { + } } } @@ -1565,7 +1566,8 @@ namespace ccf break; } default: - {} + { + } } } From e2332e380ba8984d36f5d97981439dc51d718895 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 14:10:26 +0100 Subject: [PATCH 21/23] Actually remove last_snapshot_idx from Raft --- src/consensus/raft/raft.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index ff7f7f583d15..d51ea04170d9 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -116,8 +116,6 @@ namespace raft Index commit_idx; TermHistory term_history; - Index last_snapshot_idx; - // Volatile NodeId leader_id; std::unordered_set votes_for_me; @@ -173,7 +171,6 @@ namespace raft voted_for(NoNode), last_idx(0), commit_idx(0), - last_snapshot_idx(0), leader_id(NoNode), @@ -413,8 +410,7 @@ namespace raft break; default: - { - } + {} } } From ea7dceff5b500ff01cb1304a13b33d028e708c05 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 14:20:37 +0100 Subject: [PATCH 22/23] snapsot evidence singular --- src/node/entities.h | 2 +- src/node/network_tables.h | 6 +++--- src/node/snapshotter.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/node/entities.h b/src/node/entities.h index a68a5fec46af..2828098a0f67 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -85,7 +85,7 @@ namespace ccf static constexpr auto USER_CODE_IDS = "ccf.users.code_ids"; static constexpr auto CONFIGURATION = "ccf.config"; static constexpr auto SUBMITTED_SHARES = "ccf.submitted_shares"; - static constexpr auto SNAPSHOT_EVIDENCES = "ccf.snapshot_evidences"; + static constexpr auto SNAPSHOT_EVIDENCE = "ccf.snapshot_evidence"; }; } diff --git a/src/node/network_tables.h b/src/node/network_tables.h index f58c0230f58f..d020302463ab 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -87,7 +87,7 @@ namespace ccf Secrets& secrets; Signatures& signatures; ConsensusTable& consensus; - SnapshotEvidence& snapshot_evidences; + SnapshotEvidence& snapshot_evidence; // // Pbft related tables @@ -149,8 +149,8 @@ namespace ccf Tables::SIGNATURES, kv::SecurityDomain::PUBLIC)), consensus(tables->create( Tables::CONSENSUS, kv::SecurityDomain::PUBLIC)), - snapshot_evidences(tables->create( - Tables::SNAPSHOT_EVIDENCES, kv::SecurityDomain::PUBLIC)), + snapshot_evidence(tables->create( + Tables::SNAPSHOT_EVIDENCE, kv::SecurityDomain::PUBLIC)), pbft_requests_map( tables->create(pbft::Tables::PBFT_REQUESTS)), pbft_pre_prepares_map( diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index 39a99c34f055..dd98e9b172f3 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -69,7 +69,7 @@ namespace ccf network.tables->serialise_snapshot(std::move(snapshot)); kv::Tx tx; - auto view = tx.get_view(network.snapshot_evidences); + auto view = tx.get_view(network.snapshot_evidence); auto snapshot_hash = crypto::Sha256Hash(serialised_snapshot); view->put(0, {snapshot_hash, snapshot_idx}); From 21908b2872267048cd5c6ebcbcc9b2a15edd2e75 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 19 Aug 2020 14:51:33 +0100 Subject: [PATCH 23/23] Format --- src/consensus/raft/raft.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index d51ea04170d9..68f819df9c9c 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -410,7 +410,8 @@ namespace raft break; default: - {} + { + } } }