Skip to content

Commit

Permalink
fix bug
Browse files Browse the repository at this point in the history
  • Loading branch information
JacksonYao287 committed Aug 23, 2024
1 parent c3e0ab3 commit 4191ea1
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 5 deletions.
4 changes: 3 additions & 1 deletion src/include/homestore/replication/repl_dev.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ struct repl_key {
};

bool operator==(repl_key const& other) const = default;
std::string to_string() const { return fmt::format("server={}, term={}, dsn={}", server_id, term, dsn); }
std::string to_string() const {
return fmt::format("server={}, term={}, dsn={}, hash={}", server_id, term, dsn, Hasher()(*this));
}
};

using repl_snapshot = nuraft::snapshot;
Expand Down
3 changes: 2 additions & 1 deletion src/lib/replication/repl_dev/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ uint8_t* repl_req_ctx::raw_journal_buf() { return std::get< std::unique_ptr< uin

void repl_req_ctx::set_lsn(int64_t lsn) {
DEBUG_ASSERT((m_lsn == -1) || (m_lsn == lsn),
"Changing lsn for request={} on the fly can cause race condition, not expected", to_string());
"Changing lsn for request={} on the fly can cause race condition, not expected. lsn {}, m_lsn {}",
to_string(), lsn, m_lsn);
m_lsn = lsn;
LOGTRACEMOD(replication, "Setting lsn={} for request={}", lsn, to_string());
}
Expand Down
8 changes: 7 additions & 1 deletion src/lib/replication/repl_dev/raft_repl_dev.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,13 @@ std::pair< bool, nuraft::cb_func::ReturnCode > RaftReplDev::handle_raft_event(nu
if (entry->get_val_type() != nuraft::log_val_type::app_log) { continue; }
if (entry->get_buf_ptr()->size() == 0) { continue; }
auto req = m_state_machine->localize_journal_entry_prepare(*entry);
if (req == nullptr) {
// TODO :: we need to indentify whether this log entry should be appended to log store.
// 1 for lsn, if the req#lsn is not -1, it means this log has been localized and apeneded before, we
// should skip it.
// 2 for dsn, if the req#dsn is less than the next_dsn, it means this log has been
// committed, we should skip it.
// here, we only check the first condition for now. revisit here if we need to check the second
if (req == nullptr || req->lsn() != -1) {
sisl::VectorPool< repl_req_ptr_t >::free(reqs);
return {true, nuraft::cb_func::ReturnCode::ReturnNull};
}
Expand Down
7 changes: 5 additions & 2 deletions src/tests/test_raft_repl_dev.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ class TestReplicatedDB : public homestore::ReplDevListener {
snp_data->blob = sisl::io_blob_safe(sizeof(ulong));
return 0;
}
if (snp_data->offset == -1) snp_data->offset = 0;
int64_t follower_last_lsn = snp_data->offset;
std::vector< KeyValuePair > kv_snapshot_data;
LOGINFOMOD(replication, "[Replica={}] Read logical snapshot callback follower lsn={}", g_helper->replica_num(),
Expand Down Expand Up @@ -223,8 +224,10 @@ class TestReplicatedDB : public homestore::ReplDevListener {
snp_data->is_last_obj);

if (snp_data->offset == 0) {
// For obj_id 0 we sent back the last committed lsn.
snp_data->offset = last_committed_lsn;
// TODO: this is a workaround to avoid the first snapshot data write error. revisit here and refine
// it. if last_committed_lsn is 0 and we send it back, then the leader will resend this to us, in which the
// snp_data->offset is 0. This will cause the snapshot data write error. So we set it to -1 to avoid this.
snp_data->offset = last_committed_lsn == 0 ? -1 : last_committed_lsn;
LOGINFOMOD(replication, "[Replica={}] Save logical snapshot callback return obj_id={}",
g_helper->replica_num(), snp_data->offset);
return;
Expand Down

0 comments on commit 4191ea1

Please sign in to comment.