Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take lock over Snapshotter state during callback #6882

Merged
merged 5 commits into from
Mar 5, 2025
Merged

Take lock over Snapshotter state during callback #6882

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
690e0e0
Take lock over Snapshotter state during callback
eddyashton Mar 4, 2025
4f11df8
Merge branch 'main' into snapshotter_lock_in_callback
maxtropets Mar 4, 2025
7768293
Scoped locks
eddyashton Mar 5, 2025
9af05e2
Merge branch 'snapshotter_lock_in_callback' of github.com:eddyashton/…
eddyashton Mar 5, 2025
f87da83
Merge branch 'main' of github.com:microsoft/CCF into snapshotter_lock…
eddyashton Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 27 additions & 21 deletions src/node/snapshotter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,27 @@ namespace ccf
std::unique_ptr<ccf::kv::AbstractStore::AbstractSnapshot> snapshot,
uint32_t generation_count)
{
if (pending_snapshots.size() >= max_pending_snapshots_count)
auto snapshot_version = snapshot->get_version();

{
LOG_FAIL_FMT(
"Skipping new snapshot generation as {} snapshots are already "
"pending",
pending_snapshots.size());
return;
}
std::unique_lock<ccf::pal::Mutex> guard(lock);
if (pending_snapshots.size() >= max_pending_snapshots_count)
{
LOG_FAIL_FMT(
"Skipping new snapshot generation as {} snapshots are already "
"pending",
pending_snapshots.size());
return;
}

auto snapshot_version = snapshot->get_version();
// It is possible that the signature following the snapshot evidence is
// scheduled by another thread while the below snapshot evidence
// transaction is committed. To allow for such scenario, the evidence
// seqno is recorded via `record_snapshot_evidence_idx()` on a hook
// rather than here.
pending_snapshots[generation_count] = {};
pending_snapshots[generation_count].version = snapshot_version;
}

auto serialised_snapshot = store->serialise_snapshot(std::move(snapshot));
auto serialised_snapshot_size = serialised_snapshot.size();
Expand All @@ -147,14 +158,6 @@ namespace ccf
commit_evidence = commit_evidence_;
};

// It is possible that the signature following the snapshot evidence is
// scheduled by another thread while the below snapshot evidence
// transaction is committed. To allow for such scenario, the evidence
// seqno is recorded via `record_snapshot_evidence_idx()` on a hook rather
// than here.
pending_snapshots[generation_count] = {};
pending_snapshots[generation_count].version = snapshot_version;

auto rc =
tx.commit(cd, false, nullptr, capture_ws_digest_and_commit_evidence);
if (rc != ccf::kv::CommitResult::SUCCESS)
Expand All @@ -168,11 +171,14 @@ namespace ccf

auto evidence_version = tx.commit_version();

pending_snapshots[generation_count].commit_evidence = commit_evidence;
pending_snapshots[generation_count].write_set_digest = ws_digest;
pending_snapshots[generation_count].snapshot_digest = cd.value();
pending_snapshots[generation_count].serialised_snapshot =
std::move(serialised_snapshot);
{
std::unique_lock<ccf::pal::Mutex> guard(lock);
pending_snapshots[generation_count].commit_evidence = commit_evidence;
pending_snapshots[generation_count].write_set_digest = ws_digest;
pending_snapshots[generation_count].snapshot_digest = cd.value();
pending_snapshots[generation_count].serialised_snapshot =
std::move(serialised_snapshot);
}

auto to_host = writer_factory.create_writer_to_outside();
RINGBUFFER_WRITE_MESSAGE(
Expand Down