ballot: switch to c++17 and refactor ballot

BUILD

@@ -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 = [

CMakeLists.txt

@@ -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)

src/braft/ballot.cpp

@@ -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.
@@ -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;
@@ -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;
     }
 
@@ -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

src/braft/ballot.h

@@ -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.
@@ -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

src/braft/ballot_box.cpp

@@ -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"
@@ -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;
 }

src/braft/node.cpp

@@ -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"
@@ -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(
@@ -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;
 }
 

test/CMakeLists.txt

@@ -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")

test/common.h

@@ -0,0 +1,6 @@
+#pragma once
+#ifdef private
+#undef private
+#endif
+#include <gtest/gtest.h>
+#define private public

test/test_ballot.cpp

@@ -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 {};
 
@@ -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());
 }
@@ -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) {
@@ -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());
 }