Code review adjustments

This commit is contained in:
kijanowski
2023-05-17 09:47:07 +02:00
parent 2e47715ce7
commit bda0790bd4
8 changed files with 70 additions and 32 deletions

View File

@@ -170,6 +170,7 @@ public abstract class BaseDriverAdapter<R extends Op, S> implements DriverAdapte
.add(Param.optional(List.of("workload", "yaml"), String.class, "location of workload yaml file"))
.add(Param.optional("driver", String.class))
.add(Param.defaultTo("dryrun", "none").setRegex("(op|jsonnet|none)"))
.add(Param.optional("maxtries", Integer.class))
.asReadOnly();
}

View File

@@ -33,19 +33,17 @@ import java.util.function.Supplier;
*/
@Service(value = ErrorHandler.class, selector = "verifyexpected")
public class ExpectedResultVerificationErrorHandler implements ErrorHandler, ErrorMetrics.Aware {
private static final Logger logger = LogManager.getLogger(ExpectedResultVerificationErrorHandler.class);
private static final Logger logger = LogManager.getLogger("VERIFY");
private ExceptionExpectedResultVerificationMetrics exceptionExpectedResultVerificationMetrics;
@Override
public ErrorDetail handleError(String name, Throwable t, long cycle, long durationInNanos, ErrorDetail detail) {
if (t instanceof ExpectedResultVerificationError erve) {
if (erve.getTriesLeft() == 0) {
logger.warn("Verification of result did not pass. All retries exhausted.");
logger.warn("Cycle: {} Verification of result {} did not pass following expression: {}", cycle, erve.getResultAsString(), erve.getExpectedResultExpression());
exceptionExpectedResultVerificationMetrics.countVerificationErrors();
} else {
logger.info("Cycle: {} Verification of result did not pass. {} retries left.", cycle, erve.getTriesLeft());
// TODO/MVEL: I think we should designate a separate logging channel for verification logic
// TODO JK: Should this be done via a dedicated logger in the LoggerConfig class? log4j2.xml files seem to be overridden?
exceptionExpectedResultVerificationMetrics.countVerificationRetries();
}
}

View File

@@ -110,11 +110,8 @@ public class StandardAction<A extends StandardActivity<R, ?>, R extends Op> impl
var expectedResultExpression = dispenser.getExpectedResultExpression();
if (shouldVerifyExpectedResultFor(op, expectedResultExpression)) {
var verified = MVEL.executeExpression(expectedResultExpression, result, boolean.class);
// TODO/MVEL: Wherever this logic lives, we might want to have a symbolic description which
// is emitted for logging our metrics purposes indicating the success or failure outcomes.
// perhaps something like expected-name: .... and metrics could be then <expected-name>-success and <expected-name>-error
if (!verified) {
throw new ExpectedResultVerificationError(maxTries - tries);
throw new ExpectedResultVerificationError(maxTries - tries, expectedResultExpression, result);
}
}
} catch (Exception e) {
@@ -125,6 +122,7 @@ public class StandardAction<A extends StandardActivity<R, ?>, R extends Op> impl
if (error == null) {
resultSuccessTimer.update(nanos, TimeUnit.NANOSECONDS);
dispenser.onSuccess(cycle, nanos, op.getResultSize());
break;
} else {
ErrorDetail detail = errorHandler.handleError(error, cycle, nanos);
dispenser.onError(cycle, nanos, error);

View File

@@ -207,11 +207,12 @@ class NBErrorHandlerTest {
}
private static Stream<Arguments> testExpectedResultVerificationErrorHandler() {
Logger logger = (Logger) LogManager.getLogger(ExpectedResultVerificationErrorHandler.class);
Logger logger = (Logger) LogManager.getLogger("VERIFY");
var obj = new Object();
return Stream.of(
Arguments.of(
"retries left",
new ExpectedResultVerificationError(5),
new ExpectedResultVerificationError(5, "expected", obj),
"Cycle: 1 Verification of result did not pass. 5 retries left.",
1,
0,
@@ -219,8 +220,8 @@ class NBErrorHandlerTest {
),
Arguments.of(
"no retries left",
new ExpectedResultVerificationError(0),
"Verification of result did not pass. All retries exhausted.",
new ExpectedResultVerificationError(0, "expected", obj),
String.format("Cycle: 1 Verification of result %s did not pass following expression: %s", obj.toString(), "expected"),
0,
1,
logger

View File

@@ -160,6 +160,7 @@ public class NBCLI implements Function<String[], Integer> {
.setMaxLogs(globalOptions.getLogsMax())
.setLogsDirectory(globalOptions.getLogsDirectory())
.setAnsiEnabled(globalOptions.isEnableAnsi())
.setDedicatedVerificationLogger(globalOptions.isDedicatedVerificationLogger())
.activate();
ConfigurationFactory.setConfigurationFactory(loggerConfig);
@@ -246,7 +247,7 @@ public class NBCLI implements Function<String[], Integer> {
}
NBCLIOptions options = new NBCLIOptions(args);
logger = LogManager.getLogger("NBCLI"); // TODO JK - already present in L166
logger = LogManager.getLogger("NBCLI");
NBIO.addGlobalIncludes(options.wantsIncludes());

View File

@@ -124,6 +124,7 @@ public class NBCLIOptions {
private static final String DEFAULT_CONSOLE_PATTERN = "TERSE";
private static final String DEFAULT_LOGFILE_PATTERN = "VERBOSE";
private final static String ENABLE_DEDICATED_VERIFICATION_LOGGER = "--enable-dedicated-verification-logging";
// private static final String DEFAULT_CONSOLE_LOGGING_PATTERN = "%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n";
@@ -186,6 +187,7 @@ public class NBCLIOptions {
private String graphitelogLevel="info";
private boolean wantsListCommands = false;
private boolean wantsListApps = false;
private boolean dedicatedVerificationLogger = false;
public boolean isWantsListApps() {
return wantsListApps;
@@ -227,6 +229,14 @@ public class NBCLIOptions {
return this.graphitelogLevel;
}
public boolean isDedicatedVerificationLogger() {
return this.dedicatedVerificationLogger;
}
public void enableDedicatedVerificationLogger() {
this.dedicatedVerificationLogger = true;
}
public enum Mode {
ParseGlobalsOnly,
ParseAllOptions
@@ -340,6 +350,10 @@ public class NBCLIOptions {
setWantsStackTraces(true);
arglist.removeFirst();
break;
case ENABLE_DEDICATED_VERIFICATION_LOGGER:
enableDedicatedVerificationLogger();
arglist.removeFirst();
break;
case ANNOTATE_EVENTS:
arglist.removeFirst();
String toAnnotate = readWordOrThrow(arglist, "annotated events");

View File

@@ -89,6 +89,7 @@ public class LoggerConfig extends ConfigurationFactory {
private int maxLogfiles = 100;
private String logfileLocation;
private boolean ansiEnabled;
private boolean isDedicatedVerificationLoggerEnabled = false;
public LoggerConfig() {
@@ -109,6 +110,11 @@ public class LoggerConfig extends ConfigurationFactory {
return this;
}
public LoggerConfig setDedicatedVerificationLogger(boolean enabled) {
this.isDedicatedVerificationLoggerEnabled = enabled;
return this;
}
/**
* Ensure that what is shown in the logfile includes at a minimum,
* everything that is shown on console, but allow it to show more
@@ -211,24 +217,10 @@ public class LoggerConfig extends ConfigurationFactory {
.addComponent(triggeringPolicy);
builder.add(logsAppenderBuilder);
// RESULTVERIFYLOG appender
AppenderComponentBuilder resultVerificationAppenderBuilder =
builder
.newAppender("RESULTVERIFYLOG", FileAppender.PLUGIN_NAME)
.addAttribute("append", false)
.addAttribute("fileName", loggerDir.resolve("expected-result-verification.log").toString())
.add(builder
.newLayout("PatternLayout")
.addAttribute("pattern", "%d %p %C{1.} [%t] %m%n")
);
builder.add(resultVerificationAppenderBuilder);
// Result Verification logging
builder.add(builder
.newLogger(ExpectedResultVerificationErrorHandler.class.getName(), Level.INFO)
.add(builder.newAppenderRef("RESULTVERIFYLOG"))
.addAttribute("additivity", false)
);
if (isDedicatedVerificationLoggerEnabled) {
var verificationLogfilePath = loggerDir.resolve(filebase + "_verification.log").toString();
addResultVerificationLoggingChannel(builder, verificationLogfilePath);
}
rootBuilder.add(
builder.newAppenderRef("SCENARIO_APPENDER")
@@ -388,4 +380,23 @@ public class LoggerConfig extends ConfigurationFactory {
this.loggerDir = logsDirectory;
return this;
}
private void addResultVerificationLoggingChannel(ConfigurationBuilder<BuiltConfiguration> builder, String verificationLogfilePath) {
var appenderName = "RESULTVERIFYLOG";
var appender = builder
.newAppender(appenderName, FileAppender.PLUGIN_NAME)
.addAttribute("append", false)
.addAttribute("fileName", verificationLogfilePath)
.add(builder
.newLayout("PatternLayout")
.addAttribute("pattern", "%d %p %C{1.} [%t] %m%n")
);
var logger = builder
.newLogger("VERIFY", Level.INFO)
.add(builder.newAppenderRef(appenderName))
.addAttribute("additivity", false);
builder.add(appender);
builder.add(logger);
}
}

View File

@@ -16,14 +16,28 @@
package io.nosqlbench.api.errors;
import java.io.Serializable;
public class ExpectedResultVerificationError extends RuntimeException {
private final int triesLeft;
private final Serializable expectedResultExpression;
private final Object result;
public ExpectedResultVerificationError(int triesLeft) {
public ExpectedResultVerificationError(int triesLeft, Serializable expectedResultExpression, Object result) {
this.triesLeft = triesLeft;
this.expectedResultExpression = expectedResultExpression;
this.result = result;
}
public int getTriesLeft() {
return triesLeft;
}
public Serializable getExpectedResultExpression() {
return expectedResultExpression;
}
public String getResultAsString() {
return result.toString(); // TODO JK how to traverse the first x characters of the result? parse to json? via reflection?
}
}