S3: Commit + Read path: advance commit_lsn, serve reads, watchdog

conanfile.py

@@ -10,7 +10,7 @@
 
 class HomeBlocksConan(ConanFile):
     name = "homeblocks"
-    version = "6.0.7"
+    version = "6.0.8"
 
     homepage = "https://github.com/eBay/HomeBlocks"
     description = "Block Store built on HomeStore"
@@ -54,7 +54,7 @@ def requirements(self):
         self.requires("homestore/[^8.0]@oss/dev", transitive_headers=True)
         self.requires("iomgr/[^13.0]@oss/dev", transitive_headers=True)
         self.requires("sisl/[^14.8]@oss/dev", transitive_headers=True)
-        self.requires("craft_client/0.4.0@oss/dev", transitive_headers=True) # the extracted CRAFT wire + client + reference
+        self.requires("craft_client/0.4.1@oss/dev", transitive_headers=True) # the extracted CRAFT wire + client + reference
 
     def validate(self):
         if self.info.settings.compiler.cppstd:

src/include/homeblks/home_blocks.hpp

@@ -69,7 +69,7 @@ using volume_handle = std::shared_ptr< volume >;
 // standard equivalent (invalid arg, no space, io error, unsupported op, ...) is returned as
 // std::make_error_condition(std::errc::*) directly rather than duplicated here.
 ENUM(volume_error, uint16_t, UNKNOWN_VOLUME = 1, CRC_MISMATCH, INDEX_ERROR, INTERNAL_ERROR, OFFLINE, STALE_TERM,
-     EMPTY_SLOT, WRONG_TOKEN, INVALID_ENTRY);
+     EMPTY_SLOT, WRONG_TOKEN, INVALID_ENTRY, HORIZON_STALE);
 
 ENUM(volume_state, uint32_t,
      INIT,       // created, not yet online
@@ -149,7 +149,7 @@ inline std::error_condition make_error_condition(volume_error e) noexcept {
 async_result< size_t > async_read(volume_handle const& vol, uint64_t addr, sisl::sg_list sgs);
 [[nodiscard]] [[deprecated("legacy block op; use the CRAFT async_read/async_write overloads below (see docs/craft)")]]
 async_result< size_t > async_write(volume_handle const& vol, uint64_t addr, sisl::sg_list sgs);
-[[nodiscard]] [[deprecated("legacy block op; use CRAFT async_write(..., all_zeros=true) (see docs/craft)")]]
+[[nodiscard]] [[deprecated("legacy block op; use CRAFT async_write with empty data (see docs/craft)")]]
 async_status async_unmap(volume_handle const& vol, uint64_t addr, uint64_t len);
 
 // ---- CRAFT data plane: free functions over a volume_handle (one handle == one replica device) ----
@@ -174,24 +174,27 @@ async_status async_unmap(volume_handle const& vol, uint64_t addr, uint64_t len);
 
 // Append one client-assigned write at slot `dlsn`. `addr`/`len` are BYTE offset/length and must be
 // aligned to the volume's lba_size (from craft::LoginResult), else std::errc::invalid_argument. `data` is a
-// caller-owned (iomgr) buffer: set `all_zeros=true` for a WRITE_ZEROES/unmap over [addr, addr+len) --
-// `data` must be empty in that case; otherwise this is a data write of exactly `len` bytes and `data`
-// must be non-empty. The flag, not data emptiness, is what selects the write kind -- an empty buffer
-// with all_zeros=false (or vice versa) is rejected as std::errc::invalid_argument, not silently
-// reinterpreted. Not applied to the index directly; `hdr.commit_lsn` rides along and advances the
-// frontier best-effort in dLSN order (CRAFT's piggybacked commit). STALE_TERM if hdr.term != session term.
+// caller-owned (iomgr) buffer: pass empty `data` (size==0) for a WRITE_ZEROES/unmap over [addr, addr+len)
+// (metadata-only; no block allocation); pass non-empty `data` of exactly `len` bytes for a data write.
+// The write kind is determined by data.empty() -- no separate flag. Not applied to the index directly;
+// `hdr.commit_lsn` rides along and advances the frontier best-effort in dLSN order (CRAFT's piggybacked
+// commit). STALE_TERM if hdr.term != session term.
 // The ack returns the replica's achieved {commit_lsn, last_append_lsn}: every CRAFT IO response piggybacks
 // the watermarks, so any round-trip refreshes the client's per-member model without a keep_alive.
 [[nodiscard]] async_result< craft::lsn_pair > async_write(volume_handle const& vol, craft::client_hdr hdr, int64_t dlsn,
-                                                          uint64_t addr, uint64_t len, sisl::sg_list data,
-                                                          bool all_zeros = false);
+                                                          uint64_t addr, uint64_t len, sisl::sg_list data);
 
 // Read the latest version <= `read_lsn` (horizon H) for [addr, addr+len) (BYTE offset/length, aligned to
 // lba_size). Fills the caller-owned `dest` buffer in place -- data sub-ranges get their bytes, holes get
 // zeros -- and returns craft::read_result: the sparse layout (which byte sub-ranges were data vs holes; the
 // thin/hole info) PLUS the replica's piggybacked {commit_lsn, last_append_lsn}, snapshotted atomically with
 // the read. Served from the index or the journal-tail overlay; never fetches from a peer. Advances the
-// frontier to hdr.commit_lsn (piggybacked commit). std::errc::invalid_argument if addr/len are misaligned.
+// frontier to hdr.commit_lsn (piggybacked commit) BEFORE resolving read_lsn, so a request whose own
+// piggyback (or a concurrent write/keep_alive) pushes commit_lsn past read_lsn is unanswerable -- the
+// pre-read_lsn version is gone from the index by construction (apply is a blind overwrite past
+// commit_lsn) and is not reconstructible from anything this replica still holds. Returns
+// volume_error::HORIZON_STALE in that case rather than silently serving a too-new version.
+// std::errc::invalid_argument if addr/len are misaligned.
 [[nodiscard]] async_result< craft::read_result > async_read(volume_handle const& vol, craft::client_hdr hdr,
                                                             int64_t read_lsn, uint64_t addr, uint64_t len,
                                                             sisl::sg_list dest);

src/lib/craft/craft_api.cpp

@@ -51,10 +51,10 @@ async_status logout(volume_handle const& vol, craft::client_hdr hdr) {
 }
 
 async_result< craft::lsn_pair > async_write(volume_handle const& vol, craft::client_hdr hdr, int64_t dlsn,
-                                            uint64_t addr, uint64_t len, sisl::sg_list data, bool all_zeros) {
+                                            uint64_t addr, uint64_t len, sisl::sg_list data) {
     auto* d = craft_dev_of(vol);
     if (!d) co_return no_craft_backend();
-    co_return co_await d->write(hdr, dlsn, addr, len, std::move(data), all_zeros);
+    co_return co_await d->write(hdr, dlsn, addr, len, std::move(data));
 }
 
 async_result< craft::read_result > async_read(volume_handle const& vol, craft::client_hdr hdr, int64_t read_lsn,