bugs fix

src/brpc/global.cpp

@@ -108,7 +108,7 @@ const char* const DUMMY_SERVER_PORT_FILE = "dummy_server.port";
 
 struct GlobalExtensions {
     GlobalExtensions()
-        : ch_mh_lb("murmurhash32")
+        : ch_mh_lb("murmurhash3")
         , ch_md5_lb("md5")
         , ch_ketama_lb("ketama")
         , constant_cl(0) {

src/brpc/load_balancer.cpp

@@ -65,7 +65,10 @@ SharedLoadBalancer::~SharedLoadBalancer() {
 int SharedLoadBalancer::Init(const char* lb_protocol) {
     std::string lb_name;
     butil::StringPairs lb_parms;
-    ParseParameters(lb_protocol, &lb_name, &lb_parms);
+    if (!ParseParameters(lb_protocol, &lb_name, &lb_parms)) {
+        LOG(FATAL) << "Fail to parse this load balancer protocol '" << lb_protocol << '\'';
+        return -1;
+    }
     const LoadBalancer* lb = LoadBalancerExtension()->Find(lb_name.c_str());
     if (lb == NULL) {
         LOG(FATAL) << "Fail to find LoadBalancer by `" << lb_name << "'";
@@ -96,20 +99,28 @@ void SharedLoadBalancer::Describe(std::ostream& os,
     }
 }
 
-void SharedLoadBalancer::ParseParameters(const butil::StringPiece lb_protocol, 
+bool SharedLoadBalancer::ParseParameters(const butil::StringPiece& lb_protocol,
                                          std::string* lb_name,
-                                         butil::StringPairs* parms) {
+                                         butil::StringPairs* lb_params) {
     lb_name->clear();
-    parms->clear();
+    lb_params->clear();
+    if (lb_protocol.empty()) {
+        return false;
+    }
     size_t pos = lb_protocol.find(':');
     if (pos == std::string::npos) {
         lb_name->append(lb_protocol.data(), lb_protocol.size());
     } else {
         lb_name->append(lb_protocol.data(), pos);
-        butil::StringPiece parms_piece = lb_protocol.substr(pos + sizeof(':'));
-        std::string parms_str(parms_piece.data(), parms_piece.size());
-        butil::SplitStringIntoKeyValuePairs(parms_str, '=', ' ', parms);
+        butil::StringPiece params_piece = lb_protocol.substr(pos + sizeof(':'));
+        std::string params_str(params_piece.data(), params_piece.size());
+        if (!butil::SplitStringIntoKeyValuePairs(params_str, '=', ' ', lb_params)) {
+            lb_params->clear();
+            return false;
         }
+    }
+
+    return true;
 }
 																				 
 } // namespace brpc

src/brpc/load_balancer.h

@@ -104,6 +104,7 @@ public:
     // Caller is responsible for Destroy() the instance after usage.
     virtual LoadBalancer* New() const = 0;
 
+    // Set other
     virtual bool SetParameters(const butil::StringPairs& parms) { return true; }
 
 protected:
@@ -168,9 +169,9 @@ public:
     }
 
 private:
-    static void ParseParameters(const butil::StringPiece lb_protocl, 
+    static bool ParseParameters(const butil::StringPiece& lb_protocol,
                                 std::string* lb_name,
-                                butil::StringPairs* parms);
+                                butil::StringPairs* lb_params);
     static void DescribeLB(std::ostream& os, void* arg);
     void ExposeLB();
 

src/brpc/policy/consistent_hashing_load_balancer.cpp

@@ -94,7 +94,7 @@ ConsistentHashingLoadBalancer::ConsistentHashingLoadBalancer(const char* name)
 }
 
 void ConsistentHashingLoadBalancer::Init(const std::string& name) {
-    if (name.compare("murmurhash3") == 0) {
+    if (name == "murmurhash3") {
         _build_replicas = std::bind(BuildReplicasDefault,
                                     std::placeholders::_1,
                                     std::placeholders::_2,
@@ -102,7 +102,7 @@ void ConsistentHashingLoadBalancer::Init(const std::string& name) {
                                     std::placeholders::_3);
         return;
     }
-    if (name.compare("md5") == 0) {
+    if (name == "md5") {
         _build_replicas = std::bind(BuildReplicasDefault,
                                     std::placeholders::_1,
                                     std::placeholders::_2,
@@ -110,7 +110,7 @@ void ConsistentHashingLoadBalancer::Init(const std::string& name) {
                                     std::placeholders::_3);
         return;
     }
-    if (name.compare("ketama") == 0) {
+    if (name == "ketama") {
         _build_replicas = BuildReplicasKetam;
         return;
     }
@@ -341,18 +341,18 @@ void ConsistentHashingLoadBalancer::GetLoads(
     }
 }
 
-bool ConsistentHashingLoadBalancer::SetParameters(const butil::StringPairs& parms) {
-    for (const std::pair<std::string, std::string>& parm : parms) {
-        if (parm.first.compare("replicas") == 0) {
+bool ConsistentHashingLoadBalancer::SetParameters(const butil::StringPairs& params) {
+    for (const std::pair<std::string, std::string>& param : params) {
+        if (param.first == "replicas") {
             size_t replicas = 0;
-            if (butil::StringToSizeT(parm.second, &replicas)) {
+            if (butil::StringToSizeT(param.second, &replicas)) {
                 _num_replicas = replicas;
             } else {
                 return false;
             }
-        } else {
-            return false;
+            continue;
         }
+        LOG(ERROR) << "Failed to set this unknown parameters " << param.first << '=' << param.second;
     }
 
     return true;

src/brpc/policy/consistent_hashing_load_balancer.h

@@ -43,7 +43,7 @@ public:
             return hash < code;
         }
     };
-    using HashFun = uint32_t(*)(const void*, size_t);
+    using HashFunc = uint32_t(*)(const void*, size_t);
     using BuildReplicasFunc = 
         std::function<bool (const ServerId server, const size_t num_replicas, std::vector<Node>* replicas)>;
     explicit ConsistentHashingLoadBalancer(const char* name);

src/brpc/policy/hasher.cpp

@@ -157,7 +157,7 @@ uint32_t CRCHash32(const void* key, size_t len) {
 
 const char *GetHashName(uint32_t (*hasher)(const void* key, size_t len)) {
     if (hasher == MurmurHash32) {
-        return "murmurhash32";
+        return "murmurhash3";
     }
     if (hasher == MD5Hash32) {
         return "md5";

test/brpc_load_balancer_unittest.cpp

@@ -251,7 +251,7 @@ TEST_F(LoadBalancerTest, update_while_selection) {
         } else if (round == 3) {
             lb = new brpc::policy::WeightedRoundRobinLoadBalancer;
         } else {
-            lb = new brpc::policy::ConsistentHashingLoadBalancer("murmurhash32");
+            lb = new brpc::policy::ConsistentHashingLoadBalancer("murmurhash3");
             sa.hash = ::brpc::policy::MurmurHash32;
         }
         sa.lb = lb;
@@ -364,7 +364,7 @@ TEST_F(LoadBalancerTest, fairness) {
         } else if (3 == round || 4 == round) {
             lb = new brpc::policy::WeightedRoundRobinLoadBalancer;
         } else {
-            lb = new brpc::policy::ConsistentHashingLoadBalancer("murmurhash32");
+            lb = new brpc::policy::ConsistentHashingLoadBalancer("murmurhash3");
             sa.hash = brpc::policy::MurmurHash32;
         }
         sa.lb = lb;