SERVER-10515 Convert special cased parameters to proper ServerParameters

4 files changed, 130 insertions, 53 deletions

diff --git a/jstests/noPassthrough/parameters.js b/jstests/noPassthrough/parameters.js
new file mode 100644
index 00000000000..5fb74e20f2c
--- /dev/null
+++ b/jstests/noPassthrough/parameters.js
@@ -0,0 +1,64 @@
+var dbConn = MongoRunner.runMongod();
+
+function setAndCheckParameter(dbConn, parameterName, newValue, expectedResult) {
+    jsTest.log("Test setting parameter: " + parameterName + " to value: " + newValue);
+    var getParameterCommand = { getParameter : 1 };
+    getParameterCommand[parameterName] = 1;
+    var ret = dbConn.adminCommand(getParameterCommand);
+    assert.eq(ret.ok, 1, tojson(ret));
+    oldValue = ret[parameterName];
+
+    var setParameterCommand = { setParameter : 1 };
+    setParameterCommand[parameterName] = newValue;
+    var ret = dbConn.adminCommand(setParameterCommand);
+    assert.eq(ret.ok, 1, tojson(ret));
+    assert.eq(ret.was, oldValue, tojson(ret));
+
+    var ret = dbConn.adminCommand(getParameterCommand);
+    assert.eq(ret.ok, 1, tojson(ret));
+    // If we have explicitly set an "exptectedResult", use that, else use "newValue".  This is for
+    // cases where the server does some type coersion that changes the value.
+    if (typeof expectedResult === "undefined") {
+        assert.eq(ret[parameterName], newValue, tojson(ret));
+    }
+    else {
+        assert.eq(ret[parameterName], expectedResult, tojson(ret));
+    }
+    return newValue;
+}
+
+setAndCheckParameter(dbConn, "logLevel", 1);
+setAndCheckParameter(dbConn, "logLevel", 1.5, 1);
+setAndCheckParameter(dbConn, "journalCommitInterval", 100);
+setAndCheckParameter(dbConn, "traceExceptions", true);
+setAndCheckParameter(dbConn, "traceExceptions", false);
+setAndCheckParameter(dbConn, "traceExceptions", 1, true);
+setAndCheckParameter(dbConn, "traceExceptions", 0, false);
+setAndCheckParameter(dbConn, "traceExceptions", "foo", true);
+setAndCheckParameter(dbConn, "traceExceptions", "", true);
+setAndCheckParameter(dbConn, "replMonitorMaxFailedChecks", 30);
+setAndCheckParameter(dbConn, "replMonitorMaxFailedChecks", 30.5, 30);
+setAndCheckParameter(dbConn, "replMonitorMaxFailedChecks", -30);
+
+function ensureSetParameterFailure(dbConn, parameterName, newValue) {
+    jsTest.log("Test setting parameter: " + parameterName + " to invalid value: " + newValue);
+    var setParameterCommand = { setParameter : 1 };
+    setParameterCommand[parameterName] = newValue;
+    var ret = dbConn.adminCommand(setParameterCommand);
+    assert.eq(ret.ok, 0, tojson(ret));
+    printjson(ret);
+}
+
+ensureSetParameterFailure(dbConn, "logLevel", "foo");
+ensureSetParameterFailure(dbConn, "logLevel", "1.5");
+ensureSetParameterFailure(dbConn, "logLevel", -1);
+ensureSetParameterFailure(dbConn, "journalCommitInterval", "foo");
+ensureSetParameterFailure(dbConn, "journalCommitInterval", "1.5");
+ensureSetParameterFailure(dbConn, "journalCommitInterval", 1.5);
+ensureSetParameterFailure(dbConn, "journalCommitInterval", 1000);
+ensureSetParameterFailure(dbConn, "journalCommitInterval", 1);
+ensureSetParameterFailure(dbConn, "replMonitorMaxFailedChecks", "foo");
+
+MongoRunner.stopMongod(dbConn.port);
+
+jsTest.log("noPassthrough_parameters_test succeeded!");
diff --git a/src/mongo/db/commands/parameters.cpp b/src/mongo/db/commands/parameters.cpp
index a600be68787..05e584a8287 100644
--- a/src/mongo/db/commands/parameters.cpp
+++ b/src/mongo/db/commands/parameters.cpp
@@ -77,22 +77,6 @@ namespace mongo {
 
             int before = result.len();
 
-            // TODO: convert to ServerParameters -- SERVER-10515
-
-            if (isJournalingEnabled() && (all || cmdObj.hasElement("journalCommitInterval")) &&
-                !isMongos()) {
-                result.append("journalCommitInterval",
-                              getJournalCommitInterval());
-            }
-            if( all || cmdObj.hasElement( "traceExceptions" ) ) {
-                result.append("traceExceptions",
-                              DBException::traceExceptions);
-            }
-            if( all || cmdObj.hasElement( "replMonitorMaxFailedChecks" ) ) {
-                result.append("replMonitorMaxFailedChecks",
-                              ReplicaSetMonitor::maxConsecutiveFailedChecks);
-            }
-
             const ServerParameter::Map& m = ServerParameterSet::getGlobal()->getMap();
             for ( ServerParameter::Map::const_iterator i = m.begin(); i != m.end(); ++i ) {
                 if ( all || cmdObj.hasElement( i->first.c_str() ) ) {
@@ -130,35 +114,6 @@ namespace mongo {
             int s = 0;
             bool found = false;
 
-            // TODO: convert to ServerParameters -- SERVER-10515
-
-            if( cmdObj.hasElement("journalCommitInterval") ) {
-                if (isMongos()) {
-                    errmsg = "cannot set journalCommitInterval on a mongos";
-                    return false;
-                }
-                if(!isJournalingEnabled()) {
-                    errmsg = "journaling is off";
-                    return false;
-                }
-                int x = (int) cmdObj["journalCommitInterval"].Number();
-                verify( x > 1 && x < 500 );
-                setJournalCommitInterval(x);
-                log() << "setParameter journalCommitInterval=" << x << endl;
-                s++;
-            }
-            if( cmdObj.hasElement( "traceExceptions" ) ) {
-                if( s == 0 ) result.append( "was", DBException::traceExceptions );
-                DBException::traceExceptions = cmdObj["traceExceptions"].Bool();
-                s++;
-            }
-            if( cmdObj.hasElement( "replMonitorMaxFailedChecks" ) ) {
-                if( s == 0 ) result.append( "was", ReplicaSetMonitor::maxConsecutiveFailedChecks );
-                ReplicaSetMonitor::maxConsecutiveFailedChecks =
-                    cmdObj["replMonitorMaxFailedChecks"].numberInt();
-                s++;
-            }
-
             const ServerParameter::Map& m = ServerParameterSet::getGlobal()->getMap();
             BSONObjIterator i( cmdObj );
             i.next(); // skip past command name
@@ -394,6 +349,19 @@ namespace mongo {
                                                     &serverGlobalParams.quiet,
                                                     true,
                                                     true );
+
+        ExportedServerParameter<int> MaxConsecutiveFailedChecksSetting(
+                                                    ServerParameterSet::getGlobal(),
+                                                    "replMonitorMaxFailedChecks",
+                                                    &ReplicaSetMonitor::maxConsecutiveFailedChecks,
+                                                    false, // allowedToChangeAtStartup
+                                                    true); // allowedToChangeAtRuntime
+
+        ExportedServerParameter<bool> TraceExceptionsSetting(ServerParameterSet::getGlobal(),
+                                                             "traceExceptions",
+                                                             &DBException::traceExceptions,
+                                                             false, // allowedToChangeAtStartup
+                                                             true); // allowedToChangeAtRuntime
     }
 
 }
diff --git a/src/mongo/db/storage_options.cpp b/src/mongo/db/storage_options.cpp
index 23516baf85d..2d99896aef1 100644
--- a/src/mongo/db/storage_options.cpp
+++ b/src/mongo/db/storage_options.cpp
@@ -39,13 +39,60 @@ namespace mongo {
         return storageGlobalParams.dur;
     }
 
-    void setJournalCommitInterval(unsigned newValue) {
-        storageGlobalParams.journalCommitInterval = newValue;
-    }
+    class JournalCommitIntervalSetting : public ServerParameter {
+    public:
+        JournalCommitIntervalSetting() :
+            ServerParameter(ServerParameterSet::getGlobal(), "journalCommitInterval",
+                    false, // allowedToChangeAtStartup
+                    true // allowedToChangeAtRuntime
+                    ) {}
 
-    unsigned getJournalCommitInterval() {
-        return storageGlobalParams.journalCommitInterval;
-    }
+        virtual void append(OperationContext* txn, BSONObjBuilder& b, const std::string& name) {
+            b << name << storageGlobalParams.journalCommitInterval;
+        }
+
+        virtual Status set(const BSONElement& newValueElement) {
+            long long newValue;
+            if (!newValueElement.isNumber()) {
+                StringBuilder sb;
+                sb << "Expected number type for journalCommitInterval via setParameter command: "
+                   << newValueElement;
+                return Status(ErrorCodes::BadValue, sb.str());
+            }
+            if (newValueElement.type() == NumberDouble &&
+                (newValueElement.numberDouble() - newValueElement.numberLong()) > 0) {
+                StringBuilder sb;
+                sb << "journalCommitInterval must be a whole number: "
+                   << newValueElement;
+                return Status(ErrorCodes::BadValue, sb.str());
+            }
+            newValue = newValueElement.numberLong();
+            if (newValue <= 1 || newValue >= 500) {
+                StringBuilder sb;
+                sb << "journalCommitInterval must be between 1 and 500, but attempted to set to: "
+                   << newValue;
+                return Status(ErrorCodes::BadValue, sb.str());
+            }
+            storageGlobalParams.journalCommitInterval = static_cast<unsigned>(newValue);
+            return Status::OK();
+        }
+
+        virtual Status setFromString(const std::string& str) {
+            unsigned newValue;
+            Status status = parseNumberFromString(str, &newValue);
+            if (!status.isOK()) {
+                return status;
+            }
+            if (newValue <= 1 || newValue >= 500) {
+                StringBuilder sb;
+                sb << "journalCommitInterval must be between 1 and 500, but attempted to set to: "
+                   << newValue;
+                return Status(ErrorCodes::BadValue, sb.str());
+            }
+            storageGlobalParams.journalCommitInterval = newValue;
+            return Status::OK();
+        }
+    } journalCommitIntervalSetting;
 
     ExportedServerParameter<bool> NoTableScanSetting(ServerParameterSet::getGlobal(),
                                                      "notablescan",
diff --git a/src/mongo/db/storage_options.h b/src/mongo/db/storage_options.h
index 4b02468aa05..4298be859d5 100644
--- a/src/mongo/db/storage_options.h
+++ b/src/mongo/db/storage_options.h
@@ -107,8 +107,6 @@ namespace mongo {
     extern StorageGlobalParams storageGlobalParams;
 
     bool isJournalingEnabled();
-    void setJournalCommitInterval(unsigned newValue);
-    unsigned getJournalCommitInterval();
 
     // This is not really related to persistence, but mongos and the other executables share code
     // and we use this function to determine at runtime which executable we are in.