Fix order of block deletes relative to adds on ST (vmware#1955)

Given healthy replicas execute pruning (block deletes) and block additions in a specified order, e.g.: "block(1, 1), block(2, 1), block(3, 1), prune 1, prune 2, block(4, 3)" where block(X, Y) means block X being added when current genesis is Y and prune N means prune block N, we want that a fallen-behind or crashed replica executes them in the same order on top of its blockchain. This is required so that all replicas reach to the same hash commitments at the end. Currently, the replica that receives blocks from ST will do the following: 1. Add all blocks to its chain. 2. Prune up to the same point as other replicas. However, this would lead to the following different order: "block(1, 1), block(2, 1), block(3, 1), block(4, 1), prune 1, prune 2" In order to provide the same order, we persist the genesis block ID at the time any subsequent block is added in that block. That allows us to then iterate over blocks received by ST and before adding them to the local blockchain, we first prune up to the genesis block ID in the ST block currently being processed. For example, if block 1 is already existing at the crashed replica, its actions would be: 1. block(2, 1) -> add block 2 2. block(3, 1) -> add block 3 3. block(4, 3): * prune block1 and 2 (up to 3) * add block 4 as block (4, 3) Above solution adds a single 8-byte "concord_internal" category key-value in every block. Remove `last_scheduled_block_for_pruning_` in PruningHandler and just use the genesis block ID for pruning status. Change unit tests so that they accommodate the new key. Introduce a new unit test that tests the new behaviour. Update the `test_pruning_command_with_failures` reconfiguration test so that it always does ST and add an extra step - a subsequent ST after pruning.
wzp2020 · Oct 14, 2021 · 119b374 · 119b374
1 parent 8ece3b1
commit 119b374
Show file tree

Hide file tree

Showing 11 changed files with 450 additions and 120 deletions.
diff --git a/kvbc/include/categorization/kv_blockchain.h b/kvbc/include/categorization/kv_blockchain.h
@@ -125,6 +125,10 @@ class KeyValueBlockchain {
   void linkSTChainFrom(BlockId block_id);
   void writeSTLinkTransaction(const BlockId block_id, RawBlock& block);
 
+  // If a block has the genesis ID key, prune up to it. Rationale is that this will preserve the same order of block
+  // deletes relative to block adds on source and destination replicas.
+  void pruneOnSTLink(const RawBlock& block);
+
   // computes the digest of a raw block which is the parent of block_id i.e. block_id - 1
   std::future<BlockDigest> computeParentBlockDigest(const BlockId block_id, VersionedRawBlock&& cached_raw_block);
 
@@ -199,6 +203,8 @@ class KeyValueBlockchain {
                                         ImmutableInput&& updates,
                                         concord::storage::rocksdb::NativeWriteBatch& write_batch);
 
+  void addGenesisBlockKey(Updates& updates) const;
+
   /////////////////////// Members ///////////////////////
 
   std::shared_ptr<concord::storage::rocksdb::NativeClient> native_client_;

diff --git a/kvbc/include/categorization/updates.h b/kvbc/include/categorization/updates.h
@@ -241,6 +241,11 @@ struct Updates {
   template <typename Update>
   bool appendKeyValue(const std::string& category_id, std::string&& key, typename Update::ValueType&& value);
 
+  template <typename InputType>
+  void addCategoryIfNotExisting(const std::string& category_id) {
+    category_updates_.kv.try_emplace(category_id, InputType{});
+  }
+
   std::size_t size() const { return block_merkle_size + versioned_kv_size + immutable_size; }
   bool empty() const { return size() == 0; }
   std::size_t block_merkle_size{};

diff --git a/kvbc/include/kvbc_key_types.hpp b/kvbc/include/kvbc_key_types.hpp
@@ -13,6 +13,8 @@
 #pragma once
 
 #include <cstdint>
+#include <string>
+
 namespace concord::kvbc::keyTypes {
 static const char bft_seq_num_key = 0x21;
 static const char reconfiguration_pruning_key = 0x24;
@@ -27,6 +29,7 @@ static const char reconfiguration_tls_exchange_key = 0x2e;
 
 static const char reconfiguration_restart_key = 0x30;
 static const char reconfiguration_ts_key = 0x31;
+static const std::string genesis_block_key(1, 0x32);
 
 enum CLIENT_COMMAND_TYPES : uint8_t {
   start_ = 0x0,

diff --git a/kvbc/include/pruning_handler.hpp b/kvbc/include/pruning_handler.hpp
@@ -178,7 +178,6 @@ class PruningHandler : public concord::reconfiguration::BftReconfigurationHandle
   std::uint64_t replica_id_{0};
   std::uint64_t num_blocks_to_keep_{0};
   bool run_async_{false};
-  mutable std::optional<kvbc::BlockId> last_scheduled_block_for_pruning_;
   mutable std::mutex pruning_status_lock_;
   mutable std::future<void> async_pruning_res_;
 };

diff --git a/kvbc/src/categorization/kv_blockchain.cpp b/kvbc/src/categorization/kv_blockchain.cpp
@@ -17,6 +17,9 @@
 #include "json_output.hpp"
 #include "diagnostics.h"
 #include "performance_handler.h"
+#include "kvbc_key_types.hpp"
+#include "categorization/db_categories.h"
+#include "endianness.hpp"
 
 #include <stdexcept>
 
@@ -146,6 +149,15 @@ void KeyValueBlockchain::loadCategories() {
   }
 }
 
+void KeyValueBlockchain::addGenesisBlockKey(Updates& updates) const {
+  const auto stale_on_update = true;
+  updates.addCategoryIfNotExisting<VersionedInput>(kConcordInternalCategoryId);
+  updates.appendKeyValue<VersionedUpdates>(
+      kConcordInternalCategoryId,
+      std::string{keyTypes::genesis_block_key},
+      VersionedUpdates::ValueType{concordUtils::toBigEndianStringBuffer(getGenesisBlockId()), stale_on_update});
+}
+
 // 1) Defines a new block
 // 2) calls per cateogry with its updates
 // 3) inserts the updates KV to the DB updates set per column family
@@ -154,6 +166,7 @@ BlockId KeyValueBlockchain::addBlock(Updates&& updates) {
   diagnostics::TimeRecorder scoped_timer(*histograms_.addBlock);
   // Use new client batch and column families
   auto write_batch = native_client_->getBatch();
+  addGenesisBlockKey(updates);
   auto block_id = addBlock(std::move(updates.category_updates_), write_batch);
   native_client_->write(std::move(write_batch));
   block_chain_.setAddedBlockId(block_id);
@@ -729,13 +742,32 @@ void KeyValueBlockchain::linkSTChainFrom(BlockId block_id) {
     if (!raw_block) {
       return;
     }
+    // First prune and then link the block to the chain. Rationale is that this will preserve the same order of block
+    // deletes relative to block adds on source and destination replicas.
+    pruneOnSTLink(*raw_block);
     writeSTLinkTransaction(i, *raw_block);
   }
   // Linking has fully completed and we should not have any more ST temporary blocks left. Therefore, make sure we don't
   // have any value for the latest ST temporary block ID cache.
   state_transfer_block_chain_.resetChain();
 }
 
+void KeyValueBlockchain::pruneOnSTLink(const RawBlock& block) {
+  auto cat_it = block.data.updates.kv.find(kConcordInternalCategoryId);
+  if (cat_it == block.data.updates.kv.cend()) {
+    return;
+  }
+  const auto& internal_kvs = std::get<VersionedInput>(cat_it->second).kv;
+  auto key_it = internal_kvs.find(keyTypes::genesis_block_key);
+  if (key_it != internal_kvs.cend()) {
+    const auto block_genesis_id = concordUtils::fromBigEndianBuffer<BlockId>(key_it->second.data.data());
+    while (getGenesisBlockId() >= INITIAL_GENESIS_BLOCK_ID && getGenesisBlockId() < getLastReachableBlockId() &&
+           block_genesis_id > getGenesisBlockId()) {
+      deleteGenesisBlock();
+    }
+  }
+}
+
 // Atomic delete from state transfer and add to blockchain
 void KeyValueBlockchain::writeSTLinkTransaction(const BlockId block_id, RawBlock& block) {
   auto write_batch = native_client_->getBatch();

diff --git a/kvbc/src/pruning_handler.cpp b/kvbc/src/pruning_handler.cpp
@@ -210,10 +210,6 @@ void PruningHandler::pruneThroughBlockId(kvbc::BlockId block_id) const {
   const auto genesis_block_id = ro_storage_.getGenesisBlockId();
   if (block_id >= genesis_block_id) {
     bftEngine::ControlStateManager::instance().setPruningProcess(true);
-    // last_scheduled_block_for_pruning_ is being updated only here, thus, once
-    // we set the control_state_manager, no other write request will be executed
-    // and we can set it without grabing the mutex
-    last_scheduled_block_for_pruning_ = block_id;
     auto prune = [this](kvbc::BlockId until) {
       try {
         blocks_deleter_.deleteBlocksUntil(until);
@@ -248,8 +244,8 @@ bool PruningHandler::handle(const concord::messages::PruneStatusRequest&,
   if (!pruning_enabled_) return true;
   concord::messages::PruneStatus prune_status;
   std::lock_guard lock(pruning_status_lock_);
-  prune_status.last_pruned_block =
-      last_scheduled_block_for_pruning_.has_value() ? last_scheduled_block_for_pruning_.value() : 0;
+  const auto genesis_id = ro_storage_.getGenesisBlockId();
+  prune_status.last_pruned_block = (genesis_id > INITIAL_GENESIS_BLOCK_ID ? genesis_id - 1 : 0);
   prune_status.in_progress = bftEngine::ControlStateManager::instance().getPruningProcessStatus();
   rres.response = prune_status;
   LOG_INFO(logger_, "Pruning status is " << KVLOG(prune_status.in_progress));

diff --git a/kvbc/test/categorization/blocks_test.cpp b/kvbc/test/categorization/blocks_test.cpp
@@ -19,6 +19,7 @@
 #include "categorization/column_families.h"
 #include "categorization/updates.h"
 #include "categorization/kv_blockchain.h"
+#include "categorization/db_categories.h"
 #include <iostream>
 #include <string>
 #include <utility>
@@ -79,7 +80,10 @@ TEST_F(categorized_kvbc, serialization_and_desirialization_of_block) {
 
 TEST_F(categorized_kvbc, reconstruct_merkle_updates) {
   KeyValueBlockchain block_chain{
-      db, true, std::map<std::string, CATEGORY_TYPE>{{"merkle", CATEGORY_TYPE::block_merkle}}};
+      db,
+      true,
+      std::map<std::string, CATEGORY_TYPE>{{"merkle", CATEGORY_TYPE::block_merkle},
+                                           {kConcordInternalCategoryId, CATEGORY_TYPE::versioned_kv}}};
   KeyValueBlockchain::KeyValueBlockchain_tester tester{};
 
   // Add block1
@@ -171,7 +175,9 @@ TEST_F(categorized_kvbc, reconstruct_immutable_updates) {
   KeyValueBlockchain block_chain{
       db,
       true,
-      std::map<std::string, CATEGORY_TYPE>{{"imm", CATEGORY_TYPE::immutable}, {"imm2", CATEGORY_TYPE::immutable}}};
+      std::map<std::string, CATEGORY_TYPE>{{"imm", CATEGORY_TYPE::immutable},
+                                           {"imm2", CATEGORY_TYPE::immutable},
+                                           {kConcordInternalCategoryId, CATEGORY_TYPE::versioned_kv}}};
   KeyValueBlockchain::KeyValueBlockchain_tester tester{};
 
   // Add block1
@@ -244,7 +250,11 @@ TEST_F(categorized_kvbc, reconstruct_immutable_updates) {
 }
 
 TEST_F(categorized_kvbc, fail_reconstruct_immutable_updates) {
-  KeyValueBlockchain block_chain{db, true, std::map<std::string, CATEGORY_TYPE>{{"imm", CATEGORY_TYPE::immutable}}};
+  KeyValueBlockchain block_chain{
+      db,
+      true,
+      std::map<std::string, CATEGORY_TYPE>{{"imm", CATEGORY_TYPE::immutable},
+                                           {kConcordInternalCategoryId, CATEGORY_TYPE::versioned_kv}}};
   KeyValueBlockchain::KeyValueBlockchain_tester tester{};
 
   // Add block1
@@ -274,10 +284,12 @@ TEST_F(categorized_kvbc, fail_reconstruct_immutable_updates) {
 }
 
 TEST_F(categorized_kvbc, reconstruct_versioned_kv_updates) {
-  KeyValueBlockchain block_chain{db,
-                                 true,
-                                 std::map<std::string, CATEGORY_TYPE>{{"ver", CATEGORY_TYPE::versioned_kv},
-                                                                      {"ver2", CATEGORY_TYPE::versioned_kv}}};
+  KeyValueBlockchain block_chain{
+      db,
+      true,
+      std::map<std::string, CATEGORY_TYPE>{{"ver", CATEGORY_TYPE::versioned_kv},
+                                           {"ver2", CATEGORY_TYPE::versioned_kv},
+                                           {kConcordInternalCategoryId, CATEGORY_TYPE::versioned_kv}}};
   KeyValueBlockchain::KeyValueBlockchain_tester tester{};
 
   // Add block1