SERVER-59813 Simplify ServerParameter construction and add formal readonly parameters

13 files changed, 36 insertions, 89 deletions

diff --git a/buildscripts/idl/idl/binder.py b/buildscripts/idl/idl/binder.py
index da38cb176d5..d615f4a633b 100644
--- a/buildscripts/idl/idl/binder.py
+++ b/buildscripts/idl/idl/binder.py
@@ -1320,6 +1320,10 @@ def _bind_server_parameter_set_at(ctxt, param):
     # type: (errors.ParserContext, syntax.ServerParameter) -> str
     """Translate set_at options to C++ enum value."""
 
+    if param.set_at == ['readonly']:
+        # Readonly may not be mixed with startup or runtime
+        return "ServerParameterType::kReadOnly"
+
     set_at = 0
     for psa in param.set_at:
         if psa.lower() == 'startup':
diff --git a/src/mongo/db/auth/authorization_manager_global.cpp b/src/mongo/db/auth/authorization_manager_global.cpp
index b7a21b94fa5..b4655b8970e 100644
--- a/src/mongo/db/auth/authorization_manager_global.cpp
+++ b/src/mongo/db/auth/authorization_manager_global.cpp
@@ -88,11 +88,6 @@ ServiceContext::ConstructorActionRegisterer createAuthorizationManager(
 
 }  // namespace
 
