Skip to content

Commit

Permalink
ballot: switch to c++17 and refactor ballot (#3)
Browse files Browse the repository at this point in the history
- upgrade to c++17
- refactor ballot
  • Loading branch information
ehds committed Apr 16, 2024
1 parent 219a65c commit 6ed1e95
Show file tree
Hide file tree
Showing 33 changed files with 207 additions and 182 deletions.
2 changes: 1 addition & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ cc_library(
"-Woverloaded-virtual",
"-Wnon-virtual-dtor",
"-Wno-missing-field-initializers",
"-std=c++11",
"-std=c++17",
"-DGFLAGS_NS=google",
],
linkopts = [
Expand Down
14 changes: 7 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,19 @@ set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} ${DEBUG_SYMBOL}")
if(CMAKE_SYSTEM_PROCESSOR MATCHES "(x86)|(X86)|(amd64)|(AMD64)")
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -msse4 -msse4.2")
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CPP_FLAGS} -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -Wno-reserved-user-defined-literal -fno-omit-frame-pointer")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CMAKE_CPP_FLAGS} -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-unused-parameter -fno-omit-frame-pointer")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CPP_FLAGS} -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -Wno-reserved-user-defined-literal -fno-omit-frame-pointer")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CMAKE_CPP_FLAGS} -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-unused-parameter -fno-omit-frame-pointer")

macro(use_cxx11)
macro(use_cxx17)
if(CMAKE_VERSION VERSION_LESS "3.1.3")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
else()
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
endif()
endmacro(use_cxx11)
endmacro(use_cxx17)

use_cxx11()
use_cxx17()

add_subdirectory(src)
if(WITH_TESTS)
Expand Down
34 changes: 16 additions & 18 deletions src/braft/ballot.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright (c) 2018 Baidu.com, Inc. All Rights Reserved
//
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//
// http://www.apache.org/licenses/LICENSE-2.0
//
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -16,29 +16,29 @@

#include "braft/ballot.h"

