Skip to content

Commit a828b48

Browse files
authored
Remove unused track_read_versions (#6871)
1 parent d48f074 commit a828b48

7 files changed

+30
-80
lines changed

src/kv/apply_changes.h

+18-32
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ namespace ccf::kv
2525
// sets to their underlying Maps. Calls f() at most once, iff the writes are
2626
// applied, to retrieve a unique Version for the write set and return the max
2727
// version which can have a conflict with the transaction.
28-
//
29-
// The track_read_versions parameter tells the store if it needs to track the
30-
// last read version for every key. This is required for backup execution as
31-
// described at the top of tx.h
3228

3329
using VersionLastNewMap = Version;
3430
using VersionResolver = std::function<std::tuple<Version, VersionLastNewMap>(
@@ -40,7 +36,6 @@ namespace ccf::kv
4036
ccf::kv::ConsensusHookPtrs& hooks,
4137
const MapCollection& new_maps,
4238
const std::optional<Version>& new_maps_conflict_version,
43-
bool track_read_versions,
4439
bool track_deletes_on_missing_keys,
4540
const std::optional<Version>& expected_rollback_count = std::nullopt)
4641
{
@@ -88,7 +83,7 @@ namespace ccf::kv
8883
{
8984
for (auto it = views.begin(); it != views.end(); ++it)
9085
{
91-
if (!it->second->prepare(track_read_versions))
86+
if (!it->second->prepare())
9287
{
9388
ok = false;
9489
break;
@@ -132,40 +127,31 @@ namespace ccf::kv
132127
std::tie(version, version_last_new_map) =
133128
version_resolver_fn(!new_maps.empty());
134129

135-
if (!track_read_versions)
130+
// Transfer ownership of these new maps to their target stores, iff we
131+
// have writes to them
132+
for (const auto& [map_name, map_ptr] : new_maps)
136133
{
137-
// Transfer ownership of these new maps to their target stores, iff we
138-
// have writes to them
139-
for (const auto& [map_name, map_ptr] : new_maps)
134+
const auto it = views.find(map_name);
135+
if (it != views.end() && it->second->has_writes())
140136
{
141-
const auto it = views.find(map_name);
142-
if (it != views.end() && it->second->has_writes())
143-
{
144-
map_ptr->get_store()->add_dynamic_map(version, map_ptr);
145-
}
137+
map_ptr->get_store()->add_dynamic_map(version, map_ptr);
146138
}
139+
}
147140

148-
for (auto it = views.begin(); it != views.end(); ++it)
149-
{
150-
it->second->commit(
151-
version, track_read_versions, track_deletes_on_missing_keys);
152-
}
141+
for (auto it = views.begin(); it != views.end(); ++it)
142+
{
143+
it->second->commit(version, track_deletes_on_missing_keys);
144+
}
153145

154-
// Collect ConsensusHooks
155-
for (auto it = views.begin(); it != views.end(); ++it)
146+
// Collect ConsensusHooks
147+
for (auto it = views.begin(); it != views.end(); ++it)
148+
{
149+
auto hook_ptr = it->second->post_commit();
150+
if (hook_ptr != nullptr)
156151
{
157-
auto hook_ptr = it->second->post_commit();
158-
if (hook_ptr != nullptr)
159-
{
160-
hooks.push_back(std::move(hook_ptr));
161-
}
152+
hooks.push_back(std::move(hook_ptr));
162153
}
163154
}
164-
else
165-
{
166-
// A linearizability violation was detected
167-
ok = false;
168-
}
169155
}
170156

171157
for (auto it = changes.begin(); it != changes.end(); ++it)

src/kv/committable_tx.h

-4
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ namespace ccf::kv
130130
*/
131131
CommitResult commit(
132132
const ccf::ClaimsDigest& claims = ccf::empty_claims(),
133-
bool track_read_versions = false,
134133
std::function<std::tuple<Version, Version>(bool has_new_map)>
135134
version_resolver = nullptr,
136135
std::function<void(
@@ -170,7 +169,6 @@ namespace ccf::kv
170169
hooks,
171170
pimpl->created_maps,
172171
new_maps_conflict_version,
173-
track_read_versions,
174172
track_deletes_on_missing_keys);
175173

176174
if (maps_created)
@@ -422,15 +420,13 @@ namespace ccf::kv
422420
throw std::logic_error("Reserved transaction cannot be empty");
423421

424422
std::vector<ConsensusHookPtr> hooks;
425-
bool track_read_versions = false;
426423
bool track_deletes_on_missing_keys = false;
427424
auto c = apply_changes(
428425
all_changes,
429426
[this](bool) { return std::make_tuple(version, version - 1); },
430427
hooks,
431428
pimpl->created_maps,
432429
version,
433-
track_read_versions,
434430
track_deletes_on_missing_keys,
435431
rollback_count);
436432
success = c.has_value();

src/kv/kv_types.h

+2-5
Original file line numberDiff line numberDiff line change
@@ -599,11 +599,8 @@ namespace ccf::kv
599599
virtual ~AbstractCommitter() = default;
600600

601601
virtual bool has_writes() = 0;
602-
virtual bool prepare(bool track_commits) = 0;
603-
virtual void commit(
604-
Version v,
605-
bool track_read_versions,
606-
bool track_deletes_on_missing_keys) = 0;
602+
virtual bool prepare() = 0;
603+
virtual void commit(Version v, bool track_deletes_on_missing_keys) = 0;
607604
virtual ConsensusHookPtr post_commit() = 0;
608605
};
609606

src/kv/store.h

-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ namespace ccf::kv
123123
hooks,
124124
new_maps,
125125
std::nullopt,
126-
false,
127126
track_deletes_on_missing_keys);
128127
if (!c.has_value())
129128
{

src/kv/untyped_map.h

+8-35
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ namespace ccf::kv::untyped
143143
return committed_writes || change_set.has_writes();
144144
}
145145

146-
bool prepare(bool track_read_versions) override
146+
bool prepare() override
147147
{
148148
auto& map_roll = map.get_roll();
149149

@@ -185,9 +185,7 @@ namespace ccf::kv::untyped
185185
// conflicts then ensure that the read versions also match.
186186
if (
187187
!search.has_value() ||
188-
std::get<0>(it->second) != search.value().version ||
189-
(track_read_versions &&
190-
std::get<1>(it->second) != search.value().read_version))
188+
std::get<0>(it->second) != search.value().version)
191189
{
192190
LOG_DEBUG_FMT("Read depends on invalid version of entry");
193191
return false;
@@ -198,12 +196,9 @@ namespace ccf::kv::untyped
198196
return true;
199197
}
200198

201-
void commit(
202-
Version v,
203-
bool track_read_versions,
204-
bool track_deletes_on_missing_keys) override
199+
void commit(Version v, bool track_deletes_on_missing_keys) override
205200
{
206-
if (change_set.writes.empty() && !track_read_versions)
201+
if (change_set.writes.empty())
207202
{
208203
commit_version = change_set.start_version;
209204
return;
@@ -212,30 +207,6 @@ namespace ccf::kv::untyped
212207
auto& map_roll = map.get_roll();
213208
auto state = map_roll.commits->get_tail()->state;
214209

215-
// To track conflicts the read version of all keys that are read or
216-
// written within a transaction must be updated.
217-
if (track_read_versions)
218-
{
219-
for (auto it = change_set.reads.begin(); it != change_set.reads.end();
220-
++it)
221-
{
222-
auto search = state.get(it->first);
223-
if (!search.has_value())
224-
{
225-
continue;
226-
}
227-
state =
228-
state.put(it->first, VersionV{search->version, v, search->value});
229-
}
230-
if (change_set.writes.empty())
231-
{
232-
commit_version = change_set.start_version;
233-
map.roll.commits->insert_back(map.roll.create_new_local_commit(
234-
commit_version, std::move(state), change_set.writes));
235-
return;
236-
}
237-
}
238-
239210
// Record our commit time.
240211
commit_version = v;
241212
committed_writes = true;
@@ -439,14 +410,16 @@ namespace ccf::kv::untyped
439410
return true;
440411
}
441412

442-
bool prepare(bool) override
413+
bool prepare() override
443414
{
444415
// Snapshots never conflict
445416
return true;
446417
}
447418

448-
void commit(Version, bool, bool) override
419+
void commit(Version v, bool track_deletes_on_missing_keys) override
449420
{
421+
(void)v;
422+
(void)track_deletes_on_missing_keys;
450423
// Version argument is ignored. The version of the roll after the
451424
// snapshot is applied depends on the version of the map at which the
452425
// snapshot was taken.

src/node/rpc/frontend.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ namespace ccf
777777
// else args owns a valid Tx relating to a non-pending response, which
778778
// should be applied
779779
ccf::kv::CommittableTx& tx = *args.owned_tx;
780-
ccf::kv::CommitResult result = tx.commit(ctx->claims, false);
780+
ccf::kv::CommitResult result = tx.commit(ctx->claims);
781781

782782
switch (result)
783783
{

src/node/snapshotter.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ namespace ccf
157157
commit_evidence = commit_evidence_;
158158
};
159159

160-
auto rc =
161-
tx.commit(cd, false, nullptr, capture_ws_digest_and_commit_evidence);
160+
auto rc = tx.commit(cd, nullptr, capture_ws_digest_and_commit_evidence);
162161
if (rc != ccf::kv::CommitResult::SUCCESS)
163162
{
164163
LOG_FAIL_FMT(

0 commit comments

Comments
 (0)