craft: S5 infrastructure — RAFT entry types, CraftPeerFetcher, write_counter - #172
Open
sbinmalek wants to merge 4 commits into
Open
craft: S5 infrastructure — RAFT entry types, CraftPeerFetcher, write_counter#172sbinmalek wants to merge 4 commits into
sbinmalek wants to merge 4 commits into
Conversation
…counter
Add the wire format definitions and scaffolding needed by S5
(SyncRSCommitLSN + InternalLogin RAFT state machine entries).
Implementations land in follow-up PRs; this PR makes the base branch
compile cleanly.
- craft_raft_entries.hpp: CraftEntryType enum, CraftEntryHeader,
SyncRSCommitLSNPayload (variable-length with trailing int64_t array),
InternalLoginPayload, and helpers (sync_rs_commit_lsn_key_size,
serialize_sync_rs_commit_lsn, parse_empty_slots)
- craft_repl_dev.hpp: add CraftPeerFetcher interface (server-to-server
fetch abstraction; production wired in S9), set_peer_fetcher() setter,
write_counter_ atomic (periodic SyncRSCommitLSN auto-fire trigger),
update apply_sync_rs_commit_lsn signature to carry empty_slots
- craft_repl_dev.cpp: match stub signature to updated header
- home_blks_config.fbs: add sync_rs_commit_lsn_interval setting (128)
There was a problem hiding this comment.
Pull request overview
Scaffolds CRAFT S5 RAFT metadata and peer-fetch infrastructure for later implementation.
Changes:
- Defines RAFT entry wire formats and serialization helpers.
- Adds peer-fetch and write-trigger state to
CraftReplDev. - Adds configurable SyncRSCommitLSN interval.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/lib/craft/craft_raft_entries.hpp |
Adds RAFT entry types, payloads, and helpers. |
src/lib/craft/craft_repl_dev.hpp |
Adds peer fetcher and S5 state scaffolding. |
src/lib/craft/craft_repl_dev.cpp |
Updates the apply-helper stub signature. |
src/lib/home_blks_config.fbs |
Adds the synchronization interval setting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+95
to
+97
| inline std::vector< int64_t > parse_empty_slots(const SyncRSCommitLSNPayload* p) { | ||
| const auto* src = reinterpret_cast< const int64_t* >(p + 1); | ||
| return std::vector< int64_t >(src, src + p->num_empty_slots); |
Contributor
Author
There was a problem hiding this comment.
Added the check for corrupt entry
Comment on lines
+87
to
+91
| auto* p = reinterpret_cast< SyncRSCommitLSNPayload* >(buf); | ||
| p->rs_commit_lsn = rs_commit_lsn; | ||
| p->client_token = client_token; | ||
| p->num_empty_slots = static_cast< uint32_t >(empty_slots.size()); | ||
| std::memcpy(p + 1, empty_slots.data(), empty_slots.size() * sizeof(int64_t)); |
Contributor
Author
There was a problem hiding this comment.
Added the check for empty empty_slots
Comment on lines
+85
to
+86
| inline void serialize_sync_rs_commit_lsn(uint8_t* buf, int64_t rs_commit_lsn, uint64_t client_token, | ||
| const std::vector< int64_t >& empty_slots) { |
Contributor
Author
There was a problem hiding this comment.
Will be covered in later PRs
- parse_empty_slots now takes the raw key blob instead of a trusted
struct pointer, and returns std::optional instead of an unconditional
vector. Validates the fixed prefix is present and that the persisted
num_empty_slots exactly accounts for the blob's actual size before
ever constructing the vector -- a corrupt/truncated RAFT entry no
longer risks an out-of-bounds read or a bogus multi-GB allocation
during replay.
- serialize_sync_rs_commit_lsn now zeroes the fixed prefix before
writing fields, so SyncRSCommitLSNPayload's compiler-inserted trailing
padding (20 real bytes, sizeof rounds to 24) can't leak prior buffer
contents into a persisted/replicated entry or make equivalent entries
byte-different.
- Guard the empty_slots memcpy: vector::data() may return null when
empty, and memcpy(dest, nullptr, 0) is UB even at zero length.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/v6.x #172 +/- ##
===========================================
Coverage ? 45.00%
===========================================
Files ? 18
Lines ? 1020
Branches ? 442
===========================================
Hits ? 459
Misses ? 273
Partials ? 288 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the wire format definitions and scaffolding needed by S5 (SyncRSCommitLSN + InternalLogin RAFT state machine entries). Implementations land in follow-up PRs; this PR makes the base branch compile cleanly.