-// This setting is unique in that it is read-only. The IDL subststem doesn't actually allow for
-// that, so we'll pretend it's startup-settable, then override it here.
-AuthzVersionParameter::AuthzVersionParameter(StringData name, ServerParameterType)
-    : ServerParameter(ServerParameterSet::getGlobal(), name, false, false) {}
-
 void AuthzVersionParameter::append(OperationContext* opCtx,
                                    BSONObjBuilder& b,
                                    const std::string& name) {
diff --git a/src/mongo/db/auth/authorization_manager_global_parameters.idl b/src/mongo/db/auth/authorization_manager_global_parameters.idl
index ba9cbdccc3c..081c27904b5 100644
--- a/src/mongo/db/auth/authorization_manager_global_parameters.idl
+++ b/src/mongo/db/auth/authorization_manager_global_parameters.idl
@@ -33,10 +33,9 @@ global:
 server_parameters:
     authSchemaVersion:
         description: 'Read-only value describing the current auth schema version'
-        set_at: startup # Actually, never.
+        set_at: 'readonly'
         cpp_class:
             name: AuthzVersionParameter
-            override_ctor: true
     startupAuthSchemaValidation:
         description: 'Validate auth schema on startup'
         set_at: startup
diff --git a/src/mongo/db/commands/feature_compatibility_version.cpp b/src/mongo/db/commands/feature_compatibility_version.cpp
index 92d73be1425..906cdf7923c 100644
--- a/src/mongo/db/commands/feature_compatibility_version.cpp
+++ b/src/mongo/db/commands/feature_compatibility_version.cpp
@@ -521,15 +521,6 @@ void FeatureCompatibilityVersion::clearLastFCVUpdateTimestamp() {
 }
 
 
-/**
- * Read-only server parameter for featureCompatibilityVersion.
- */
-// No ability to specify 'none' as set_at type,
-// so use 'startup' in the IDL file, then override to none here.
-FeatureCompatibilityVersionParameter::FeatureCompatibilityVersionParameter(StringData name,
-                                                                           ServerParameterType)
-    : ServerParameter(ServerParameterSet::getGlobal(), name, false, false) {}
-
 void FeatureCompatibilityVersionParameter::append(OperationContext* opCtx,
                                                   BSONObjBuilder& b,
                                                   const std::string& name) {
diff --git a/src/mongo/db/commands/feature_compatibility_version.idl b/src/mongo/db/commands/feature_compatibility_version.idl
index 881e7a25f73..1161730d965 100644
--- a/src/mongo/db/commands/feature_compatibility_version.idl
+++ b/src/mongo/db/commands/feature_compatibility_version.idl
@@ -32,11 +32,9 @@ global:
 server_parameters:
     featureCompatibilityVersion:
         description: 'Read-only view of current Feature Compatability Version'
-        # Actually, never.
-        set_at: startup
+        set_at: 'readonly'
         cpp_class:
             name: FeatureCompatibilityVersionParameter
-            override_ctor: true
     internalValidateFeaturesAsPrimary:
         deprecated_name: "internalValidateFeaturesAsMaster"
         description: >
diff --git a/src/mongo/db/storage/storage_options.cpp b/src/mongo/db/storage/storage_options.cpp
index 3da32b91f19..5c1b08c6037 100644
--- a/src/mongo/db/storage/storage_options.cpp
+++ b/src/mongo/db/storage/storage_options.cpp
@@ -64,14 +64,6 @@ void StorageGlobalParams::reset() {
 
 StorageGlobalParams storageGlobalParams;
 
-// Storage global parameters exported read-only via the getParameter mechanism.
-// The IDL has no ability to specify 'none' as set_at type,
-// so use 'startup' in the IDL file, then override to none here.
-StorageDirectoryPerDbParameter::StorageDirectoryPerDbParameter(StringData name, ServerParameterType)
-    : ServerParameter(
-          ServerParameterSet::getGlobal(), name, false /* allowedToChangeAtStartup */, false
-          /* allowedToChangeAtRuntime */) {}
-
 Status StorageDirectoryPerDbParameter::setFromString(const std::string&) {
     return {ErrorCodes::IllegalOperation,
             str::stream() << name() << " cannot be set via setParameter"};
diff --git a/src/mongo/db/storage/storage_parameters.idl b/src/mongo/db/storage/storage_parameters.idl
index 2f59f49ea3f..ae048332794 100644
--- a/src/mongo/db/storage/storage_parameters.idl
+++ b/src/mongo/db/storage/storage_parameters.idl
@@ -90,11 +90,9 @@ server_parameters:
 
     storageGlobalParams.directoryperdb:
         description: 'Read-only view of directory per db config parameter'
-        # Actually, never.  TODO(SERVER-59813): Use correct IDL for this once support is added.
-        set_at: startup
+        set_at: 'readonly'
         cpp_class:
             name: StorageDirectoryPerDbParameter
-            override_ctor: true
 
     timeseriesValidateCompressedBuckets:
         description: 'Validate that there is no data loss before committing compressed timeseries buckets'
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.cpp
index df10ee2fa96..1e6f7816f65 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.cpp
@@ -78,15 +78,6 @@ Status WiredTigerEngineRuntimeConfigParameter::setFromString(const std::string&
     return Status::OK();
 }
 
-// WiredTiger parameter exported read-only via the getParameter mechanism.
-// The IDL has no ability to specify 'none' as set_at type,
-// so use 'startup' in the IDL file, then override to none here.
-WiredTigerDirectoryForIndexesParameter::WiredTigerDirectoryForIndexesParameter(StringData name,
-                                                                               ServerParameterType)
-    : ServerParameter(
-          ServerParameterSet::getGlobal(), name, false /* allowedToChangeAtStartup */, false
-          /* allowedToChangeAtRuntime */) {}
-
 Status WiredTigerDirectoryForIndexesParameter::setFromString(const std::string&) {
     return {ErrorCodes::IllegalOperation,
             str::stream() << name() << " cannot be set via setParameter"};
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.idl b/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.idl
index fececcda86b..1dc776ffefe 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.idl
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_parameters.idl
@@ -186,11 +186,9 @@ server_parameters:
 
     wiredTigerDirectoryForIndexes:
        description: 'Read-only view of DirectoryForIndexes config parameter'
-       # Actually, never.  TODO(SERVER-59813): Use correct IDL for this once support is added.
-       set_at: startup
+       set_at: 'readonly'
        cpp_class:
            name: WiredTigerDirectoryForIndexesParameter
-           override_ctor: true
 
     wiredTigerSkipTableLoggingChecksOnStartup:
       description: >-
diff --git a/src/mongo/idl/feature_flag.cpp b/src/mongo/idl/feature_flag.cpp
index c058c108d46..a6c7a44c994 100644
--- a/src/mongo/idl/feature_flag.cpp
+++ b/src/mongo/idl/feature_flag.cpp
@@ -87,7 +87,7 @@ void FeatureFlag::set(bool enabled) {
 }
 
 FeatureFlagServerParameter::FeatureFlagServerParameter(StringData name, FeatureFlag& storage)
-    : ServerParameter(ServerParameterSet::getGlobal(), name, true, false), _storage(storage) {}
+    : ServerParameter(name, ServerParameterType::kStartupOnly), _storage(storage) {}
 
 void FeatureFlagServerParameter::append(OperationContext* opCtx,
                                         BSONObjBuilder& b,
diff --git a/src/mongo/idl/server_parameter.cpp b/src/mongo/idl/server_parameter.cpp
index 05db8b9c856..e15bd17f483 100644
--- a/src/mongo/idl/server_parameter.cpp
+++ b/src/mongo/idl/server_parameter.cpp
@@ -41,29 +41,12 @@ MONGO_INITIALIZER_GROUP(EndServerParameterRegistration,
                         ("BeginServerParameterRegistration"),
                         ("BeginStartupOptionHandling"))
 
-ServerParameter::ServerParameter(StringData name, ServerParameterType spt)
-    : ServerParameter(ServerParameterSet::getGlobal(),
-                      name,
-                      spt != SPT::kRuntimeOnly,
-                      spt != SPT::kStartupOnly) {}
-
-ServerParameter::ServerParameter(ServerParameterSet* sps,
-                                 StringData name,
-                                 bool allowedToChangeAtStartup,
-                                 bool allowedToChangeAtRuntime)
-    : _name(name.toString()),
-      _allowedToChangeAtStartup(allowedToChangeAtStartup),
-      _allowedToChangeAtRuntime(allowedToChangeAtRuntime) {
-    if (sps) {
-        sps->add(this);
-    }
-}
+ServerParameter::ServerParameter(StringData name, ServerParameterType spt, NoRegistrationTag)
+    : _name(name.toString()), _type(spt) {}
 
-ServerParameter::ServerParameter(ServerParameterSet* sps, StringData name)
-    : _name(name.toString()), _allowedToChangeAtStartup(true), _allowedToChangeAtRuntime(true) {
-    if (sps) {
-        sps->add(this);
-    }
+ServerParameter::ServerParameter(StringData name, ServerParameterType spt)
+    : ServerParameter(name, spt, NoRegistrationTag{}) {
+    ServerParameterSet::getGlobal()->add(this);
 }
 
 namespace {
@@ -119,11 +102,7 @@ void ServerParameterSet::remove(const std::string& name) {
 
 IDLServerParameterDeprecatedAlias::IDLServerParameterDeprecatedAlias(StringData name,
                                                                      ServerParameter* sp)
-    : ServerParameter(ServerParameterSet::getGlobal(),
-                      name,
-                      sp->allowedToChangeAtStartup(),
-                      sp->allowedToChangeAtRuntime()),
-      _sp(sp) {
+    : ServerParameter(name, sp->getServerParameterType()), _sp(sp) {
     if (_sp->isTestOnly()) {
         setTestOnly();
     }
@@ -173,9 +152,7 @@ public:
     DisabledTestParameter() = delete;
 
     DisabledTestParameter(ServerParameter* sp)
-        : ServerParameter(
-              nullptr, sp->name(), sp->allowedToChangeAtStartup(), sp->allowedToChangeAtRuntime()),
-          _sp(sp) {
+        : ServerParameter(sp->name(), sp->getServerParameterType(), NoRegistrationTag{}), _sp(sp) {
         setTestOnly();
     }
 
diff --git a/src/mongo/idl/server_parameter.h b/src/mongo/idl/server_parameter.h
index 276f542837a..8d8705071f6 100644
--- a/src/mongo/idl/server_parameter.h
+++ b/src/mongo/idl/server_parameter.h
@@ -56,6 +56,10 @@ namespace mongo {
  * At runtime, { setParameter : 1, ...} is used.
  */
 enum class ServerParameterType {
+    /**
+     * May not be set at any time.
+     */
+    kReadOnly,
 
     /**
      * Parameter can only be set via runCommand.
@@ -82,11 +86,6 @@ public:
     using Map = std::map<std::string, ServerParameter*>;
 
     ServerParameter(StringData name, ServerParameterType spt);
-    ServerParameter(ServerParameterSet* sps,
-                    StringData name,
-                    bool allowedToChangeAtStartup,
-                    bool allowedToChangeAtRuntime);
-    ServerParameter(ServerParameterSet* sps, StringData name);
     virtual ~ServerParameter() = default;
 
     std::string name() const {
@@ -97,16 +96,21 @@ public:
      * @return if you can set on command line or config file
      */
     bool allowedToChangeAtStartup() const {
-        return _allowedToChangeAtStartup;
+        return (_type == ServerParameterType::kStartupOnly) ||
+            (_type == ServerParameterType::kStartupAndRuntime);
     }
 
     /**
      * @param if you can use (get|set)Parameter
      */
     bool allowedToChangeAtRuntime() const {
-        return _allowedToChangeAtRuntime;
+        return (_type == ServerParameterType::kRuntimeOnly) ||
+            (_type == ServerParameterType::kStartupAndRuntime);
     }
 
+    ServerParameterType getServerParameterType() const {
+        return _type;
+    }
 
     virtual void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) = 0;
 
@@ -132,10 +136,13 @@ protected:
     // Helper for translating setParameter values from BSON to string.
     StatusWith<std::string> coerceToString(const BSONElement&, bool redact);
 
+    // Used by DisabledTestParameter to avoid re-registering the server parameter.
+    struct NoRegistrationTag {};
+    ServerParameter(StringData name, ServerParameterType spt, NoRegistrationTag);
+
 private:
     std::string _name;
-    bool _allowedToChangeAtStartup;
-    bool _allowedToChangeAtRuntime;
+    ServerParameterType _type;
     bool _testOnly = false;
 };
 
diff --git a/src/mongo/idl/server_parameter_with_storage.h b/src/mongo/idl/server_parameter_with_storage.h
index 1871aa6156a..603c04c50a5 100644
--- a/src/mongo/idl/server_parameter_with_storage.h
+++ b/src/mongo/idl/server_parameter_with_storage.h
@@ -190,13 +190,10 @@ public:
     using element_type = typename SW::type;
 
     IDLServerParameterWithStorage(StringData name, T& storage)
-        : ServerParameter(ServerParameterSet::getGlobal(),
-                          name,
-                          paramType == SPT::kStartupOnly || paramType == SPT::kStartupAndRuntime,
-                          paramType == SPT::kRuntimeOnly || paramType == SPT::kStartupAndRuntime),
-          _storage(storage) {
-        static_assert(thread_safe || paramType == SPT::kStartupOnly,
-                      "Runtime server parameters must be thread safe");
+        : ServerParameter(name, paramType), _storage(storage) {
+        constexpr bool notRuntime =
+            (paramType == SPT::kStartupOnly) || (paramType == SPT::kReadOnly);
+        static_assert(thread_safe || notRuntime, "Runtime server parameters must be thread safe");
     }
 
     /**