diff --git a/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/opdispensers/Cqld4PreparedStmtDispenser.java b/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/opdispensers/Cqld4PreparedStmtDispenser.java index 4e72dad5c..2acfe1573 100644 --- a/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/opdispensers/Cqld4PreparedStmtDispenser.java +++ b/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/opdispensers/Cqld4PreparedStmtDispenser.java @@ -91,7 +91,8 @@ public class Cqld4PreparedStmtDispenser extends Cqld4BaseOpDispenser { isRetryReplace(), getMaxLwtRetries(), processors, - getExpectedResultExpression() + getExpectedResultExpression() // TODO/MVEL: When this is moved to BaseOpDispenser + StandardAction + // there will be no need to add it to the value type for the op, since the dispenser can hold it ); } catch (Exception exception) { return CQLD4PreparedStmtDiagnostics.rebindWithDiagnostics( diff --git a/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/optypes/Cqld4CqlOp.java b/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/optypes/Cqld4CqlOp.java index 4d46e5685..1d687bfb1 100644 --- a/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/optypes/Cqld4CqlOp.java +++ b/adapter-cqld4/src/main/java/io/nosqlbench/adapter/cqld4/optypes/Cqld4CqlOp.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 nosqlbench + * Copyright (c) 2022-2023 nosqlbench * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -109,6 +109,10 @@ public abstract class Cqld4CqlOp implements CycleOp, VariableCapture, Iterator reader = rs.iterator(); int pages = 0; + // TODO/MVEL: An optimization to this would be to collect the results in a result set processor, + // but allow/require this processor to be added to an op _only_ in the event that it would + // be needed by a downstream consumer like the MVEL expected result evaluator + var resultRows = new ArrayList(); while (true) { int pageRows = rs.getAvailableWithoutFetching(); @@ -125,6 +129,7 @@ public abstract class Cqld4CqlOp implements CycleOp, VariableCapture, break; } totalRows += pageRows; // TODO JK what is this for? + // TODO/MVEL: JK: this is meant to go to a total-rows metric, although it is not wired correctly yet } processors.flush(); return rs; @@ -156,6 +161,11 @@ public abstract class Cqld4CqlOp implements CycleOp, VariableCapture, @Override public boolean verified() { // TODO JK can this be made CQL agnostic? And moved to BaseOpDispenser? + // TODO/MVEL: Yes, it can. The initial implementation should, and it should actually be inside + // the main StandardAction, _after_ CycleOp or ChainingOp result is computed return MVEL.executeExpression(expectedResultExpression, results.get(), 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 -success and -error } } diff --git a/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/BaseOpDispenser.java b/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/BaseOpDispenser.java index 79212b554..341186df5 100644 --- a/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/BaseOpDispenser.java +++ b/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/BaseOpDispenser.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 nosqlbench + * Copyright (c) 2022-2023 nosqlbench * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,6 +69,8 @@ public abstract class BaseOpDispenser implements OpDispenser configureExpectations(op); } + // TODO/MVEL: Please add some error handling around that explains to the user + // what happened in the event of a compilation failure. private void configureExpectations(ParsedOp op) { op.getOptionalStaticValue("expected-result", String.class) .map(MVEL::compileExpression) diff --git a/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/flowtypes/Op.java b/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/flowtypes/Op.java index d70014c8f..7a78ab7a4 100644 --- a/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/flowtypes/Op.java +++ b/adapters-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/flowtypes/Op.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 nosqlbench + * Copyright (c) 2022-2023 nosqlbench * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,6 +33,8 @@ package io.nosqlbench.engine.api.activityimpl.uniform.flowtypes; */ // TODO: optimize the runtime around the specific op type public interface Op extends OpResultSize { + // TODO/MVEL: Let's take this out of here to keep Op as a tagging interface + // I think it will sit fine on the BaseOpDispenser, and lifecycles align for this nicely default boolean verified() { return false; } diff --git a/engine-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/actions/StandardAction.java b/engine-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/actions/StandardAction.java index 0f3d236a0..ce91d2bfc 100644 --- a/engine-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/actions/StandardAction.java +++ b/engine-api/src/main/java/io/nosqlbench/engine/api/activityimpl/uniform/actions/StandardAction.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 nosqlbench + * Copyright (c) 2022-2023 nosqlbench * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -115,12 +115,13 @@ public class StandardAction, R extends Op> impl if (dispenser.getExpectedResultExpression() != null) { // TODO JK refactor the whole if/else break/continue tree if (op.verified()) { // TODO JK Could this be moved to BaseOpDispenser? - logger.info(() -> "Verification of result passed"); + logger.info(() -> "Verification of result passed"); // TODO/MVEL: this is too verbose per cycle break; } else { // retry var triesLeft = maxTries - tries; logger.info("Verification of result did not pass - {} retries left", triesLeft); + // TODO/MVEL: I think we should designate a separate logging channel for verification logic if (triesLeft == 0) { var retriesExhausted = new RuntimeException("Max retries for verification step exhausted."); // TODO JK do we need a dedicated exception here? VerificationRetriesExhaustedException? var errorDetail = errorHandler.handleError(retriesExhausted, cycle, nanos); @@ -129,6 +130,9 @@ public class StandardAction, R extends Op> impl break; } continue; + // TODO/MVEL: I think we should collapse all this if possible to throwing a UnverifiedError and let error handlers do their thing. + // TODO/MVEL: This would work nicely with existing mechanisms and allow users to route errors and status codes as they like. + // TODO/MVEL: A future refinement would be to allow customized error handlers (+-) per dispenser } } else { break;