SERVER-26449 BoundServerParameter

Implement BoundServerParameter class which accepts
getter/setter callbacks to be invoked on append()/setFromString().

Moved ExportedServerParameter to be a child of BoundServerParameter.
This noticably simplifies the implementation.

Implement LockedServerParameter class as child of
BoundServerParameter which provides storage and unique_lock<mutex>
guarded accessors.

4 files changed, 341 insertions, 163 deletions

diff --git a/src/mongo/db/server_parameters.cpp b/src/mongo/db/server_parameters.cpp
index ded970acd10..d1a9dc96280 100644
--- a/src/mongo/db/server_parameters.cpp
+++ b/src/mongo/db/server_parameters.cpp
@@ -83,50 +83,4 @@ void ServerParameterSet::add(ServerParameter* sp) {
     x = sp;
 }
 
-template <typename T, ServerParameterType paramType>
-Status ExportedServerParameter<T, paramType>::setFromString(const string& str) {
-    T value;
-    Status status = parseNumberFromString(str, &value);
-    if (!status.isOK())
-        return status;
-    return set(value);
-}
-
-#define EXPORTED_SERVER_PARAMETER(PARAM_TYPE)                                                   \
-    template <>                                                                                 \
-    Status ExportedServerParameter<bool, PARAM_TYPE>::setFromString(const string& str) {        \
-        if (str == "true" || str == "1")                                                        \
-            return set(true);                                                                   \
-        if (str == "false" || str == "0")                                                       \
-            return set(false);                                                                  \
-        return Status(ErrorCodes::BadValue, "can't convert string to bool");                    \
-    }                                                                                           \
-                                                                                                \
-    template Status ExportedServerParameter<int, PARAM_TYPE>::setFromString(const string& str); \
-                                                                                                \
-    template Status ExportedServerParameter<long long, PARAM_TYPE>::setFromString(              \
-        const string& str);                                                                     \
-                                                                                                \
-    template Status ExportedServerParameter<double, PARAM_TYPE>::setFromString(const string& str);
-
-// Define instances for each possible combination of number types we support, and
-// ServerParameterType
-EXPORTED_SERVER_PARAMETER(ServerParameterType::kStartupOnly);
-EXPORTED_SERVER_PARAMETER(ServerParameterType::kRuntimeOnly);
-EXPORTED_SERVER_PARAMETER(ServerParameterType::kStartupAndRuntime);
-
-template <>
-Status ExportedServerParameter<string, ServerParameterType::kStartupOnly>::setFromString(
-    const string& str) {
-    return set(str);
-}
-
-template <>
-Status ExportedServerParameter<vector<string>, ServerParameterType::kStartupOnly>::setFromString(
-    const string& str) {
-    vector<string> v;
-    splitStringDelim(str, &v, ',');
-    return set(v);
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/server_parameters.h b/src/mongo/db/server_parameters.h
index c5ff2e7ae46..04edefa3754 100644
--- a/src/mongo/db/server_parameters.h
+++ b/src/mongo/db/server_parameters.h
@@ -32,12 +32,16 @@
 
 #include <map>
 #include <string>
+#include <vector>
 
 #include "mongo/base/static_assert.h"
 #include "mongo/base/status.h"
 #include "mongo/db/jsobj.h"
 #include "mongo/platform/atomic_proxy.h"
 #include "mongo/platform/atomic_word.h"
+#include "mongo/stdx/functional.h"
+#include "mongo/stdx/mutex.h"
+#include "mongo/util/stringutils.h"
 
 namespace mongo {
 
@@ -135,6 +139,131 @@ enum class ServerParameterType {
 };
 
 /**
+ * Lets you make server level settings easily configurable.
+ * Hooks into (set|get)Parameter, as well as command line processing
+ */
+template <typename T>
+class BoundServerParameter : public ServerParameter {
+private:
+    using setter = stdx::function<Status(const T&)>;
+    using getter = stdx::function<T()>;
+    using SPT = ServerParameterType;
+
+public:
+    BoundServerParameter(const std::string& name,
+                         const setter set,
+                         const getter get,
+                         SPT paramType = SPT::kStartupOnly)
+        : BoundServerParameter(ServerParameterSet::getGlobal(), name, set, get, paramType) {}
+
+    BoundServerParameter(ServerParameterSet* sps,
+                         const std::string& name,
+                         const setter set,
+                         const getter get,
+                         SPT paramType = SPT::kStartupOnly)
+        : ServerParameter(sps,
+                          name,
+                          paramType == SPT::kStartupOnly || paramType == SPT::kStartupAndRuntime,
+                          paramType == SPT::kRuntimeOnly || paramType == SPT::kStartupAndRuntime),
+          _setter(set),
+          _getter(get) {}
+    ~BoundServerParameter() override = default;
+
+    void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) override {
+        b.append(name, _getter());
+    }
+
+    Status set(const BSONElement& newValueElement) override {
+        T newValue;
+
+        if (!newValueElement.coerce(&newValue)) {
+            return Status(ErrorCodes::BadValue, "Can't coerce value");
+        }
+
+        return _setter(newValue);
+    }
+
+    Status setFromString(const std::string& str) override;
+
+private:
+    const setter _setter;
+    const getter _getter;
+};
+
+template <>
+inline Status BoundServerParameter<bool>::setFromString(const std::string& str) {
+    if ((str == "1") || (str == "true")) {
+        return _setter(true);
+    }
+    if ((str == "0") || (str == "false")) {
+        return _setter(false);
+    }
+    return Status(ErrorCodes::BadValue, "Value is not a valid boolean");
+}
+
+template <>
+inline Status BoundServerParameter<std::string>::setFromString(const std::string& str) {
+    return _setter(str);
+}
+
+template <>
+inline Status BoundServerParameter<std::vector<std::string>>::setFromString(
+    const std::string& str) {
+    std::vector<std::string> v;
+    splitStringDelim(str, &v, ',');
+    return _setter(v);
+}
+
+template <typename T>
+inline Status BoundServerParameter<T>::setFromString(const std::string& str) {
+    T value;
+    Status status = parseNumberFromString(str, &value);
+    if (!status.isOK()) {
+        return status;
+    }
+    return _setter(value);
+}
+
+template <typename T>
+class LockedServerParameter : public BoundServerParameter<T> {
+private:
+    using SPT = ServerParameterType;
+
+public:
+    LockedServerParameter(const std::string& name,
+                          const T& initval,
+                          SPT paramType = SPT::kStartupAndRuntime)
+        : LockedServerParameter(ServerParameterSet::getGlobal(), name, initval, paramType) {}
+
+    LockedServerParameter(ServerParameterSet* sps,
+                          const std::string& name,
+                          const T& initval,
+                          SPT paramType = SPT::kStartupAndRuntime)
+        : BoundServerParameter<T>(sps,
+                                  name,
+                                  [this](const T& v) { return setLocked(v); },
+                                  [this]() { return getLocked(); },
+                                  paramType),
+          _value(initval) {}
+    ~LockedServerParameter() override = default;
+
+    Status setLocked(const T& value) {
+        stdx::unique_lock<stdx::mutex> lk(_mutex);
+        _value = value;
+        return Status::OK();
+    }
+
+    T getLocked() const {
+        stdx::unique_lock<stdx::mutex> lk(_mutex);
+        return _value;
+    }
+
+private:
+    mutable stdx::mutex _mutex;
+    T _value;
+};
+
+/**
  * Type trait for ServerParameterType to identify which types are safe to use at runtime because
  * they have std::atomic or equivalent types.
  */
@@ -164,35 +293,59 @@ template <typename T, ServerParameterType paramType>
 class server_parameter_storage_type {
 public:
     using value_type = AtomicWord<T>;
+    static T get(value_type* v) {
+        return v->load();
+    }
+    static void set(value_type* v, const T& newValue) {
+        v->store(newValue);
+    }
 };
 
 template <typename T>
 class server_parameter_storage_type<T, ServerParameterType::kStartupOnly> {
 public:
     using value_type = T;
+    static T get(value_type* v) {
+        return *v;
+    }
+    static void set(value_type* v, const T& newValue) {
+        *v = newValue;
+    }
 };
 
 template <>
 class server_parameter_storage_type<double, ServerParameterType::kRuntimeOnly> {
 public:
     using value_type = AtomicDouble;
+    static double get(value_type* v) {
+        return v->load();
+    }
+    static void set(value_type* v, const double& newValue) {
+        v->store(newValue);
+    }
 };
 
 template <>
 class server_parameter_storage_type<double, ServerParameterType::kStartupAndRuntime> {
 public:
     using value_type = AtomicDouble;
+    static double get(value_type* v) {
+        return v->load();
+    }
+    static void set(value_type* v, const double& newValue) {
+        v->store(newValue);
+    }
 };
 
 /**
- * Implementation of ServerParameter for reading and writing a server parameter with a given
+ * Implementation of BoundServerParameter for reading and writing a server parameter with a given
  * name and type into a specific C++ variable.
  *
  * NOTE: ServerParameters set at runtime can be read or written to at anytime, and are not
  * thread-safe without atomic types or other concurrency techniques.
  */
 template <typename T, ServerParameterType paramType>
-class ExportedServerParameter : public ServerParameter {
+class ExportedServerParameter : public BoundServerParameter<T> {
 public:
     MONGO_STATIC_ASSERT_MSG(paramType == ServerParameterType::kStartupOnly ||
                                 is_safe_runtime_parameter_type<T>::value,
@@ -209,21 +362,28 @@ public:
      * may be set at runtime, e.g.  via the setParameter command.
      */
     ExportedServerParameter(ServerParameterSet* sps, const std::string& name, storage_type* value)
-        : ServerParameter(sps,
-                          name,
-                          paramType == ServerParameterType::kStartupOnly ||
-                              paramType == ServerParameterType::kStartupAndRuntime,
-                          paramType == ServerParameterType::kRuntimeOnly ||
-                              paramType == ServerParameterType::kStartupAndRuntime),
+        : BoundServerParameter<T>(
+              sps,
+              name,
+              [this](const T& v) { return set(v); },
+              [this] { return server_parameter_storage_type<T, paramType>::get(_value); },
+              paramType),
           _value(value) {}
-    virtual ~ExportedServerParameter() {}
+    ~ExportedServerParameter() override {}
 
-    virtual void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name);
-
-    virtual Status set(const BSONElement& newValueElement);
-    virtual Status set(const T& newValue);
+    // Don't let the template method hide our inherited method
+    Status set(const BSONElement& newValueElement) override {
+        return BoundServerParameter<T>::set(newValueElement);
+    }
 
-    virtual Status setFromString(const std::string& str);
+    virtual Status set(const T& newValue) {
+        auto const status = validate(newValue);
+        if (!status.isOK()) {
+            return status;
+        }
+        server_parameter_storage_type<T, paramType>::set(_value, newValue);
+        return Status::OK();
+    }
 
 protected:
     virtual Status validate(const T& potentialNewValue) {
@@ -257,5 +417,3 @@ protected:
  */
 #define MONGO_EXPORT_RUNTIME_SERVER_PARAMETER(NAME, TYPE, INITIAL_VALUE) \
     MONGO_EXPORT_SERVER_PARAMETER_IMPL(NAME, TYPE, INITIAL_VALUE, ServerParameterType::kRuntimeOnly)
-
-#include "server_parameters_inline.h"
diff --git a/src/mongo/db/server_parameters_inline.h b/src/mongo/db/server_parameters_inline.h
deleted file mode 100644
index 21f862cf105..00000000000
--- a/src/mongo/db/server_parameters_inline.h
+++ /dev/null
@@ -1,95 +0,0 @@
-// server_parameters_inline.h
-
-/**
-*    Copyright (C) 2012 10gen Inc.
-*
-*    This program is free software: you can redistribute it and/or  modify
-*    it under the terms of the GNU Affero General Public License, version 3,
-*    as published by the Free Software Foundation.
-*
-*    This program is distributed in the hope that it will be useful,
-*    but WITHOUT ANY WARRANTY; without even the implied warranty of
-*    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-*    GNU Affero General Public License for more details.
-*
-*    You should have received a copy of the GNU Affero General Public License
-*    along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*
-*    As a special exception, the copyright holders give permission to link the
-*    code of portions of this program with the OpenSSL library under certain
-*    conditions as described in each individual source file and distribute
-*    linked combinations including the program with the OpenSSL library. You
-*    must comply with the GNU Affero General Public License in all respects for
-*    all of the code used other than as permitted herein. If you modify file(s)
-*    with this exception, you may extend this exception to your version of the
-*    file(s), but you are not obligated to do so. If you do not wish to do so,
-*    delete this exception statement from your version. If you delete this
-*    exception statement from all source files in the program, then also delete
-*    it in the license file.
-*/
-
-#include "mongo/util/stringutils.h"
-
-namespace mongo {
-
-// We create template specializations for each possible value type which is supported at runtime.
-// The only value types which are supported at runtime are types which can be stored in
-// AtomicWord<T> or AtomicProxy<T> which both have explicit load and store methods. The storage type
-// for a value type is chosen by the server_parameter_storage_type type trait. Since there is no
-// support for partial template specialization of member functions, we generate 4 (the Atomic types)
-// x 2 (RuntimeOnly, StartupAndRuntime) implementations of append and set.
-#define EXPORTED_ATOMIC_SERVER_PARAMETER_TYPE(VALUE_TYPE, PARAM_TYPE)          \
-    template <>                                                                \
-    inline void ExportedServerParameter<VALUE_TYPE, PARAM_TYPE>::append(       \
-        OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) { \
-        b.append(name, _value->load());                                        \
-    }                                                                          \
-                                                                               \
-    template <>                                                                \
-    inline Status ExportedServerParameter<VALUE_TYPE, PARAM_TYPE>::set(        \
-        const VALUE_TYPE& newValue) {                                          \
-        Status v = validate(newValue);                                         \
-        if (!v.isOK())                                                         \
-            return v;                                                          \
-                                                                               \
-        _value->store(newValue);                                               \
-        return Status::OK();                                                   \
-    }
-
-#define EXPORTED_ATOMIC_SERVER_PARAMETER(PARAM_TYPE)             \
-    EXPORTED_ATOMIC_SERVER_PARAMETER_TYPE(bool, PARAM_TYPE)      \
-    EXPORTED_ATOMIC_SERVER_PARAMETER_TYPE(int, PARAM_TYPE)       \
-    EXPORTED_ATOMIC_SERVER_PARAMETER_TYPE(long long, PARAM_TYPE) \
-    EXPORTED_ATOMIC_SERVER_PARAMETER_TYPE(double, PARAM_TYPE)
-
-EXPORTED_ATOMIC_SERVER_PARAMETER(ServerParameterType::kRuntimeOnly);
-EXPORTED_ATOMIC_SERVER_PARAMETER(ServerParameterType::kStartupAndRuntime);
-
-template <typename T, ServerParameterType paramType>
-inline Status ExportedServerParameter<T, paramType>::set(const BSONElement& newValueElement) {
-    T newValue;
-
-    if (!newValueElement.coerce(&newValue))
-        return Status(ErrorCodes::BadValue, "can't set value");
-
-    return set(newValue);
-}
-
-template <typename T, ServerParameterType paramType>
-inline Status ExportedServerParameter<T, paramType>::set(const T& newValue) {
-    Status v = validate(newValue);
-    if (!v.isOK())
-        return v;
-
-    *_value = newValue;
-    return Status::OK();
-}
-
-template <typename T, ServerParameterType paramType>
-void ExportedServerParameter<T, paramType>::append(OperationContext* opCtx,
-                                                   BSONObjBuilder& b,
-                                                   const std::string& name) {
-    b.append(name, *_value);
-}
-
-}  // namespace mongo
diff --git a/src/mongo/db/server_parameters_test.cpp b/src/mongo/db/server_parameters_test.cpp
index 83fdb3e93b5..f008ff0de95 100644
--- a/src/mongo/db/server_parameters_test.cpp
+++ b/src/mongo/db/server_parameters_test.cpp
@@ -37,21 +37,180 @@
 
 namespace mongo {
 
+namespace {
+auto const getStr = [](ServerParameter& sp) {
+    BSONObjBuilder b;
+    sp.append(nullptr, b, "x");
+    return b.obj().firstElement().String();
+};
+auto const getInt = [](ServerParameter& sp) {
+    BSONObjBuilder b;
+    sp.append(nullptr, b, "x");
+    return b.obj().firstElement().Int();
+};
+auto const getBool = [](ServerParameter& sp) {
+    BSONObjBuilder b;
+    sp.append(nullptr, b, "x");
+    return b.obj().firstElement().Bool();
+};
+
 using std::string;
 using std::vector;
 
+TEST(ServerParameters, boundInt) {
+    int ival = 123;
+    BoundServerParameter<int> bspi("bspl",
+                                   [&ival](const int& v) {
+                                       ival = v;
+                                       return Status::OK();
+                                   },
+                                   [&ival] { return ival; },
+                                   ServerParameterType::kStartupOnly);
+    ASSERT_EQUALS(123, getInt(bspi));
+
+    const struct {
+        std::string setval;
+        int expval;
+        bool succeed;
+    } setl[] = {
+        {"234", 234, true},
+        {"5.5", -1, false},
+        {"345", 345, true},
+        {"flowers", -1, false},
+        {"  456", -1, false},
+        {" 567 ", -1, false},
+        {"678  ", -1, false},
+        {"789-0", -1, false},
+    };
+    for (auto const& p : setl) {
+        ASSERT_EQ(bspi.setFromString(p.setval).isOK(), p.succeed);
+        if (p.succeed) {
+            ASSERT_EQUALS(p.expval, ival);
+            ASSERT_EQUALS(p.expval, getInt(bspi));
+        }
+    }
+}
+
+TEST(ServerParameter, boundBool) {
+    bool bval = true;
+    BoundServerParameter<bool> bspb("bspb",
+                                    [&bval](const bool& v) {
+                                        bval = v;
+                                        return Status::OK();
+                                    },
+                                    [&bval] { return bval; },
+                                    ServerParameterType::kStartupOnly);
+    ASSERT_TRUE(getBool(bspb));
+
+    struct {
+        std::string setval;
+        bool expval;
+        bool succeed;
+    } setb[] = {
+        {"1", true, true},
+        {"0", false, true},
+        {"true", true, true},
+        {"false", false, true},
+
+        {"yes", false, false},
+        {"no", false, false},
+        {"", false, false},
+        {"-1", false, false},
+    };
+    for (auto const& p : setb) {
+        ASSERT_EQ(bspb.setFromString(p.setval).isOK(), p.succeed);
+        if (p.succeed) {
+            ASSERT_EQUALS(p.expval, bval);
+            ASSERT_EQUALS(p.expval, getBool(bspb));
+        }
+    }
+}
+
+TEST(ServerParameters, boundStringExplicitLock) {
+    stdx::mutex mut;
+    std::string value("initial");
+    BoundServerParameter<std::string> bspsel("bsp",
+                                             [&value, &mut](const std::string& v) {
+                                                 stdx::unique_lock<stdx::mutex> lk(mut);
+                                                 value = v;
+                                                 return Status::OK();
+                                             },
+                                             [&value, &mut] {
+                                                 stdx::unique_lock<stdx::mutex> lk(mut);
+                                                 return value;
+                                             });
+
+    ASSERT_EQUALS("initial", getStr(bspsel));
+
+    const std::string sets[] = {"first-set", "second-set", "third-set"};
+    for (auto const& p : sets) {
+        ASSERT_TRUE(bspsel.set(BSON("x" << p).firstElement()).isOK());
+        ASSERT_EQUALS(p, getStr(bspsel));
+    }
+}
+
+TEST(ServerParameters, boundIntLock) {
+    LockedServerParameter<int> bspi("lsp", 1234);
+    ASSERT_EQUALS(1234, getInt(bspi));
+    ASSERT_EQUALS(1234, bspi.getLocked());
+
+    std::ostringstream maxint;
+    maxint << std::numeric_limits<int>::max();
+    std::ostringstream lowint;
+    lowint << (std::numeric_limits<int>::lowest() + 1);
+
+    std::ostringstream toobig;
+    toobig << std::numeric_limits<int>::max() << "0";
+    std::ostringstream toosmall;
+    toosmall << std::numeric_limits<int>::lowest() << "0";
+
+    const struct {
+        std::string setstr;
+        int setint;
+        bool succeed;
+    } sets[] = {
+        {"5678", 5678, true},
+        {"67", 67, true},
+        {maxint.str(), std::numeric_limits<int>::max(), true},
+        {lowint.str(), std::numeric_limits<int>::lowest() + 1, true},
+        {toobig.str(), -1, false},
+        {toosmall.str(), -1, false},
+        {"flowers", -1, false},
+        {"123.456", -1, false},
+        {"123-456", -1, false},
+        {"  123", -1, false},
+        {" 123 ", -1, false},
+        {"123  ", -1, false},
+    };
+    for (auto const& p : sets) {
+        ASSERT_EQ(bspi.setFromString(p.setstr).isOK(), p.succeed);
+        if (p.succeed) {
+            ASSERT_EQUALS(p.setint, getInt(bspi));
+            ASSERT_EQUALS(p.setint, bspi.getLocked());
+        }
+    }
+
+    const int seti[] = {
+        -1, 0, 1, std::numeric_limits<int>::lowest() + 1, std::numeric_limits<int>::max()};
+    for (auto const& p : seti) {
+        ASSERT_TRUE(bspi.setLocked(p).isOK());
+        ASSERT_EQUALS(p, getInt(bspi));
+        ASSERT_EQUALS(p, bspi.getLocked());
+    }
+}
+
 TEST(ServerParameters, Simple1) {
     AtomicInt32 f(5);
     ExportedServerParameter<int, ServerParameterType::kStartupAndRuntime> ff(NULL, "ff", &f);
     ASSERT_EQUALS("ff", ff.name());
 
-    ff.set(6).transitional_ignore();
+    ASSERT_TRUE(ff.set(6).isOK());
     ASSERT_EQUALS(6, f.load());
 
-    ff.set(BSON("x" << 7).firstElement()).transitional_ignore();
+    ASSERT_TRUE(ff.set(BSON("x" << 7).firstElement()).isOK());
     ASSERT_EQUALS(7, f.load());
 
-    ff.setFromString("8").transitional_ignore();
+    ASSERT_TRUE(ff.setFromString("8").isOK());
     ASSERT_EQUALS(8, f.load());
 }
 
@@ -63,7 +222,7 @@ TEST(ServerParameters, Vector1) {
     BSONObj x = BSON("x" << BSON_ARRAY("a"
                                        << "b"
                                        << "c"));
-    vv.set(x.firstElement()).transitional_ignore();
+    ASSERT_TRUE(vv.set(x.firstElement()).isOK());
 
     ASSERT_EQUALS(3U, v.size());
     ASSERT_EQUALS("a", v[0]);
@@ -79,9 +238,11 @@ TEST(ServerParameters, Vector1) {
     ASSERT(x.firstElement().woCompare(y.firstElement(), false) == 0);
 
 
-    vv.setFromString("d,e").transitional_ignore();
+    ASSERT_TRUE(vv.setFromString("d,e").isOK());
     ASSERT_EQUALS(2U, v.size());
     ASSERT_EQUALS("d", v[0]);
     ASSERT_EQUALS("e", v[1]);
 }
-}
+
+}  // namespace
+}  // namespace mongo