revived_from_all_failed: split KeyValuePairSplitter into different pr

src/brpc/cluster_recover_policy.cpp

@@ -54,6 +54,7 @@ bool DefaultClusterRecoverPolicy::StopRecoverIfNecessary() {
     if (_last_usable_change_time_ms != 0 && _last_usable != 0 &&
             (now_ms - _last_usable_change_time_ms > _hold_time_ms)) {
         _recovering = false;
+        _last_usable = 0;
         _last_usable_change_time_ms = 0;
         mu.unlock();
         return false;
@@ -109,26 +110,27 @@ bool GetRecoverPolicyByParams(const butil::StringPiece& params,
     int64_t minimum_working_instances = -1;
     int64_t hold_time_ms = -1;
     bool has_meet_params = false;
-    for (butil::KeyValuePairsSplitter sp(params.begin(), params.end(), '=', ' ');
-            sp; ++sp) {
-        if (sp.value().empty()) {
-            LOG(ERROR) << "Empty value for " << sp.key() << " in lb parameter";
+    for (butil::StringSplitter sp(params.begin(), params.end(), ' '); sp != nullptr; ++sp) {
+        butil::StringPiece key_value(sp.field(), sp.length());
+        size_t p = key_value.find('=');
+        if (p == key_value.npos || p == key_value.size() - 1) {
+            // No value configed.
             return false;
         }
-        if (sp.key() == "minimum_working_instances") {
-            if (!butil::StringToInt64(sp.value(), &minimum_working_instances)) {
+        if (key_value.substr(0, p) == "minimum_working_instances") {
+            if (!butil::StringToInt64(key_value.substr(p + 1), &minimum_working_instances)) {
                 return false;
             }
             has_meet_params = true;
             continue;
-        } else if (sp.key() == "hold_time_ms") {
-            if (!butil::StringToInt64(sp.value(), &hold_time_ms)) {
+        } else if (key_value.substr(0, p) == "hold_time_ms") {
+            if (!butil::StringToInt64(key_value.substr(p + 1), &hold_time_ms)) {
                 return false;
             }
             has_meet_params = true;
             continue;
         }
-        LOG(ERROR) << "Failed to set this unknown parameters " << sp.key_and_value();
+        LOG(ERROR) << "Failed to set this unknown parameters " << key_value;
     }
     if (minimum_working_instances > 0 && hold_time_ms > 0) {
         ptr_out->reset(

src/brpc/cluster_recover_policy.h

@@ -36,6 +36,8 @@ class ServerId;
 // servers when recovering from all servers are down.
 class ClusterRecoverPolicy {
 public:
+    virtual ~ClusterRecoverPolicy() {}
+
     // Indicate that recover from all server being down is happening.
     virtual void StartRecover() = 0;
 

src/brpc/policy/consistent_hashing_load_balancer.cpp

@@ -385,13 +385,13 @@ bool ConsistentHashingLoadBalancer::SetParameters(const butil::StringPiece& para
             LOG(ERROR) << "Empty value for " << sp.key() << " in lb parameter";
             return false;
         }
-        if (sp.key() == "replicas") {
-            if (!butil::StringToSizeT(sp.value(), &_num_replicas)) {
+        if (key_value.substr(0, p) == "replicas") {
+            if (!butil::StringToSizeT(key_value.substr(p + 1), &_num_replicas)) {
                 return false;
             }
             continue;
         }
-        LOG(ERROR) << "Failed to set this unknown parameters " << sp.key_and_value();
+        LOG(ERROR) << "Failed to set this unknown parameters " << key_value;
     }
     return true;
 }

src/butil/string_splitter.h

@@ -22,7 +22,6 @@
 
 #include <stdlib.h>
 #include <stdint.h>
-#include "butil/strings/string_piece.h"
 
 // It's common to encode data into strings separated by special characters
 // and decode them back, but functions such as `split_string' has to modify

test/string_splitter_unittest.cpp

@@ -386,56 +386,4 @@ TEST_F(StringSplitterTest, key_value_pairs_splitter_sanity) {
     }
 }
 
-TEST_F(StringSplitterTest, key_value_pairs_splitter_sanity) {
-    std::string kvstr = "key1=value1&key2=value2&key3=value3";
-    {
-        butil::KeyValuePairsSplitter splitter(kvstr, '=', '&');
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key1");
-        ASSERT_EQ(splitter.value(), "value1");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key2");
-        ASSERT_EQ(splitter.value(), "value2");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key3");
-        ASSERT_EQ(splitter.value(), "value3");
-        ++splitter;
-        ASSERT_FALSE(splitter);
-    }
-    {
-        butil::KeyValuePairsSplitter splitter(kvstr.data(), kvstr.data() + kvstr.size(), '=', '&');
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key1");
-        ASSERT_EQ(splitter.value(), "value1");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key2");
-        ASSERT_EQ(splitter.value(), "value2");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key3");
-        ASSERT_EQ(splitter.value(), "value3");
-        ++splitter;
-        ASSERT_FALSE(splitter);
-    }
-    {
-        butil::KeyValuePairsSplitter splitter(kvstr.c_str(), '=', '&');
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key1");
-        ASSERT_EQ(splitter.value(), "value1");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key2");
-        ASSERT_EQ(splitter.value(), "value2");
-        ++splitter;
-        ASSERT_TRUE(splitter);
-        ASSERT_EQ(splitter.key(), "key3");
-        ASSERT_EQ(splitter.value(), "value3");
-        ++splitter;
-        ASSERT_FALSE(splitter);
-    }
-}
-
 }