diff --git a/src/kv/apply_changes.h b/src/kv/apply_changes.h index 52bb01606f3..a041b49d906 100644 --- a/src/kv/apply_changes.h +++ b/src/kv/apply_changes.h @@ -25,10 +25,6 @@ namespace ccf::kv // sets to their underlying Maps. Calls f() at most once, iff the writes are // applied, to retrieve a unique Version for the write set and return the max // version which can have a conflict with the transaction. - // - // The track_read_versions parameter tells the store if it needs to track the - // last read version for every key. This is required for backup execution as - // described at the top of tx.h using VersionLastNewMap = Version; using VersionResolver = std::function( @@ -40,7 +36,6 @@ namespace ccf::kv ccf::kv::ConsensusHookPtrs& hooks, const MapCollection& new_maps, const std::optional& new_maps_conflict_version, - bool track_read_versions, bool track_deletes_on_missing_keys, const std::optional& expected_rollback_count = std::nullopt) { @@ -88,7 +83,7 @@ namespace ccf::kv { for (auto it = views.begin(); it != views.end(); ++it) { - if (!it->second->prepare(track_read_versions)) + if (!it->second->prepare()) { ok = false; break; @@ -132,40 +127,31 @@ namespace ccf::kv std::tie(version, version_last_new_map) = version_resolver_fn(!new_maps.empty()); - if (!track_read_versions) + // Transfer ownership of these new maps to their target stores, iff we + // have writes to them + for (const auto& [map_name, map_ptr] : new_maps) { - // Transfer ownership of these new maps to their target stores, iff we - // have writes to them - for (const auto& [map_name, map_ptr] : new_maps) + const auto it = views.find(map_name); + if (it != views.end() && it->second->has_writes()) { - const auto it = views.find(map_name); - if (it != views.end() && it->second->has_writes()) - { - map_ptr->get_store()->add_dynamic_map(version, map_ptr); - } + map_ptr->get_store()->add_dynamic_map(version, map_ptr); } + } - for (auto it = views.begin(); it != views.end(); ++it) - { - it->second->commit( - version, track_read_versions, track_deletes_on_missing_keys); - } + for (auto it = views.begin(); it != views.end(); ++it) + { + it->second->commit(version, track_deletes_on_missing_keys); + } - // Collect ConsensusHooks - for (auto it = views.begin(); it != views.end(); ++it) + // Collect ConsensusHooks + for (auto it = views.begin(); it != views.end(); ++it) + { + auto hook_ptr = it->second->post_commit(); + if (hook_ptr != nullptr) { - auto hook_ptr = it->second->post_commit(); - if (hook_ptr != nullptr) - { - hooks.push_back(std::move(hook_ptr)); - } + hooks.push_back(std::move(hook_ptr)); } } - else - { - // A linearizability violation was detected - ok = false; - } } for (auto it = changes.begin(); it != changes.end(); ++it) diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index 7f73f590253..413081b5de0 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -130,7 +130,6 @@ namespace ccf::kv */ CommitResult commit( const ccf::ClaimsDigest& claims = ccf::empty_claims(), - bool track_read_versions = false, std::function(bool has_new_map)> version_resolver = nullptr, std::functioncreated_maps, new_maps_conflict_version, - track_read_versions, track_deletes_on_missing_keys); if (maps_created) @@ -422,7 +420,6 @@ namespace ccf::kv throw std::logic_error("Reserved transaction cannot be empty"); std::vector hooks; - bool track_read_versions = false; bool track_deletes_on_missing_keys = false; auto c = apply_changes( all_changes, @@ -430,7 +427,6 @@ namespace ccf::kv hooks, pimpl->created_maps, version, - track_read_versions, track_deletes_on_missing_keys, rollback_count); success = c.has_value(); diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 91eaf8a096a..976fe4609be 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -599,11 +599,8 @@ namespace ccf::kv virtual ~AbstractCommitter() = default; virtual bool has_writes() = 0; - virtual bool prepare(bool track_commits) = 0; - virtual void commit( - Version v, - bool track_read_versions, - bool track_deletes_on_missing_keys) = 0; + virtual bool prepare() = 0; + virtual void commit(Version v, bool track_deletes_on_missing_keys) = 0; virtual ConsensusHookPtr post_commit() = 0; }; diff --git a/src/kv/untyped_map.h b/src/kv/untyped_map.h index 767c5da2f78..0de9c39c88a 100644 --- a/src/kv/untyped_map.h +++ b/src/kv/untyped_map.h @@ -143,7 +143,7 @@ namespace ccf::kv::untyped return committed_writes || change_set.has_writes(); } - bool prepare(bool track_read_versions) override + bool prepare() override { auto& map_roll = map.get_roll(); @@ -185,9 +185,7 @@ namespace ccf::kv::untyped // conflicts then ensure that the read versions also match. if ( !search.has_value() || - std::get<0>(it->second) != search.value().version || - (track_read_versions && - std::get<1>(it->second) != search.value().read_version)) + std::get<0>(it->second) != search.value().version) { LOG_DEBUG_FMT("Read depends on invalid version of entry"); return false; @@ -198,12 +196,9 @@ namespace ccf::kv::untyped return true; } - void commit( - Version v, - bool track_read_versions, - bool track_deletes_on_missing_keys) override + void commit(Version v, bool track_deletes_on_missing_keys) override { - if (change_set.writes.empty() && !track_read_versions) + if (change_set.writes.empty()) { commit_version = change_set.start_version; return; @@ -212,30 +207,6 @@ namespace ccf::kv::untyped auto& map_roll = map.get_roll(); auto state = map_roll.commits->get_tail()->state; - // To track conflicts the read version of all keys that are read or - // written within a transaction must be updated. - if (track_read_versions) - { - for (auto it = change_set.reads.begin(); it != change_set.reads.end(); - ++it) - { - auto search = state.get(it->first); - if (!search.has_value()) - { - continue; - } - state = - state.put(it->first, VersionV{search->version, v, search->value}); - } - if (change_set.writes.empty()) - { - commit_version = change_set.start_version; - map.roll.commits->insert_back(map.roll.create_new_local_commit( - commit_version, std::move(state), change_set.writes)); - return; - } - } - // Record our commit time. commit_version = v; committed_writes = true; @@ -439,14 +410,16 @@ namespace ccf::kv::untyped return true; } - bool prepare(bool) override + bool prepare() override { // Snapshots never conflict return true; } - void commit(Version, bool, bool) override + void commit(Version v, bool track_deletes_on_missing_keys) override { + (void)v; + (void)track_deletes_on_missing_keys; // Version argument is ignored. The version of the roll after the // snapshot is applied depends on the version of the map at which the // snapshot was taken. diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 67064d98073..0f8052ac04a 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -777,7 +777,7 @@ namespace ccf // else args owns a valid Tx relating to a non-pending response, which // should be applied ccf::kv::CommittableTx& tx = *args.owned_tx; - ccf::kv::CommitResult result = tx.commit(ctx->claims, false); + ccf::kv::CommitResult result = tx.commit(ctx->claims); switch (result) { diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index f9a4e32ac94..64b51dc8c77 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -155,8 +155,7 @@ namespace ccf pending_snapshots[generation_count] = {}; pending_snapshots[generation_count].version = snapshot_version; - auto rc = - tx.commit(cd, false, nullptr, capture_ws_digest_and_commit_evidence); + auto rc = tx.commit(cd, nullptr, capture_ws_digest_and_commit_evidence); if (rc != ccf::kv::CommitResult::SUCCESS) { LOG_FAIL_FMT(