namespace braft {
#include <cassert>
#include "braft/configuration.h"

Ballot::Ballot() : _quorum(0), _old_quorum(0) {}
Ballot::~Ballot() {}
namespace braft {

void Ballot::init(const Configuration& conf, const Configuration* old_conf) {
void Ballot::init(const Configuration& conf, std::optional<const Configuration> old_conf) {
_peers.clear();
_old_peers.clear();
_quorum = 0;
_old_quorum = 0;

CHECK_GT(conf.size(), 0);
_peers.reserve(conf.size());
for (Configuration::const_iterator
iter = conf.begin(); iter != conf.end(); ++iter) {
for (auto iter = conf.begin(); iter != conf.end(); ++iter) {
_peers.push_back(*iter);
}
_quorum = _peers.size() / 2 + 1;
if (!old_conf) {
if (!old_conf.has_value()) {
return;
}
_old_peers.reserve(old_conf->size());
for (Configuration::const_iterator
iter = old_conf->begin(); iter != old_conf->end(); ++iter) {
for (Configuration::const_iterator iter = old_conf->begin();
iter != old_conf->end(); ++iter) {
_old_peers.push_back(*iter);
}
_old_quorum = _old_peers.size() / 2 + 1;
Expand All @@ -54,11 +54,11 @@ Ballot::PosHint Ballot::grant(const PeerId& peer, PosHint hint) {
}
hint.pos0 = iter - _peers.begin();
} else {
hint.pos0 = -1;
hint.pos0 = 0;
}

if (_old_peers.empty()) {
hint.pos1 = -1;
hint.pos1 = 0;
return hint;
}

Expand All @@ -71,14 +71,12 @@ Ballot::PosHint Ballot::grant(const PeerId& peer, PosHint hint) {
}
hint.pos1 = iter - _old_peers.begin();
} else {
hint.pos1 = -1;
hint.pos1 = 0;
}

return hint;
}

void Ballot::grant(const PeerId& peer) {
grant(peer, PosHint());
}
void Ballot::grant(const PeerId& peer) { grant(peer, PosHint()); }

} // namespace braft
74 changes: 34 additions & 40 deletions src/braft/ballot.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright (c) 2018 Baidu.com, Inc. All Rights Reserved
//
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//
// http://www.apache.org/licenses/LICENSE-2.0
//
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -14,63 +14,57 @@

// Authors: Zhangyi Chen([email protected])

#ifndef BRAFT_BALLOT_H
#define BRAFT_BALLOT_H
#pragma once
#include <algorithm>
#include <optional>
#include <vector>

#include "braft/configuration.h"

namespace braft {

class Ballot {
public:
public:
struct PosHint {
PosHint() : pos0(-1), pos1(-1) {}
int pos0;
int pos1;
PosHint() : pos0(0), pos1(0) {}
size_t pos0;
size_t pos1;
};

Ballot();
~Ballot();
void swap(Ballot& rhs) {
_peers.swap(rhs._peers);
std::swap(_quorum, rhs._quorum);
_old_peers.swap(rhs._old_peers);
std::swap(_old_quorum, rhs._old_quorum);
}

void init(const Configuration& conf, const Configuration* old_conf);
Ballot() : _quorum(0), _old_quorum(0){};
// FIXME(ehds): Remove optional.
// void init(const Configuration& conf, const Configuration& old_conf);
void init(const Configuration& conf,
std::optional<const Configuration> old_conf);
PosHint grant(const PeerId& peer, PosHint hint);
void grant(const PeerId& peer);
bool granted() const { return _quorum <= 0 && _old_quorum <= 0; }
private:
bool granted() const { return quorum() <= 0 && old_quorum() <= 0; }
int quorum() const { return _quorum; }
int old_quorum() const { return _old_quorum; }

private:
struct UnfoundPeerId {
UnfoundPeerId(const PeerId& peer_id) : peer_id(peer_id), found(false) {}
PeerId peer_id;
bool found;
bool operator==(const PeerId& id) const {
return peer_id == id;
}
bool operator==(const PeerId& id) const { return peer_id == id; }
};
std::vector<UnfoundPeerId>::iterator find_peer(
const PeerId& peer, std::vector<UnfoundPeerId>& peers, int pos_hint) {
if (pos_hint < 0 || pos_hint >= (int)peers.size()
|| peers[pos_hint].peer_id != peer) {
for (std::vector<UnfoundPeerId>::iterator
iter = peers.begin(); iter != peers.end(); ++iter) {
if (*iter == peer) {
return iter;
}
}
return peers.end();

using UnfoundPeerIds = std::vector<UnfoundPeerId>;
using UnfoundPeerIter = UnfoundPeerIds::iterator;

static UnfoundPeerIter find_peer(const PeerId& peer, UnfoundPeerIds& peers,
size_t pos_hint) {
if (pos_hint >= peers.size() || peers[pos_hint].peer_id != peer) {
return std::find(peers.begin(), peers.end(), peer);
}
return peers.begin() + pos_hint;
}
std::vector<UnfoundPeerId> _peers;

UnfoundPeerIds _peers;
int _quorum;
std::vector<UnfoundPeerId> _old_peers;
UnfoundPeerIds _old_peers;
int _old_quorum;
};

};

#endif //BRAFT_BALLOT_H
}; // namespace braft
8 changes: 5 additions & 3 deletions src/braft/ballot_box.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include <butil/scoped_lock.h>
#include <bvar/latency_recorder.h>
#include <bthread/unstable.h>
#include <cstddef>
#include <optional>
#include "braft/ballot_box.h"
#include "braft/util.h"
#include "braft/fsm_caller.h"
Expand Down Expand Up @@ -121,12 +123,12 @@ int BallotBox::reset_pending_index(int64_t new_pending_index) {
int BallotBox::append_pending_task(const Configuration& conf, const Configuration* old_conf,
Closure* closure) {
Ballot bl;
bl.init(conf, old_conf);
bl.init(conf,
old_conf == nullptr ? std::nullopt : std::make_optional(*old_conf));

BAIDU_SCOPED_LOCK(_mutex);
CHECK(_pending_index > 0);
_pending_meta_queue.push_back(Ballot());
_pending_meta_queue.back().swap(bl);
_pending_meta_queue.push_back(std::move(bl));
_closure_queue->append_pending_closure(closure);
return 0;
}
Expand Down
10 changes: 6 additions & 4 deletions src/braft/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
#include <brpc/errno.pb.h>
#include <brpc/controller.h>
#include <brpc/channel.h>
#include <optional>

#include "braft/configuration.h"
#include "braft/errno.pb.h"
#include "braft/util.h"
#include "braft/raft.h"
Expand Down Expand Up @@ -2068,9 +2070,9 @@ void NodeImpl::apply(LogEntryAndClosure tasks[], size_t size) {
entries.push_back(tasks[i].entry);
entries.back()->id.term = _current_term;
entries.back()->type = ENTRY_TYPE_DATA;
_ballot_box->append_pending_task(_conf.conf,
_conf.stable() ? NULL : &_conf.old_conf,
tasks[i].done);
_ballot_box->append_pending_task(
_conf.conf, _conf.stable() ? nullptr : &_conf.old_conf,
tasks[i].done);
}
_log_manager->append_entries(&entries,
new LeaderStableClosure(
Expand Down Expand Up @@ -3480,7 +3482,7 @@ void NodeImpl::get_leader_lease_status(LeaderLeaseStatus* lease_status) {

void NodeImpl::VoteBallotCtx::init(NodeImpl* node, bool triggered) {
reset(node);
_ballot.init(node->_conf.conf, node->_conf.stable() ? NULL : &(node->_conf.old_conf));
_ballot.init(node->_conf.conf, std::optional<Configuration>() );
_triggered = triggered;
}

Expand Down
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ include_directories(${CMAKE_SOURCE_DIR}/test)

set(CMAKE_CPP_FLAGS "-DGFLAGS_NS=${GFLAGS_NS}")
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -D__const__=__unused__ -D_GNU_SOURCE -DUSE_SYMBOLIZE -DNO_TCMALLOC -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DUNIT_TEST -g -Dprivate=public -Dprotected=public -D__STRICT_ANSI__ -include sstream_workaround.h")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CPP_FLAGS} -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -fno-omit-frame-pointer -Wno-unused-result")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CPP_FLAGS} -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -fno-omit-frame-pointer -Wno-unused-result")

if (WITH_COVERAGE)
if(CMAKE_C_COMPILER_ID STREQUAL "GNU")
Expand Down
6 changes: 6 additions & 0 deletions test/common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#pragma once
#ifdef private
#undef private
#endif
#include <gtest/gtest.h>
#define private public
50 changes: 26 additions & 24 deletions test/test_ballot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@

// Authors: Zhangyi Chen([email protected])

#include <gtest/gtest.h>
#include <optional>

#include "braft/ballot.h"
#include "common.h"

class BallotTest : public testing::Test {};

Expand All @@ -29,17 +31,17 @@ TEST(BallotTest, sanity) {
conf.add_peer(peer2);
conf.add_peer(peer3);
braft::Ballot bl;
bl.init(conf, NULL);
ASSERT_EQ(2, bl._quorum);
ASSERT_EQ(0, bl._old_quorum);
bl.init(conf, std::nullopt);
ASSERT_EQ(2, bl.quorum());
ASSERT_EQ(0, bl.old_quorum());
bl.grant(peer1);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl.quorum());
braft::Ballot::PosHint hint = bl.grant(peer1, braft::Ballot::PosHint());
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl.quorum());
hint = bl.grant(peer1, hint);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl.quorum());
hint = bl.grant(peer4, hint);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl.quorum());
hint = bl.grant(peer2, hint);
ASSERT_TRUE(bl.granted());
}
Expand All @@ -54,27 +56,27 @@ TEST(BallotTest, joint_consensus_same_conf) {
conf.add_peer(peer2);
conf.add_peer(peer3);
braft::Ballot bl;
bl.init(conf, &conf);
ASSERT_EQ(2, bl._quorum);
ASSERT_EQ(2, bl._old_quorum);
bl.init(conf, conf);
ASSERT_EQ(2, bl.quorum());
ASSERT_EQ(2, bl.old_quorum());
bl.grant(peer1);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl._old_quorum);
ASSERT_EQ(1, bl.quorum());
ASSERT_EQ(1, bl.old_quorum());
braft::Ballot::PosHint hint = bl.grant(peer1, braft::Ballot::PosHint());
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl._old_quorum);
ASSERT_EQ(1, bl.quorum());
ASSERT_EQ(1, bl.old_quorum());
hint = bl.grant(peer1, hint);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl._old_quorum);
ASSERT_EQ(1, bl.quorum());
ASSERT_EQ(1, bl.old_quorum());
hint = bl.grant(peer4, hint);
ASSERT_EQ(1, bl._quorum);
ASSERT_EQ(1, bl._old_quorum);
ASSERT_EQ(1, bl.quorum());
ASSERT_EQ(1, bl.old_quorum());
ASSERT_FALSE(bl.granted());
hint = bl.grant(peer2, hint);
ASSERT_TRUE(bl.granted());
hint = bl.grant(peer3, hint);
ASSERT_EQ(-1, bl._quorum);
ASSERT_EQ(-1, bl._old_quorum);
ASSERT_EQ(-1, bl.quorum());
ASSERT_EQ(-1, bl.old_quorum());
}

TEST(BallotTest, joint_consensus_different_conf) {
Expand All @@ -92,12 +94,12 @@ TEST(BallotTest, joint_consensus_different_conf) {
conf2.add_peer(peer3);
conf2.add_peer(peer4);
braft::Ballot bl;
bl.init(conf, &conf2);
bl.init(conf, conf2);
bl.grant(peer1);
bl.grant(peer2);
ASSERT_FALSE(bl.granted());
ASSERT_EQ(0, bl._quorum);
ASSERT_EQ(1, bl._old_quorum);
ASSERT_EQ(0, bl.quorum());
ASSERT_EQ(1, bl.old_quorum());
bl.grant(peer4);
ASSERT_TRUE(bl.granted());
}
Loading

0 comments on commit 6ed1e95

Please sign in to comment.