Create new ConfigKnobOverrides and ManualKnobOverrides classes

fdbserver/ConfigFollowerInterface.cpp

@@ -45,10 +45,12 @@ bool ConfigFollowerInterface::operator!=(ConfigFollowerInterface const& rhs) con
 	return !(*this == rhs);
 }
 
-ConfigClassSet ConfigClassSet::fromParamString(std::string const& paramString) {
+ConfigClassSet::ConfigClassSet() = default;
-	// TODO: Validate input and implement
+
-	ConfigClassSet result;
+ConfigClassSet::ConfigClassSet(VectorRef<KeyRef> configClasses) {
-	return result;
+	for (const auto& configClass : configClasses) {
+		classes.insert(configClass);
+	}
 }
 
 bool ConfigClassSet::contains(KeyRef configClass) const {

fdbserver/ConfigFollowerInterface.h

@@ -27,14 +27,14 @@
 
 class ConfigClassSet {
 	std::set<Key> classes;
-
 public:
 	static constexpr FileIdentifier file_identifier = 9854021;
 
 	bool operator==(ConfigClassSet const& rhs) const { return classes == rhs.classes; }
 	bool operator!=(ConfigClassSet const& rhs) const { return !(*this == rhs); }
 
-	static ConfigClassSet fromParamString(std::string const& paramString);
+	ConfigClassSet();
+	ConfigClassSet(VectorRef<KeyRef> configClasses);
 
 	bool contains(KeyRef configClass) const;
 

fdbserver/LocalConfiguration.actor.cpp

@@ -27,32 +27,109 @@
 
 namespace {
 
-const KeyRef configClassesKey = "configClasses"_sr;
+const KeyRef configPathKey = "configPath"_sr;
 const KeyRef lastSeenVersionKey = "lastSeenVersion"_sr;
 const KeyRangeRef knobOverrideKeys = KeyRangeRef("knobOverride/"_sr, "knobOverride0"_sr);
 
+bool updateSingleKnob(Key knobName, Value knobValue) {
+	return false;
+}
+
+template <class K, class... Rest>
+bool updateSingleKnob(Key knobName, Value knobValue, K& k, Rest&... rest) {
+	if (k.setKnob(knobName.toString(), knobValue.toString())) {
+		return true;
+	} else {
+		return updateSingleKnob(knobName, knobValue, rest...);
+	}
+}
+
+class ConfigKnobOverrides {
+	Standalone<VectorRef<KeyRef>> configPath;
+	std::map<Key, std::map<Key, Value>> configClassToKnobToValue;
+
+public:
+	ConfigKnobOverrides() = default;
+	ConfigKnobOverrides(std::string const& paramString) {
+		// TODO: Validate string
+		// FIXME: Fix this implementation
+		/*
+		auto b = paramString.begin();
+		while (b != paramString.end()) {
+		    auto e = std::find(b, paramString.end(), '/');
+		    configPath.emplace_back_deep(configPath.arena(), reinterpret_cast<uint8_t const *>(&(*b)), e-b);
+		    if (e != paramString.end()) {
+		        b = e+1;
+		    }
+		}
+		*/
+	}
+	ConfigClassSet getConfigClassSet() const { return ConfigClassSet(configPath); }
+	void set(KeyRef configClass, KeyRef knobName, ValueRef value) {
+		configClassToKnobToValue[configClass][knobName] = value;
+	}
+	void remove(KeyRef configClass, KeyRef knobName) { configClassToKnobToValue[configClass].erase(knobName); }
+
+	template <class... KS>
+	void update(KS&... knobCollections) const {
+		for (const auto& configClass : configPath) {
+			const auto& knobToValue = configClassToKnobToValue.find(configClass);
+			if (knobToValue != configClassToKnobToValue.end()) {
+				for (const auto& [knobName, knobValue] : knobToValue->second) {
+					// Assert here because we should be validating on the client
+					ASSERT(updateSingleKnob(knobName, knobValue, knobCollections...));
+				}
+			}
+		}
+	}
+
+	bool hasSameConfigPath(ConfigKnobOverrides const& other) const { return configPath == other.configPath; }
+
+	template <class Ar>
+	void serialize(Ar& ar) {
+		serializer(ar, configPath);
+	}
+};
+
+class ManualKnobOverrides {
+	std::map<Key, Value> overrides;
+
+public:
+	ManualKnobOverrides(std::map<Key, Value>&& overrides) : overrides(std::move(overrides)) {}
+
+	template <class... KS>
+	void update(KS&... knobCollections) const {
+		for (const auto& [knobName, knobValue] : overrides) {
+			if (!updateSingleKnob(knobName, knobValue, knobCollections...)) {
+				fprintf(stderr, "WARNING: Unrecognized knob option '%s'\n", knobName.toString().c_str());
+				TraceEvent(SevWarnAlways, "UnrecognizedKnobOption").detail("Knob", printable(knobName));
+			}
+		}
+	}
+};
+
 } // namespace
 
 class LocalConfigurationImpl {
 	IKeyValueStore* kvStore; // FIXME: fix leaks?
-	ConfigClassSet configClasses;
 	Version lastSeenVersion { 0 };
 	Future<Void> initFuture;
 	FlowKnobs flowKnobs;
 	ClientKnobs clientKnobs;
 	ServerKnobs serverKnobs;
 	TestKnobs testKnobs;
-	std::map<Key, Value> manuallyOverriddenKnobs;
+	ManualKnobOverrides manualKnobOverrides;
-	std::map<ConfigKey, Value> configDatabaseOverriddenKnobs;
+	ConfigKnobOverrides configKnobOverrides;
 
-	ACTOR static Future<Void> saveConfigClasses(LocalConfigurationImpl* self) {
+	ACTOR static Future<Void> saveConfigPath(LocalConfigurationImpl* self) {
-		self->kvStore->set(KeyValueRef(configClassesKey, BinaryWriter::toValue(self->configClasses, IncludeVersion())));
+		self->kvStore->set(
+		    KeyValueRef(configPathKey, BinaryWriter::toValue(self->configKnobOverrides, IncludeVersion())));
 		wait(self->kvStore->commit());
 		return Void();
 	}
 
 	ACTOR static Future<Void> clearKVStore(LocalConfigurationImpl *self) {
-		self->kvStore->clear(singleKeyRange(configClassesKey));
+		self->kvStore->clear(singleKeyRange(configPathKey));
 		self->kvStore->clear(knobOverrideKeys);
 		wait(self->kvStore->commit());
 		return Void();
@@ -73,36 +150,30 @@ class LocalConfigurationImpl {
 	ACTOR static Future<Void> init(LocalConfigurationImpl* self) {
 		wait(self->kvStore->init());
 		wait(getLastSeenVersion(self));
-		state Optional<Value> storedConfigClassesValue = wait(self->kvStore->readValue(configClassesKey));
+		state Optional<Value> storedConfigPathValue = wait(self->kvStore->readValue(configPathKey));
-		if (!storedConfigClassesValue.present()) {
+		if (!storedConfigPathValue.present()) {
-			wait(saveConfigClasses(self));
+			wait(saveConfigPath(self));
 			self->updateInMemoryKnobs();
 			return Void();
 		}
-		state ConfigClassSet storedConfigClasses = BinaryReader::fromStringRef<ConfigClassSet>(storedConfigClassesValue.get(), IncludeVersion());
+		state ConfigKnobOverrides storedConfigPath =
-		if (storedConfigClasses != self->configClasses) {
+		    BinaryReader::fromStringRef<ConfigKnobOverrides>(storedConfigPathValue.get(), IncludeVersion());
+		if (!storedConfigPath.hasSameConfigPath(self->configKnobOverrides)) {
 			// All local information is outdated
 			wait(clearKVStore(self));
-			wait(saveConfigClasses(self));
+			wait(saveConfigPath(self));
 			self->updateInMemoryKnobs();
 			return Void();
 		}
 		Standalone<RangeResultRef> range = wait(self->kvStore->readRange(knobOverrideKeys));
 		for (const auto &kv : range) {
 			auto configKey = BinaryReader::fromStringRef<ConfigKey>(kv.key, IncludeVersion());
-			self->configDatabaseOverriddenKnobs[configKey] = kv.value;
+			self->configKnobOverrides.set(configKey.configClass, configKey.knobName, kv.value);
 		}
 		self->updateInMemoryKnobs();
 		return Void();
 	}
 
-	bool updateKnob(Key knobName, Value knobValue) {
-		return (flowKnobs.setKnob(knobName.toString(), knobValue.toString()) ||
-		        clientKnobs.setKnob(knobName.toString(), knobValue.toString()) ||
-		        serverKnobs.setKnob(knobName.toString(), knobValue.toString()) ||
-		        testKnobs.setKnob(knobName.toString(), knobValue.toString()));
-	}
-
 	void initializeKnobs() {
 		flowKnobs.initialize();
 		clientKnobs.initialize();
@@ -112,24 +183,15 @@ class LocalConfigurationImpl {
 
 	void resetKnobs() {
 		flowKnobs.reset();
-		clientKnobs.initialize();
+		clientKnobs.reset();
-		serverKnobs.initialize();
+		serverKnobs.reset();
-		testKnobs.initialize();
+		testKnobs.reset();
 	}
 
 	void updateInMemoryKnobs() {
 		resetKnobs();
-		for (const auto& [configKey, knobValue] : configDatabaseOverriddenKnobs) {
+		configKnobOverrides.update(flowKnobs, clientKnobs, serverKnobs, testKnobs);
-			// TODO: Order updates by config class
+		manualKnobOverrides.update(flowKnobs, clientKnobs, serverKnobs, testKnobs);
-			// Assert here because we should be validating on the client
-			ASSERT(updateKnob(configKey.knobName, knobValue));
-		}
-		for (const auto& [knobName, knobValue] : manuallyOverriddenKnobs) {
-			if (!updateKnob(knobName, knobValue)) {
-				fprintf(stderr, "WARNING: Unrecognized knob option '%s'\n", knobName.toString().c_str());
-				TraceEvent(SevWarnAlways, "UnrecognizedKnobOption").detail("Knob", printable(knobName));
-			}
-		}
 		// Must reinitialize in order to update dependent knobs
 		initializeKnobs();
 	}
@@ -137,7 +199,7 @@ class LocalConfigurationImpl {
 	ACTOR static Future<Void> applyKnobUpdates(LocalConfigurationImpl *self, ConfigFollowerGetFullDatabaseReply reply) {
 		self->kvStore->clear(knobOverrideKeys);
 		for (const auto& [configKey, knobValue] : reply.database) {
-			self->configDatabaseOverriddenKnobs[configKey] = knobValue;
+			self->configKnobOverrides.set(configKey.configClass, configKey.knobName, knobValue);
 		}
 		self->kvStore->set(KeyValueRef(lastSeenVersionKey, BinaryWriter::toValue(self->lastSeenVersion, IncludeVersion())));
 		wait(self->kvStore->commit());
@@ -151,10 +213,10 @@ class LocalConfigurationImpl {
 			auto serializedKey = BinaryWriter::toValue(mutation.getKey(), IncludeVersion());
 			if (mutation.isSet()) {
 				self->kvStore->set(KeyValueRef(serializedKey.withPrefix(knobOverrideKeys.begin), mutation.getValue()));
-				self->configDatabaseOverriddenKnobs[mutation.getKey()] = mutation.getValue();
+				self->configKnobOverrides.set(mutation.getConfigClass(), mutation.getKnobName(), mutation.getValue());
 			} else {
 				self->kvStore->clear(singleKeyRange(serializedKey.withPrefix(knobOverrideKeys.begin)));
-				self->configDatabaseOverriddenKnobs.erase(mutation.getKey());
+				self->configKnobOverrides.remove(mutation.getConfigClass(), mutation.getKnobName());
 			}
 		}
 		self->lastSeenVersion = reply.mostRecentVersion;
@@ -166,14 +228,18 @@ class LocalConfigurationImpl {
 
 	ACTOR static Future<Void> fetchChanges(LocalConfigurationImpl *self, ConfigFollowerInterface broadcaster) {
 		try {
-			ConfigFollowerGetChangesReply changesReply = wait(broadcaster.getChanges.getReply(ConfigFollowerGetChangesRequest{ self->lastSeenVersion, self->configClasses }));
+			ConfigFollowerGetChangesReply changesReply =
+			    wait(broadcaster.getChanges.getReply(ConfigFollowerGetChangesRequest{
+			        self->lastSeenVersion, self->configKnobOverrides.getConfigClassSet() }));
 			// TODO: Avoid applying if there are no updates
 			wait(applyKnobUpdates(self, changesReply));
 		} catch (Error &e) {
 			if (e.code() == error_code_version_already_compacted) {
 				ConfigFollowerGetVersionReply versionReply = wait(broadcaster.getVersion.getReply(ConfigFollowerGetVersionRequest{}));
 				self->lastSeenVersion = versionReply.version;
-				ConfigFollowerGetFullDatabaseReply fullDBReply = wait(broadcaster.getFullDatabase.getReply(ConfigFollowerGetFullDatabaseRequest{ self->lastSeenVersion, self->configClasses}));
+				ConfigFollowerGetFullDatabaseReply fullDBReply =
+				    wait(broadcaster.getFullDatabase.getReply(ConfigFollowerGetFullDatabaseRequest{
+				        self->lastSeenVersion, self->configKnobOverrides.getConfigClassSet() }));
 				// TODO: Avoid applying if there are no updates
 				wait(applyKnobUpdates(self, fullDBReply));
 			} else {
@@ -210,11 +276,11 @@ class LocalConfigurationImpl {
 	}
 
 public:
-	LocalConfigurationImpl(ConfigClassSet const& configClasses,
+	LocalConfigurationImpl(std::string const& configPath,
 	                       std::string const& dataFolder,
-	                       std::map<Key, Value>&& manuallyOverriddenKnobs,
+	                       std::map<Key, Value>&& manualKnobOverrides,
 	                       UID id)
-	  : configClasses(configClasses), manuallyOverriddenKnobs(std::move(manuallyOverriddenKnobs)) {
+	  : configKnobOverrides(configPath), manualKnobOverrides(std::move(manualKnobOverrides)) {
 		platform::createDirectory(dataFolder);
 		kvStore = keyValueStoreMemory(joinPath(dataFolder, "localconf-" + id.toString()), id, 500e6);
 	}
@@ -250,11 +316,11 @@ public:
 	}
 };
 
-LocalConfiguration::LocalConfiguration(ConfigClassSet const& configClasses,
+LocalConfiguration::LocalConfiguration(std::string const& configPath,
                                        std::string const& dataFolder,
-                                       std::map<Key, Value>&& manuallyOverriddenKnobs,
+                                       std::map<Key, Value>&& manualKnobOverrides,
                                        UID id)
-  : impl(std::make_unique<LocalConfigurationImpl>(configClasses, dataFolder, std::move(manuallyOverriddenKnobs), id)) {}
+  : impl(std::make_unique<LocalConfigurationImpl>(configPath, dataFolder, std::move(manualKnobOverrides), id)) {}
 
 LocalConfiguration::~LocalConfiguration() = default;
 
@@ -287,3 +353,8 @@ Future<Void> LocalConfiguration::consume(Reference<AsyncVar<ServerDBInfo> const>
 void TestKnobs::initialize() {
 	init(TEST, 0);
 }
+
+void TestKnobs::reset() {
+	explicitlySetKnobs.clear();
+	initialize();
+}

fdbserver/LocalConfiguration.h

@@ -32,15 +32,16 @@ class TestKnobs : public Knobs {
 public:
 	int64_t TEST;
 	void initialize();
+	void reset();
 };
 
 class LocalConfiguration {
 	std::unique_ptr<class LocalConfigurationImpl> impl;
 
 public:
-	LocalConfiguration(ConfigClassSet const& configClasses,
+	LocalConfiguration(std::string const& configPath,
 	                   std::string const& dataFolder,
-	                   std::map<Key, Value>&& manuallyOverriddenKnobs,
+	                   std::map<Key, Value>&& manualKnobOverrides,
 	                   UID id);
 	~LocalConfiguration();
 	Future<Void> init();

fdbserver/SimulatedCluster.actor.cpp

@@ -227,7 +227,7 @@ ACTOR Future<ISimulator::KillType> simulatedFDBDRebooter(Reference<ClusterConnec
 					                       "",
 					                       -1,
 					                       whitelistBinPaths,
-					                       ConfigClassSet{}));
+					                       {}));
 				}
 				if (runBackupAgents != AgentNone) {
 					futures.push_back(runBackup(connFile));

fdbserver/WorkerInterface.actor.h

@@ -824,7 +824,7 @@ ACTOR Future<Void> fdbd(Reference<ClusterConnectionFile> ccf,
                         std::string metricsPrefix,
                         int64_t memoryProfilingThreshold,
                         std::string whitelistBinPaths,
-                        ConfigClassSet configClassSet);
+                        std::string configPath);
 
 ACTOR Future<Void> clusterController(Reference<ClusterConnectionFile> ccf,
                                      Reference<AsyncVar<Optional<ClusterControllerFullInterface>>> currentCC,

fdbserver/fdbserver.actor.cpp

@@ -50,6 +50,7 @@
 #include "fdbserver/CoroFlow.h"
 #include "fdbserver/DataDistribution.actor.h"
 #include "fdbserver/IKeyValueStore.h"
+#include "fdbserver/LocalConfiguration.h"
 #include "fdbserver/MoveKeys.actor.h"
 #include "fdbserver/NetworkTest.h"
 #include "fdbserver/ServerDBInfo.h"
@@ -90,7 +91,7 @@ enum {
 	OPT_DCID, OPT_MACHINE_CLASS, OPT_BUGGIFY, OPT_VERSION, OPT_BUILD_FLAGS, OPT_CRASHONERROR, OPT_HELP, OPT_NETWORKIMPL, OPT_NOBUFSTDOUT, OPT_BUFSTDOUTERR,
 	OPT_TRACECLOCK, OPT_NUMTESTERS, OPT_DEVHELP, OPT_ROLLSIZE, OPT_MAXLOGS, OPT_MAXLOGSSIZE, OPT_KNOB, OPT_TESTSERVERS, OPT_TEST_ON_SERVERS, OPT_METRICSCONNFILE,
 	OPT_METRICSPREFIX, OPT_LOGGROUP, OPT_LOCALITY, OPT_IO_TRUST_SECONDS, OPT_IO_TRUST_WARN_ONLY, OPT_FILESYSTEM, OPT_PROFILER_RSS_SIZE, OPT_KVFILE,
-	OPT_TRACE_FORMAT, OPT_WHITELIST_BINPATH, OPT_BLOB_CREDENTIAL_FILE, OPT_CONFIG_CLASSES
+	OPT_TRACE_FORMAT, OPT_WHITELIST_BINPATH, OPT_BLOB_CREDENTIAL_FILE, OPT_CONFIG_PATH
 };
 
 CSimpleOpt::SOption g_rgOptions[] = {
@@ -172,7 +173,7 @@ CSimpleOpt::SOption g_rgOptions[] = {
 	{ OPT_TRACE_FORMAT      ,    "--trace_format",              SO_REQ_SEP },
 	{ OPT_WHITELIST_BINPATH,     "--whitelist_binpath",         SO_REQ_SEP },
 	{ OPT_BLOB_CREDENTIAL_FILE,  "--blob_credential_file",      SO_REQ_SEP },
-	{ OPT_CONFIG_CLASSES,        "--config_classes",            SO_REQ_SEP },
+	{ OPT_CONFIG_PATH,           "--config_path",               SO_REQ_SEP },
 
 #ifndef TLS_DISABLED
 	TLS_OPTION_FLAGS
@@ -969,7 +970,7 @@ struct CLIOptions {
 	std::vector<std::string> blobCredentials; // used for fast restore workers & backup workers
 	const char* blobCredsFromENV = nullptr;
 
-	ConfigClassSet configClassSet;
+	std::string configPath;
 
 	Reference<ClusterConnectionFile> connectionFile;
 	Standalone<StringRef> machineId;
@@ -1412,8 +1413,8 @@ private:
 					} while (t.size() != 0);
 				}
 				break;
-			case OPT_CONFIG_CLASSES:
+			case OPT_CONFIG_PATH:
-				configClassSet = ConfigClassSet::fromParamString(args.OptionArg());
+				configPath = args.OptionArg();
 				break;
 
 #ifndef TLS_DISABLED
@@ -1951,7 +1952,7 @@ int main(int argc, char* argv[]) {
 				                      opts.metricsPrefix,
 				                      opts.rsssize,
 				                      opts.whitelistBinPaths,
-				                      opts.configClassSet));
+				                      opts.configPath));
 				actors.push_back(histogramReport());
 				// actors.push_back( recurring( []{}, .001 ) );  // for ASIO latency measurement
 

fdbserver/worker.actor.cpp

@@ -2020,10 +2020,10 @@ ACTOR Future<Void> fdbd(Reference<ClusterConnectionFile> connFile,
                         std::string metricsPrefix,
                         int64_t memoryProfileThreshold,
                         std::string whitelistBinPaths,
-                        ConfigClassSet configClassSet) {
+                        std::string configPath) {
 	state vector<Future<Void>> actors;
 	state Promise<Void> recoveredDiskFiles;
-	state LocalConfiguration localConfig(configClassSet, dataFolder, {}, UID{});
+	state LocalConfiguration localConfig(configPath, dataFolder, {}, UID{});
 	wait(localConfig.init());
 
 	actors.push_back(serveProtocolInfo());