From d42245a4c7bbd74fd3f8e4514c429fef04357eb8 Mon Sep 17 00:00:00 2001 From: Dave Fisher Date: Fri, 13 Dec 2024 14:49:51 -0800 Subject: [PATCH] More Advisor Parameter and Configuration Tests (#2114) * More Advisor Parameter and Configuration Tests * Check for workload parameters not in adapter --- .../nb/api/config/standard/ConfigModel.java | 23 +++++++++---- .../nb/api/config/standard/NBConfigModel.java | 3 ++ .../uniform/StandardActivity.java | 33 ++++++++++++------- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/ConfigModel.java b/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/ConfigModel.java index c6b1cba26..b0ff0a6e5 100644 --- a/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/ConfigModel.java +++ b/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/ConfigModel.java @@ -16,7 +16,9 @@ package io.nosqlbench.nb.api.config.standard; +import io.nosqlbench.nb.api.advisor.NBAdvisorOutput; import io.nosqlbench.nb.api.errors.BasicError; +import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -218,6 +220,17 @@ public class ConfigModel implements NBConfigModel { return new NBConfiguration(this.asReadOnly(), validConfig); } + @Override + public void assertNoConflicts(Map config, String type) { + for (String configkey : config.keySet()) { + Param element = this.paramsByName.get(configkey); + if (element != null) { + String warning = "Config parameter '" + configkey + "' is also a " + type + ". Check for possible conflicts."; + NBAdvisorOutput.output(Level.WARN, warning); + } + } + } + @Override public void assertValidConfig(Map config) { ConfigModel expanded = expand(this, config); @@ -289,13 +302,9 @@ public class ConfigModel implements NBConfigModel { + ", possible parameter names are " + this.paramsByName.keySet() + "."; if (element == null) { String warnonly = System.getenv("NB_CONFIG_WARNINGS_ONLY"); - if (warnonly != null) { - System.out.println("WARNING: " + warning); - } else { - StringBuilder paramhelp = new StringBuilder( - "Unknown config parameter '" + configkey + "' in config model while configuring " + getOf().getSimpleName() - + ", possible parameter names are " + this.paramsByName.keySet() + "." - ); + logger.warn("WARNING: " + warning); + if (warnonly == null) { + StringBuilder paramhelp = new StringBuilder(warning); ConfigSuggestions.getForParam(this, configkey) .ifPresent(suggestion -> paramhelp.append(" ").append(suggestion)); throw new BasicError(paramhelp.toString()); diff --git a/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/NBConfigModel.java b/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/NBConfigModel.java index bf0987612..c000b9062 100644 --- a/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/NBConfigModel.java +++ b/nb-apis/nb-api/src/main/java/io/nosqlbench/nb/api/config/standard/NBConfigModel.java @@ -16,6 +16,7 @@ package io.nosqlbench.nb.api.config.standard; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -39,6 +40,8 @@ public interface NBConfigModel { void assertValidConfig(Map config); + void assertNoConflicts(Map config, String type); + NBConfiguration apply(Map config); Param getParam(String... name); diff --git a/nb-engine/nb-engine-core/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/StandardActivity.java b/nb-engine/nb-engine-core/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/StandardActivity.java index a7651d2e9..1abe910ee 100644 --- a/nb-engine/nb-engine-core/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/StandardActivity.java +++ b/nb-engine/nb-engine-core/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/StandardActivity.java @@ -74,9 +74,11 @@ public class StandardActivity exte List pops = new ArrayList<>(); ConcurrentHashMap, ? extends Space>> mappers = new ConcurrentHashMap<>(); NBConfigModel activityModel = activityDef.getConfigModel(); - String defaultDriverName = activityDef.getActivityDriver(); - DriverAdapter, Space> defaultAdapter = getDriverAdapter(defaultDriverName); NBConfigModel yamlmodel; + NBConfigModel adapterModel; + String defaultDriverName = activityDef.getActivityDriver(); + DriverAdapter,Space> defaultAdapter = getDriverAdapter(defaultDriverName); + DriverAdapter,Space> adapter; OpsDocList workload; Optional yaml_loc = activityDef.getParams().getOptionalString("yaml", "workload"); if (yaml_loc.isPresent()) { @@ -91,19 +93,20 @@ public class StandardActivity exte //NBConfigModel supersetConfig = ConfigModel.of(StandardActivity.class).add(activityModel); // Load the op templates List opTemplates = loadOpTemplates(defaultAdapter); + NBConfigModel combinedAdapterModel = ConfigModel.of(StandardActivity.class); for (OpTemplate ot : opTemplates) { logger.info(() -> "StandardActivity.opTemplate = "+ot); String driverName = ot.getOptionalStringParam("driver", String.class) .or(() -> ot.getOptionalStringParam("type", String.class)) .orElse(defaultDriverName); if (!adapters.containsKey(driverName)) { - DriverAdapter,Space> adapter = - defaultDriverName.equals(driverName) ? defaultAdapter : getDriverAdapter(driverName); + adapter = defaultDriverName.equals(driverName) ? defaultAdapter : getDriverAdapter(driverName); NBConfigModel combinedModel = yamlmodel; //NBConfigModel combinedModel = activityModel; NBConfiguration combinedConfig = combinedModel.matchConfig(activityDef.getParams()); if (adapter instanceof NBConfigurable configurable) { - NBConfigModel adapterModel = configurable.getConfigModel(); + adapterModel = configurable.getConfigModel(); + combinedAdapterModel.add(adapterModel); supersetConfig.add(adapterModel); combinedModel = adapterModel.add(yamlmodel); //combinedModel = adapterModel.add(activityModel); @@ -112,13 +115,16 @@ public class StandardActivity exte } adapters.put(driverName, adapter); mappers.put(driverName, adapter.getOpMapper()); + } else { + adapter = adapters.get(driverName); + } + if (adapter instanceof NBConfigurable configurable) { + adapterModel = configurable.getConfigModel(); + adapterModel.assertValidConfig(ot.getParams()); } paramsAdvisor.validateAll(ot.getParams().keySet()); paramsAdvisor.validateAll(ot.getTags().keySet()); - //TO-DO - paramsAdvisor.validateAll(ot.getBindings().keySet()); - supersetConfig.assertValidConfig(activityDef.getParams().getStringStringMap()); - supersetConfig.log(); - DriverAdapter, Space> adapter = adapters.get(driverName); + paramsAdvisor.validateAll(ot.getBindings().keySet()); adapterlist.add(adapter); ParsedOp pop = new ParsedOp(ot, adapter.getConfiguration(), List.of(adapter.getPreprocessor()), this); logger.info("StandardActivity.pop="+pop); @@ -134,6 +140,11 @@ public class StandardActivity exte paramsAdvisor.validateAll(supersetConfig.getNamedParams().keySet()); paramsAdvisor.logName().evaluate(); + combinedAdapterModel.assertNoConflicts(yamlmodel.getNamedParams(), "Template"); + combinedAdapterModel.log(); + supersetConfig.assertValidConfig(activityDef.getParams().getStringStringMap()); + supersetConfig.log(); + if (0 == mappers.keySet().stream().filter(n -> n.equals(defaultDriverName)).count()) { logger.warn(() -> "All op templates used a different driver than the default '" + defaultDriverName + "'"); } @@ -173,9 +184,9 @@ public class StandardActivity exte .map(l -> l.load(this, NBLabels.forKV()) ) .orElseThrow(() -> new OpConfigError("Unable to load '" + driverName + "' driver adapter.\n"+ - "Rebuild NB5 to include this driver adapter. "+ + "If this is a valid driver then you may need to rebuild NoSqlBench to include this driver adapter. "+ "Change 'false' for the driver in "+ - "'./nb-adapters/pom.xml' and './nb-adapters/nb-adapters-included/pom.xml' first.")); + "'./nb-adapters/pom.xml' and './nb-adapters/nb-adapters-included/pom.xml'.")); } @Override