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

conanfile.py

@@ -10,7 +10,7 @@
 
 class HomeBlocksConan(ConanFile):
     name = "homeblocks"
-    version = "6.0.6"
+    version = "6.0.7"
 
     homepage = "https://github.com/eBay/HomeBlocks"
     description = "Block Store built on HomeStore"

src/include/homeblks/home_blocks.hpp

@@ -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,17 +174,15 @@ 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

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,

src/lib/craft/tests/CMakeLists.txt

@@ -1,5 +1,13 @@
 cmake_minimum_required(VERSION 3.11)
 
+# Light CRAFT tests include home_blks_config.hpp, which instantiates home_blks_config_factory
+# whose constructor references home_blks_config_fbs[] -- the schema binary generated by
+# settings_gen_cpp(... homeblocks_core ...) in src/lib/CMakeLists.txt.  That generated object is
+# only compiled into homeblocks_core / homeblocks; light tests don't link either.  Providing the
+# generated source directly to each light test binary is the minimal fix.
+set(HB_CONFIG_BINDUMP ${CMAKE_BINARY_DIR}/src/lib/generated/home_blks_config_bindump.cpp)
+set_source_files_properties(${HB_CONFIG_BINDUMP} PROPERTIES GENERATED TRUE)
+
 # Unit test for CraftReplDev::truncate (S4). Compiles craft_repl_dev.cpp directly to avoid
 # dragging in craft_api.cpp → volume.hpp (heavy HomeStore volume plumbing not needed here).
 add_executable(test_craft_truncate)
@@ -8,13 +16,35 @@ target_sources(test_craft_truncate PRIVATE
     ../craft_repl_dev.cpp
 )
 target_compile_definitions(test_craft_truncate PRIVATE _PRERELEASE)
+target_sources(test_craft_truncate PRIVATE ${HB_CONFIG_BINDUMP})
 target_link_libraries(test_craft_truncate
     ${COMMON_TEST_DEPS}
     -rdynamic
 )
+add_dependencies(test_craft_truncate ${PROJECT_NAME}_core)
 
 add_test(NAME CraftTruncateTest COMMAND test_craft_truncate)
 
+# Real multi-threaded tests for CraftReplDev's own internal locking (missing_mu_, overlay_mu_,
+# commit_running_). Same pattern as test_craft_truncate (compile craft_repl_dev.cpp directly, no
+# HomeStore/iomgr) -- MockCraftJournalBackend's coroutine bodies never suspend across a real async
+# boundary, so real std::thread callers drive genuinely concurrent CraftReplDev execution with no
+# reactor needed.
+add_executable(test_craft_concurrency)
+target_sources(test_craft_concurrency PRIVATE
+    test_craft_concurrency.cpp
+    ../craft_repl_dev.cpp
+)
+target_compile_definitions(test_craft_concurrency PRIVATE _PRERELEASE)
+target_sources(test_craft_concurrency PRIVATE ${HB_CONFIG_BINDUMP})
+target_link_libraries(test_craft_concurrency
+    ${COMMON_TEST_DEPS}
+    -rdynamic
+)
+add_dependencies(test_craft_concurrency ${PROJECT_NAME}_core)
+
+add_test(NAME CraftConcurrencyTest COMMAND test_craft_concurrency)
+
 # Unit tests for CraftReplDev::get_lsns(), get_rs_commit_lsn(), and fetch_data() (S6).
 # Same pattern as test_craft_truncate: compile craft_repl_dev.cpp directly.
 add_executable(test_craft_peer_exchange)
@@ -23,10 +53,12 @@ target_sources(test_craft_peer_exchange PRIVATE
     ../craft_repl_dev.cpp
 )
 target_compile_definitions(test_craft_peer_exchange PRIVATE _PRERELEASE)
+target_sources(test_craft_peer_exchange PRIVATE ${HB_CONFIG_BINDUMP})
 target_link_libraries(test_craft_peer_exchange
     ${COMMON_TEST_DEPS}
     -rdynamic
 )
