Add file-based persistent log store and state manager#635
Add file-based persistent log store and state manager#635bihari123 wants to merge 1 commit intoeBay:masterfrom
Conversation
- Add file_based_log_store: persists Raft log entries to disk - Add file_based_state_mgr: persists cluster config and server state - Add --persistent flag to calculator example - Add --persistent-dir=<dir> option for custom storage location - Update example_common.hxx to support persistent storage This provides a minimal example of durable storage for NuRaft, addressing the gap between in-memory examples and production use.
greensky00
left a comment
greensky00
left a comment
There was a problem hiding this comment.
Thank you for submitting the PR. I've left a few comments as a first round review. Please take a look.
| // Global flag to enable persistent storage. | ||
| // If true, file-based storage will be used instead of in-memory. | ||
| static bool USE_PERSISTENT_STORAGE = false; | ||
|
|
||
| // Directory for persistent storage. | ||
| static std::string PERSISTENT_STORAGE_DIR = "./nuraft_data"; | ||
|
|
||
| // Helper to set persistent storage options. | ||
| inline void set_persistent_storage(bool enable, const std::string& dir = "./nuraft_data") { | ||
| USE_PERSISTENT_STORAGE = enable; | ||
| if (enable) { | ||
| PERSISTENT_STORAGE_DIR = dir; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
These are used by calc_server only, not common features. Need to move it to calc_server.cxx.
There was a problem hiding this comment.
+)
After moving them to calc_server, set default dir path to ./calc_server_data_<server_id>.
| // State manager: use persistent or in-memory based on flag. | ||
| if (USE_PERSISTENT_STORAGE) { | ||
| std::cout << "using persistent storage: " << PERSISTENT_STORAGE_DIR << std::endl; | ||
| stuff.smgr_ = cs_new<file_based_state_mgr>( stuff.server_id_, | ||
| stuff.endpoint_, | ||
| PERSISTENT_STORAGE_DIR ); | ||
| } else { | ||
| std::cout << "using in-memory storage" << std::endl; | ||
| stuff.smgr_ = cs_new<inmem_state_mgr>( stuff.server_id_, | ||
| stuff.endpoint_ ); | ||
| } |
There was a problem hiding this comment.
Same as above, I suggest modifying init_raft function to take an additional parameter ptr<state_mgr>, which is inmem_state_mgr by default. Only calc_server would pass file-based state mgr.
| logger.cc | ||
| in_memory_log_store.cxx) | ||
| in_memory_log_store.cxx | ||
| file_based_log_store.cxx) |
There was a problem hiding this comment.
echo_server will not use it, after it is removed from example_common.hxx.
| logger.cc | ||
| in_memory_log_store.cxx) | ||
| in_memory_log_store.cxx | ||
| file_based_log_store.cxx) |
| /** | ||
| * In-memory cache of log entries for faster access. | ||
| * Map of <log index, log data>. | ||
| */ | ||
| std::map<ulong, ptr<log_entry>> logs_; |
There was a problem hiding this comment.
The cache is not part of a "minimal implementation" and is unnecessary for this example. The current implementation in this PR combines the existing in-memory log store with file-backed storage, which is likely to confuse readers. Please remove the cache and directly read/write from/to files.
| std::cout << "[TRACE][file_based_state_mgr_ctor] Creating state manager for server " | ||
| << srv_id << " at " << endpoint << ", state dir: " << state_dir << std::endl; |
There was a problem hiding this comment.
Please get rid of these stdout messages, or pass stuff.raft_logger_ here and use it instead.
|
thanks for the review. I'll do these changes on the weekend. |
2 participants
This provides a minimal example of durable storage for NuRaft,
addressing the gap between in-memory examples and production use. This comes after the discussion in the PR: #632