diff --git a/src/braft/configuration.h b/src/braft/configuration.h index e673008..dcfb8ce 100644 --- a/src/braft/configuration.h +++ b/src/braft/configuration.h @@ -73,7 +73,9 @@ struct PeerId { bool is_empty() const { return (addr.ip == butil::IP_ANY && addr.port == 0 && idx == 0); } + bool is_witness() const { return role == WITNESS; } + int parse(const std::string& str) { reset(); char ip_str[64]; diff --git a/src/braft/configuration_manager.cpp b/src/braft/configuration_manager.cpp index f92fce3..668f226 100644 --- a/src/braft/configuration_manager.cpp +++ b/src/braft/configuration_manager.cpp @@ -18,7 +18,7 @@ namespace braft { -int ConfigurationManager::add(const ConfigurationEntry& entry) { +int ConfigurationManager::add(ConfigurationEntry&& entry) { if (!_configurations.empty()) { if (_configurations.back().id.index >= entry.id.index) { CHECK(false) << "Did you forget to call truncate_suffix before " @@ -26,7 +26,7 @@ int ConfigurationManager::add(const ConfigurationEntry& entry) { return -1; } } - _configurations.push_back(entry); + _configurations.push_back(std::move(entry)); return 0; } @@ -44,9 +44,9 @@ void ConfigurationManager::truncate_suffix(const int64_t last_index_kept) { } } -void ConfigurationManager::set_snapshot(const ConfigurationEntry& entry) { +void ConfigurationManager::set_snapshot(ConfigurationEntry&& entry) { CHECK_GE(entry.id, _snapshot.id); - _snapshot = entry; + _snapshot = std::move(entry); } void ConfigurationManager::get(int64_t last_included_index, diff --git a/src/braft/configuration_manager.h b/src/braft/configuration_manager.h index f53c5c1..ef30a76 100644 --- a/src/braft/configuration_manager.h +++ b/src/braft/configuration_manager.h @@ -30,9 +30,17 @@ struct ConfigurationEntry { ConfigurationEntry() {} ConfigurationEntry(const LogEntry& entry) { id = entry.id; - conf = *(entry.peers); - if (entry.old_peers) { - old_conf = *(entry.old_peers); + conf = (entry.peers); + if (!entry.old_peers.empty()) { + old_conf = entry.old_peers; + } + } + + ConfigurationEntry(LogEntry&& entry) { + id = entry.id; + conf = std::move(entry.peers); + if (!entry.old_peers.empty()) { + old_conf = std::move(entry.old_peers); } } @@ -55,7 +63,7 @@ class ConfigurationManager { ~ConfigurationManager() {} // add new configuration at index - int add(const ConfigurationEntry& entry); + int add(ConfigurationEntry&& entry); // [1, first_index_kept) are being discarded void truncate_prefix(int64_t first_index_kept); @@ -63,7 +71,7 @@ class ConfigurationManager { // (last_index_kept, infinity) are being discarded void truncate_suffix(int64_t last_index_kept); - void set_snapshot(const ConfigurationEntry& snapshot); + void set_snapshot(ConfigurationEntry&& snapshot); void get(int64_t last_included_index, ConfigurationEntry* entry); diff --git a/src/braft/fsm_caller.cpp b/src/braft/fsm_caller.cpp index 7cb34ff..0aeb9a6 100644 --- a/src/braft/fsm_caller.cpp +++ b/src/braft/fsm_caller.cpp @@ -275,11 +275,11 @@ void FSMCaller::do_committed(int64_t committed_index) { for (; iter_impl.is_good();) { if (iter_impl.entry()->type != ENTRY_TYPE_DATA) { if (iter_impl.entry()->type == ENTRY_TYPE_CONFIGURATION) { - if (iter_impl.entry()->old_peers == NULL) { + if (!iter_impl.entry()->old_peers.empty()) { // Joint stage is not supposed to be noticeable by end // users. _fsm->on_configuration_committed( - Configuration(*iter_impl.entry()->peers), + Configuration(iter_impl.entry()->peers), iter_impl.entry()->id.index); } } diff --git a/src/braft/log.cpp b/src/braft/log.cpp index a50d7a7..d5cc4dc 100644 --- a/src/braft/log.cpp +++ b/src/braft/log.cpp @@ -327,8 +327,7 @@ int Segment::load(ConfigurationManager* configuration_manager) { entry->id.term = header.term; butil::Status status = parse_configuration_meta(data, entry); if (status.ok()) { - ConfigurationEntry conf_entry(*entry); - configuration_manager->add(conf_entry); + configuration_manager->add({std::move(*entry)}); } else { LOG(ERROR) << "fail to parse configuration meta, path: " << _path << " entry_off " << entry_off; diff --git a/src/braft/log_entry.cpp b/src/braft/log_entry.cpp index a7eb162..4c5490e 100644 --- a/src/braft/log_entry.cpp +++ b/src/braft/log_entry.cpp @@ -22,15 +22,9 @@ namespace braft { bvar::Adder g_nentries("raft_num_log_entries"); -LogEntry::LogEntry() : type(ENTRY_TYPE_UNKNOWN), peers(NULL), old_peers(NULL) { - g_nentries << 1; -} +LogEntry::LogEntry() : type(ENTRY_TYPE_UNKNOWN) { g_nentries << 1; } -LogEntry::~LogEntry() { - g_nentries << -1; - delete peers; - delete old_peers; -} +LogEntry::~LogEntry() { g_nentries << -1; } butil::Status parse_configuration_meta(const butil::IOBuf& data, LogEntry* entry) { @@ -41,14 +35,12 @@ butil::Status parse_configuration_meta(const butil::IOBuf& data, status.set_error(EINVAL, "Fail to parse ConfigurationPBMeta"); return status; } - entry->peers = new std::vector; for (int j = 0; j < meta.peers_size(); ++j) { - entry->peers->push_back(PeerId(meta.peers(j))); + entry->peers.push_back(PeerId(meta.peers(j))); } if (meta.old_peers_size() > 0) { - entry->old_peers = new std::vector; for (int i = 0; i < meta.old_peers_size(); i++) { - entry->old_peers->push_back(PeerId(meta.old_peers(i))); + entry->old_peers.push_back(PeerId(meta.old_peers(i))); } } return status; @@ -58,12 +50,12 @@ butil::Status serialize_configuration_meta(const LogEntry* entry, butil::IOBuf& data) { butil::Status status; ConfigurationPBMeta meta; - for (size_t i = 0; i < entry->peers->size(); ++i) { - meta.add_peers((*(entry->peers))[i].to_string()); + for (size_t i = 0; i < entry->peers.size(); ++i) { + meta.add_peers((entry->peers)[i].to_string()); } - if (entry->old_peers) { - for (size_t i = 0; i < entry->old_peers->size(); ++i) { - meta.add_old_peers((*(entry->old_peers))[i].to_string()); + if (!entry->old_peers.empty()) { + for (size_t i = 0; i < entry->old_peers.size(); ++i) { + meta.add_old_peers((entry->old_peers)[i].to_string()); } } butil::IOBufAsZeroCopyOutputStream wrapper(&data); diff --git a/src/braft/log_entry.h b/src/braft/log_entry.h index 4633103..3831190 100644 --- a/src/braft/log_entry.h +++ b/src/braft/log_entry.h @@ -40,8 +40,8 @@ struct LogEntry : public butil::RefCountedThreadSafe { public: EntryType type; // log type LogId id; - std::vector* peers; // peers - std::vector* old_peers; // peers + std::vector peers; // peers + std::vector old_peers; // peers butil::IOBuf data; LogEntry(); diff --git a/src/braft/log_manager.cpp b/src/braft/log_manager.cpp index 5adb136..c64e67c 100644 --- a/src/braft/log_manager.cpp +++ b/src/braft/log_manager.cpp @@ -419,8 +419,7 @@ void LogManager::append_entries(std::vector* entries, // Add ref for disk_thread (*entries)[i]->AddRef(); if ((*entries)[i]->type == ENTRY_TYPE_CONFIGURATION) { - ConfigurationEntry conf_entry(*((*entries)[i])); - _config_manager->add(conf_entry); + _config_manager->add({*((*entries)[i])}); } } @@ -631,7 +630,7 @@ void LogManager::set_snapshot(const SnapshotMeta* meta) { entry.id = LogId(meta->last_included_index(), meta->last_included_term()); entry.conf = conf; entry.old_conf = old_conf; - _config_manager->set_snapshot(entry); + _config_manager->set_snapshot(std::move(entry)); int64_t term = unsafe_get_term(meta->last_included_index()); const LogId last_but_one_snapshot_id = _last_snapshot_id; diff --git a/src/braft/node.cpp b/src/braft/node.cpp index a8de78b..228661a 100644 --- a/src/braft/node.cpp +++ b/src/braft/node.cpp @@ -487,8 +487,7 @@ int NodeImpl::bootstrap(const BootstrapOptions& options) { entry->AddRef(); entry->id.term = _current_term; entry->type = ENTRY_TYPE_CONFIGURATION; - entry->peers = new std::vector; - options.group_conf.list_peers(entry->peers); + options.group_conf.list_peers(&(entry->peers)); std::vector entries; entries.push_back(entry); @@ -2141,11 +2140,9 @@ void NodeImpl::unsafe_apply_configuration(const Configuration& new_conf, entry->AddRef(); entry->id.term = _current_term; entry->type = ENTRY_TYPE_CONFIGURATION; - entry->peers = new std::vector; - new_conf.list_peers(entry->peers); + new_conf.list_peers(&(entry->peers)); if (old_conf) { - entry->old_peers = new std::vector; - old_conf->list_peers(entry->old_peers); + old_conf->list_peers(&(entry->old_peers)); } ConfigurationChangeDone* configuration_change_done = new ConfigurationChangeDone(this, _current_term, leader_start, @@ -2593,15 +2590,13 @@ void NodeImpl::handle_append_entries_request( log_entry->id.index = index; log_entry->type = (EntryType)entry.type(); if (entry.peers_size() > 0) { - log_entry->peers = new std::vector; for (int i = 0; i < entry.peers_size(); i++) { - log_entry->peers->push_back(entry.peers(i)); + log_entry->peers.push_back(entry.peers(i)); } CHECK_EQ(log_entry->type, ENTRY_TYPE_CONFIGURATION); if (entry.old_peers_size() > 0) { - log_entry->old_peers = new std::vector; for (int i = 0; i < entry.old_peers_size(); i++) { - log_entry->old_peers->push_back(entry.old_peers(i)); + log_entry->old_peers.push_back(entry.old_peers(i)); } } } else { diff --git a/src/braft/replicator.cpp b/src/braft/replicator.cpp index 64bc1d0..378bcd2 100644 --- a/src/braft/replicator.cpp +++ b/src/braft/replicator.cpp @@ -24,6 +24,9 @@ #include // std::unique_ptr #include // DEFINE_int32 +#include +#include + #include "braft/ballot_box.h" // BallotBox #include "braft/log_entry.h" // LogEntry #include "braft/node.h" // NodeImpl @@ -627,14 +630,14 @@ int Replicator::_prepare_entry(int offset, EntryMeta* em, butil::IOBuf* data) { } em->set_term(entry->id.term); em->set_type(entry->type); - if (entry->peers != NULL) { - CHECK(!entry->peers->empty()) << "log_index=" << log_index; - for (size_t i = 0; i < entry->peers->size(); ++i) { - em->add_peers((*entry->peers)[i].to_string()); + if (!entry->peers.empty()) { + CHECK(!entry->peers.empty()) << "log_index=" << log_index; + for (size_t i = 0; i < entry->peers.size(); ++i) { + em->add_peers((entry->peers)[i].to_string()); } - if (entry->old_peers != NULL) { - for (size_t i = 0; i < entry->old_peers->size(); ++i) { - em->add_old_peers((*entry->old_peers)[i].to_string()); + if (!entry->old_peers.empty()) { + for (size_t i = 0; i < entry->old_peers.size(); ++i) { + em->add_old_peers((entry->old_peers)[i].to_string()); } } } else { @@ -1572,7 +1575,10 @@ int ReplicatorGroup::find_the_next_candidate(PeerId* peer_id, iter != _rmap.end(); ++iter) { peers.emplace_back(peerInfo(iter->first, iter->second)); } - std::random_shuffle(peers.begin(), peers.end()); + + std::random_device rd; + std::mt19937 g(rd()); + std::shuffle(peers.begin(), peers.end(), g); for (auto iter = peers.begin(); iter != peers.end(); ++iter) { if (!conf.contains(iter->peer_id)) { continue; diff --git a/test/test_ballot.cpp b/test/test_ballot.cpp index 7a2e737..a8d1627 100644 --- a/test/test_ballot.cpp +++ b/test/test_ballot.cpp @@ -17,6 +17,7 @@ #include #include "braft/ballot.h" +#include "bthread/countdown_event.h" #include "common.h" class BallotTest : public testing::Test {}; diff --git a/test/test_configuration.cpp b/test/test_configuration.cpp index d7e04c3..d1f8e31 100644 --- a/test/test_configuration.cpp +++ b/test/test_configuration.cpp @@ -135,7 +135,8 @@ TEST_F(TestUsageSuits, ConfigurationManager) { peers.emplace_back("1.1.1.1:1000:2"); entry.conf = peers; entry.id = {8, 1}; - conf_manager.add(entry); + conf_manager.add(ConfigurationEntry{entry}); + ASSERT_EQ(LogId(8, 1), conf_manager.last_configuration().id); conf_manager.get(10, &it1); @@ -146,11 +147,11 @@ TEST_F(TestUsageSuits, ConfigurationManager) { entry.id = LogId(10, 1); entry.conf = peers; - conf_manager.add(entry); + conf_manager.add(ConfigurationEntry{entry}); peers.emplace_back("1.1.1.1:1000:3"); entry.id = LogId(20, 1); entry.conf = peers; - conf_manager.add(entry); + conf_manager.add(ConfigurationEntry{entry}); ASSERT_EQ(LogId(20, 1), conf_manager.last_configuration().id); conf_manager.truncate_prefix(15); diff --git a/test/test_log.cpp b/test/test_log.cpp index 537b492..a11d540 100644 --- a/test/test_log.cpp +++ b/test/test_log.cpp @@ -815,10 +815,9 @@ TEST_F(LogStorageTest, configuration) { entry.type = braft::ENTRY_TYPE_CONFIGURATION; entry.id.term = 1; entry.id.index = 2; - entry.peers = new std::vector; - entry.peers->push_back(braft::PeerId("1.1.1.1:1000:0")); - entry.peers->push_back(braft::PeerId("1.1.1.1:2000:0")); - entry.peers->push_back(braft::PeerId("1.1.1.1:3000:0")); + entry.peers.push_back(braft::PeerId("1.1.1.1:1000:0")); + entry.peers.push_back(braft::PeerId("1.1.1.1:2000:0")); + entry.peers.push_back(braft::PeerId("1.1.1.1:3000:0")); storage->append_entry(&entry); } @@ -851,9 +850,8 @@ TEST_F(LogStorageTest, configuration) { entry.type = braft::ENTRY_TYPE_CONFIGURATION; entry.id.term = 1; entry.id.index = index; - entry.peers = new std::vector; - entry.peers->push_back(braft::PeerId("1.1.1.1:1000:0")); - entry.peers->push_back(braft::PeerId("1.1.1.1:2000:0")); + entry.peers.push_back(braft::PeerId("1.1.1.1:1000:0")); + entry.peers.push_back(braft::PeerId("1.1.1.1:2000:0")); storage->append_entry(&entry); } @@ -1109,14 +1107,12 @@ TEST_F(LogStorageTest, joint_configuration) { for (int i = 1; i <= 20; ++i) { scoped_refptr entry = new braft::LogEntry; entry->id = braft::LogId(i, 1); - entry->peers = new std::vector; entry->type = braft::ENTRY_TYPE_CONFIGURATION; for (int j = 0; j < 3; ++j) { - entry->peers->push_back("127.0.0.1:" + std::to_string(i + j)); + entry->peers.push_back("127.0.0.1:" + std::to_string(i + j)); } - entry->old_peers = new std::vector; for (int j = 1; j <= 3; ++j) { - entry->old_peers->push_back("127.0.0.1:" + std::to_string(i + j)); + entry->old_peers.push_back("127.0.0.1:" + std::to_string(i + j)); } ASSERT_EQ(0, log_storage->append_entry(entry)); } @@ -1125,8 +1121,8 @@ TEST_F(LogStorageTest, joint_configuration) { braft::LogEntry* entry = log_storage->get_entry(i); ASSERT_TRUE(entry != NULL); ASSERT_EQ(entry->type, braft::ENTRY_TYPE_CONFIGURATION); - ASSERT_TRUE(entry->peers != NULL); - ASSERT_TRUE(entry->old_peers != NULL); + ASSERT_TRUE(!entry->peers.empty()); + ASSERT_TRUE(!entry->old_peers.empty()); braft::Configuration conf; for (int j = 0; j < 3; ++j) { conf.add_peer("127.0.0.1:" + std::to_string(i + j)); @@ -1135,10 +1131,10 @@ TEST_F(LogStorageTest, joint_configuration) { for (int j = 1; j <= 3; ++j) { old_conf.add_peer("127.0.0.1:" + std::to_string(i + j)); } - ASSERT_TRUE(conf.equals(*entry->peers)) - << conf << " xxxx " << braft::Configuration(*entry->peers); + ASSERT_TRUE(conf.equals(entry->peers)) + << conf << " xxxx " << braft::Configuration(entry->peers); - ASSERT_TRUE(old_conf.equals(*entry->old_peers)); + ASSERT_TRUE(old_conf.equals(entry->old_peers)); entry->Release(); } @@ -1149,8 +1145,8 @@ TEST_F(LogStorageTest, joint_configuration) { braft::LogEntry* entry = log_storage->get_entry(i); ASSERT_TRUE(entry != NULL); ASSERT_EQ(entry->type, braft::ENTRY_TYPE_CONFIGURATION); - ASSERT_TRUE(entry->peers != NULL); - ASSERT_TRUE(entry->old_peers != NULL); + ASSERT_TRUE(!entry->peers.empty()); + ASSERT_TRUE(!entry->old_peers.empty()); braft::Configuration conf; for (int j = 0; j < 3; ++j) { conf.add_peer("127.0.0.1:" + std::to_string(i + j)); @@ -1159,10 +1155,10 @@ TEST_F(LogStorageTest, joint_configuration) { for (int j = 1; j <= 3; ++j) { old_conf.add_peer("127.0.0.1:" + std::to_string(i + j)); } - ASSERT_TRUE(conf.equals(*entry->peers)) - << conf << " xxxx " << braft::Configuration(*entry->peers); + ASSERT_TRUE(conf.equals(entry->peers)) + << conf << " xxxx " << braft::Configuration(entry->peers); - ASSERT_TRUE(old_conf.equals(*entry->old_peers)); + ASSERT_TRUE(old_conf.equals(entry->old_peers)); entry->Release(); } @@ -1170,8 +1166,8 @@ TEST_F(LogStorageTest, joint_configuration) { braft::LogEntry* entry = log_storage->get_entry(i); ASSERT_TRUE(entry != NULL); ASSERT_EQ(entry->type, braft::ENTRY_TYPE_CONFIGURATION); - ASSERT_TRUE(entry->peers != NULL); - ASSERT_TRUE(entry->old_peers != NULL); + ASSERT_TRUE(!entry->peers.empty()); + ASSERT_TRUE(!entry->old_peers.empty()); ASSERT_EQ(1, entry->id.term); braft::Configuration conf; for (int j = 0; j < 3; ++j) { @@ -1181,8 +1177,8 @@ TEST_F(LogStorageTest, joint_configuration) { for (int j = 1; j <= 3; ++j) { old_conf.add_peer("127.0.0.1:" + std::to_string(i + j)); } - ASSERT_TRUE(conf.equals(*entry->peers)); - ASSERT_TRUE(old_conf.equals(*entry->old_peers)); + ASSERT_TRUE(conf.equals(entry->peers)); + ASSERT_TRUE(old_conf.equals(entry->old_peers)); entry->Release(); } diff --git a/test/test_log_entry.cpp b/test/test_log_entry.cpp index bae8a5d..08298ca 100644 --- a/test/test_log_entry.cpp +++ b/test/test_log_entry.cpp @@ -35,7 +35,7 @@ TEST_F(TestUsageSuits, LogEntry) { peers.push_back(braft::PeerId("1.2.3.4:2000")); peers.push_back(braft::PeerId("1.2.3.4:3000")); entry->type = braft::ENTRY_TYPE_CONFIGURATION; - entry->peers = new std::vector(peers); + entry->peers = std::move(peers); entry->AddRef(); entry->Release(); diff --git a/test/test_log_manager.cpp b/test/test_log_manager.cpp index bc8883c..58cadf8 100644 --- a/test/test_log_manager.cpp +++ b/test/test_log_manager.cpp @@ -141,10 +141,9 @@ TEST_F(LogManagerTest, configuration_changes) { braft::LogEntry* entry = new braft::LogEntry; entry->AddRef(); entry->type = braft::ENTRY_TYPE_CONFIGURATION; - entry->peers = new std::vector(peers); + entry->peers = peers; if (peers.size() > 1u) { - entry->old_peers = new std::vector( - peers.begin() + 1, peers.end()); + entry->old_peers = {peers.begin() + 1, peers.end()}; } entry->AddRef(); entry->id = braft::LogId(i + 1, 1); @@ -198,7 +197,7 @@ TEST_F(LogManagerTest, truncate_suffix_also_revert_configuration) { braft::LogEntry* entry = new braft::LogEntry; entry->AddRef(); entry->type = braft::ENTRY_TYPE_CONFIGURATION; - entry->peers = new std::vector(peers); + entry->peers = std::move(peers); entry->AddRef(); entry->id = braft::LogId(i + 1, 1); saved_entries[i] = entry; @@ -379,7 +378,7 @@ TEST_F(LogManagerTest, pipelined_append) { entry->AddRef(); entry->type = braft::ENTRY_TYPE_CONFIGURATION; entry->id = braft::LogId(N, 1); - entry->peers = new std::vector(peers); + entry->peers = std::move(peers); entries0.push_back(entry); } SyncClosure sc0; @@ -410,7 +409,7 @@ TEST_F(LogManagerTest, pipelined_append) { entry->AddRef(); entry->type = braft::ENTRY_TYPE_CONFIGURATION; entry->id = braft::LogId(N, 2); - entry->peers = new std::vector(peers); + entry->peers = std::move(peers); entries1.push_back(entry); } SyncClosure sc1;