+add_dependencies(test_craft_peer_exchange ${PROJECT_NAME}_core)
 
 add_test(NAME CraftPeerExchangeTest COMMAND test_craft_peer_exchange)
 
@@ -38,13 +70,51 @@ target_sources(test_craft_write PRIVATE
     ../craft_repl_dev.cpp
 )
 target_compile_definitions(test_craft_write PRIVATE _PRERELEASE)
+target_sources(test_craft_write PRIVATE ${HB_CONFIG_BINDUMP})
 target_link_libraries(test_craft_write
     ${COMMON_TEST_DEPS}
     -rdynamic
 )
+add_dependencies(test_craft_write ${PROJECT_NAME}_core)
 
 add_test(NAME CraftWriteTest COMMAND test_craft_write)
 
+# Unit tests for CraftReplDev::commit() (S3: Commit Path), against a fake std::map-backed index.
+# Same pattern as test_craft_truncate: compile craft_repl_dev.cpp directly, no HomeStore bring-up.
+add_executable(test_craft_commit)
+target_sources(test_craft_commit PRIVATE
+    test_craft_commit.cpp
+    ../craft_repl_dev.cpp
+)
+target_compile_definitions(test_craft_commit PRIVATE _PRERELEASE)
+target_sources(test_craft_commit PRIVATE ${HB_CONFIG_BINDUMP})
+target_link_libraries(test_craft_commit
+    ${COMMON_TEST_DEPS}
+    -rdynamic
+)
+add_dependencies(test_craft_commit ${PROJECT_NAME}_core)
+
+add_test(NAME CraftCommitTest COMMAND test_craft_commit)
+
+# Unit tests for CraftReplDev's client-liveness watchdog (S7). Same pattern as test_craft_truncate
+# (compile craft_repl_dev.cpp directly, no HomeStore bring-up), but this binary's main() starts a
+# minimal, HomeStore-free iomgr instance -- needed because scheduling a real timer requires a
+# running reactor pool, unlike every other test in this light suite.
+add_executable(test_craft_watchdog)
+target_sources(test_craft_watchdog PRIVATE
+    test_craft_watchdog.cpp
+    ../craft_repl_dev.cpp
+)
+target_compile_definitions(test_craft_watchdog PRIVATE _PRERELEASE)
+target_sources(test_craft_watchdog PRIVATE ${HB_CONFIG_BINDUMP})
+target_link_libraries(test_craft_watchdog
+    ${COMMON_TEST_DEPS}
+    -rdynamic
+)
+add_dependencies(test_craft_watchdog ${PROJECT_NAME}_core)
+
+add_test(NAME CraftWatchdogTest COMMAND test_craft_watchdog)
+
 # Exercises HomeStoreCraftJournalBackend against a REAL HomeStore home_log_store -- the
 # production backend was previously never executed by any test. Links the full homeblocks
 # library, unlike the tests above, because a real home_log_store requires a running HomeStore
@@ -80,3 +150,21 @@ target_link_libraries(test_craft_journal_slot_wire
 )
 
 add_test(NAME CraftJournalSlotWireTest COMMAND test_craft_journal_slot_wire)
+
+# Heavy integration test for CraftReplDev::commit()/read() against a REAL VolumeIndexTable and real
+# data blocks (via a real, ordinarily-created volume) -- same pattern as
+# test_craft_homestore_backend.cpp (links the full library, real HomeStore bring-up).
+add_executable(test_craft_commit_hs)
+target_sources(test_craft_commit_hs PRIVATE
+    test_craft_commit_hs.cpp
+)
+target_include_directories(test_craft_commit_hs PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}/../../volume/tests
+)
+target_link_libraries(test_craft_commit_hs
+    ${PROJECT_NAME}
+    ${COMMON_TEST_DEPS}
+    -rdynamic
+)
+
+add_test(NAME CraftCommitHsTest COMMAND test_craft_commit_hs --index_chunk_size_mb=128 --data_chunk_size_mb=128)