aratno commented on code in PR #24: URL: https://github.com/apache/cassandra-harry/pull/24#discussion_r1414751715
########## harry-core/src/harry/generators/EntropySource.java: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package harry.generators; + +import harry.core.VisibleForTesting; + +/** + * Random generator interface that offers: + * * Settable seed + * * Ability to generate multiple "next" random seeds + * * Ability to generate multiple "dependent" seeds, from which we can retrace the base seed with subtraction + */ +public interface EntropySource +{ + long next(); + void seed(long seed); Review Comment: Is re-seeding supported after `next` is called? ########## harry-core/src/harry/model/sut/DoubleWritingSut.java: ########## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package harry.model.sut; + +import java.util.concurrent.CompletableFuture; +import java.util.regex.Pattern; + +public class DoubleWritingSut implements SystemUnderTest +{ + private final SystemUnderTest primary; + private final SystemUnderTest secondary; + + public DoubleWritingSut(SystemUnderTest primary, + SystemUnderTest secondary) + { + this.primary = primary; + this.secondary = secondary; + } + public boolean isShutdown() + { + return primary.isShutdown(); + } + + public void shutdown() + { + primary.shutdown(); + } + + private static final Pattern pattern = Pattern.compile("select", Pattern.CASE_INSENSITIVE); + + public Object[][] execute(String statement, ConsistencyLevel cl, Object... bindings) + { + if (pattern.matcher(statement).find()) + return primary.execute(statement, cl, bindings); + + secondary.execute(statement, cl, bindings); + return primary.execute(statement, cl, bindings); + } + + public CompletableFuture<Object[][]> executeAsync(String statement, ConsistencyLevel cl, Object... bindings) + { + return null; Review Comment: Would prefer an UnsupportedOperationException here ########## harry-core/src/harry/model/SelectHelper.java: ########## @@ -271,6 +275,18 @@ public static ResultSetRow resultSetToRow(SchemaSpec schema, OpSelectors.Monoton System.arraycopy(result, partitionKey.length + clusteringKey.length, staticColumns, 0, staticColumns.length); System.arraycopy(result, partitionKey.length + clusteringKey.length + staticColumns.length, regularColumns, 0, regularColumns.length); + + List<Long> visited_lts_list; + if (schema.trackLts) + { + visited_lts_list = (List<Long>) result[result.length - 1]; + visited_lts_list.sort(Long::compare); Review Comment: Don't we want these in the original order returned by the query? ########## harry-core/src/harry/visitors/MutatingVisitor.java: ########## @@ -116,21 +163,23 @@ public void afterLts(long lts, long pd) statements.clear(); bindings.clear(); - executeWithRetries(lts, pd, new CompiledStatement(query, bindingsArray)); + CompiledStatement compiledStatement = new CompiledStatement(query, bindingsArray); + executeWithRetries(lts, pd, compiledStatement); tracker.endModification(lts); + hadVisibleVisit = false; } @Override - public void operation(long lts, long pd, long cd, long opId, OpSelectors.OperationKind opType) + public void operation(Operation operation) { - CompiledStatement statement = operationInternal(lts, pd, cd, opId, opType); - statements.add(statement.cql()); - Collections.addAll(bindings, statement.bindings()); + hadVisibleVisit = operation.opKind() != OpSelectors.OperationKind.DELETE_PARTITION; Review Comment: Aren't delete range / slice also not visible? ########## harry-core/src/harry/generators/EntropySource.java: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package harry.generators; + +import harry.core.VisibleForTesting; + +/** + * Random generator interface that offers: + * * Settable seed + * * Ability to generate multiple "next" random seeds + * * Ability to generate multiple "dependent" seeds, from which we can retrace the base seed with subtraction + */ +public interface EntropySource +{ + long next(); + void seed(long seed); + + // We derive from entropy source here to avoid letting the step change state for other states + // For example, if you start drawing more entropy bits from one of the steps, but won't change + // other steps, their states won't change either. Review Comment: I like this description + API, makes the PCG "parallel streams" concept easier to understand for people that are less familiar ########## harry-core/src/harry/dsl/HistoryBuilder.java: ########## @@ -19,598 +19,511 @@ package harry.dsl; import java.util.*; -import java.util.function.Consumer; import java.util.function.LongSupplier; -import harry.core.Run; +import harry.core.Configuration; +import harry.core.MetricReporter; +import harry.ddl.SchemaSpec; +import harry.generators.EntropySource; +import harry.generators.JdkRandomEntropySource; +import harry.model.Model; import harry.model.OpSelectors; +import harry.model.QuiescentChecker; +import harry.model.clock.ApproximateMonotonicClock; +import harry.model.sut.SystemUnderTest; +import harry.operations.Query; +import harry.reconciler.Reconciler; +import harry.runner.DataTracker; import harry.visitors.MutatingRowVisitor; import harry.visitors.MutatingVisitor; import harry.visitors.ReplayingVisitor; import harry.visitors.VisitExecutor; -import static harry.model.OpSelectors.DefaultPdSelector.PARTITION_DESCRIPTOR_STREAM_ID; +// TODO: we can implement a pluggable time source via a custom clock, too; the only limitation is that order +// of timestamps has to be consistent with LTS order Review Comment: Don't all Harry clocks have this requirement? OffsetClock and ApproximateMonotonicClock both provide monotonicity of RTS given monotonic LTSs. ########## harry-core/src/harry/dsl/HistoryBuilder.java: ########## @@ -19,598 +19,511 @@ package harry.dsl; import java.util.*; -import java.util.function.Consumer; import java.util.function.LongSupplier; -import harry.core.Run; +import harry.core.Configuration; +import harry.core.MetricReporter; +import harry.ddl.SchemaSpec; +import harry.generators.EntropySource; +import harry.generators.JdkRandomEntropySource; +import harry.model.Model; import harry.model.OpSelectors; +import harry.model.QuiescentChecker; +import harry.model.clock.ApproximateMonotonicClock; +import harry.model.sut.SystemUnderTest; +import harry.operations.Query; +import harry.reconciler.Reconciler; +import harry.runner.DataTracker; import harry.visitors.MutatingRowVisitor; import harry.visitors.MutatingVisitor; import harry.visitors.ReplayingVisitor; import harry.visitors.VisitExecutor; -import static harry.model.OpSelectors.DefaultPdSelector.PARTITION_DESCRIPTOR_STREAM_ID; +// TODO: we can implement a pluggable time source via a custom clock, too; the only limitation is that order +// of timestamps has to be consistent with LTS order // TODO: we could use some sort of compact data structure or file format for navigable operation history -public class HistoryBuilder implements Iterable<ReplayingVisitor.Visit> +// this can have a _huge_ advantage of being able to produce arbitrary patterns, since you can specify history +// either in a _very_ random way, or in a _very_ predictable way, but still leave a footprint extremely small, +// since if you do not specify the details, we just produce fully random operations. + +/** + * History builder is a component for a simple yet flexible generation of arbitrary data. + * + * Review Comment: It seems like this is intended to be a common entrypoint for people to create new Harry tests. Those people are likely to look at this file first. Can you expand on this doc? ########## harry-core/src/harry/visitors/GeneratingVisitor.java: ########## @@ -50,10 +55,178 @@ private void generate(long lts, long pd) int opsPerLts = descriptorSelector.operationsPerLts(lts); for (long opId = 0; opId < opsPerLts; opId++) { - long cd = descriptorSelector.cd(pd, lts, opId, schema); - OpSelectors.OperationKind opType = descriptorSelector.operationType(pd, lts, opId); - operation(lts, pd, cd, opId, opType); + OpSelectors.OperationKind opKind = descriptorSelector.operationType(pd, lts, opId); + BaseOperation operation; + switch (opKind) + { + case INSERT: + case UPDATE: + operation = writeOp(lts, pd, opId, opKind); + break; + case INSERT_WITH_STATICS: + case UPDATE_WITH_STATICS: + operation = writeRegularAndStatic(lts, pd, opId, opKind); + break; + case DELETE_ROW: + { + long cd = descriptorSelector.cd(pd, lts, opId, schema); + operation = new GeneratedDeleteRowOp(lts, pd, cd, opId, opKind); + break; + } + case DELETE_COLUMN: + case DELETE_COLUMN_WITH_STATICS: + { + long cd = descriptorSelector.cd(pd, lts, opId, schema); + BitSet columns = descriptorSelector.columnMask(pd, lts, opId, opKind); + operation = new GeneratedDeleteColumnsOp(lts, pd, cd, opId, opKind, columns); + break; + } + case DELETE_PARTITION: + case DELETE_RANGE: + case DELETE_SLICE: + operation = new GeneratedDeleteOp(lts, pd, opId, opKind, rangeSelector); + break; + default: + throw new IllegalStateException("All cases are covered but not " + opKind); + } + operation(operation); } afterLts(lts, pd); } + + public BaseOperation writeOp(long lts, long pd, long opId, OpSelectors.OperationKind kind) + { + long cd = descriptorSelector.cd(pd, lts, opId, schema); + + return new GeneratedWriteOp(lts, pd, cd, opId, kind) + { + @Override + public long[] vds() + { + return descriptorSelector.vds(pd, cd(), lts, opId, opKind(), schema); + } + }; + } + + public BaseOperation writeRegularAndStatic(long lts, long pd, long opId, OpSelectors.OperationKind kind) + { + long cd = descriptorSelector.cd(pd, lts, opId, schema); + + return new GeneratedWriteWithStaticOp(lts, pd, cd, opId, kind) + { + public long[] sds() + { + return descriptorSelector.sds(pd, cd(), lts, opId, opKind(), schema); + } + + @Override + public long[] vds() + { + return descriptorSelector.vds(pd, cd(), lts, opId, opKind(), schema); + } + }; + } + + public abstract static class GeneratedWriteOp extends BaseOperation implements ReplayingVisitor.WriteOp + { + protected final long cd; + public GeneratedWriteOp(long lts, long pd, long cd, long opId, OpSelectors.OperationKind opKind) + { + super(lts, pd, opId, opKind); + this.cd = cd; + } + + @Override + public long cd() + { + return cd; + } + } + + public abstract static class GeneratedWriteWithStaticOp extends GeneratedWriteOp implements ReplayingVisitor.WriteStaticOp + { + public GeneratedWriteWithStaticOp(long lts, long pd, long cd, long opId, OpSelectors.OperationKind opKind) + { + super(lts, pd, cd, opId, opKind); + } + } + + public static class GeneratedDeleteRowOp extends BaseOperation implements ReplayingVisitor.DeleteRowOp + { + private final long cd; + public GeneratedDeleteRowOp(long lts, long pd, long cd, long opId, OpSelectors.OperationKind opKind) + { + super(lts, pd, opId, opKind); + this.cd = cd; + } + + @Override + public long cd() + { + return cd; + } + } + + public static class GeneratedDeleteOp extends BaseOperation implements ReplayingVisitor.DeleteOp + { + private final Query relations; + + public GeneratedDeleteOp(long lts, long pd, long opId, OpSelectors.OperationKind opKind, QueryGenerator queryGenerator) + { + this(lts, pd, opId, opKind, queryGenerator.inflate( lts, opId, queryKind(opKind))); + } + + public GeneratedDeleteOp(long lts, long pd, long opId, OpSelectors.OperationKind opKind, Query relations) + { + super(lts, pd, opId, opKind); + this.relations = relations; + } + + @Override + public Query relations() + { + return relations; + } + + protected static Query.QueryKind queryKind(OpSelectors.OperationKind opKind) + { + switch (opKind) + { + case DELETE_PARTITION: + return Query.QueryKind.SINGLE_PARTITION; + case DELETE_ROW: + return Query.QueryKind.SINGLE_CLUSTERING; + case DELETE_RANGE: + return Query.QueryKind.CLUSTERING_RANGE; + case DELETE_SLICE: + return Query.QueryKind.CLUSTERING_SLICE; + default: + throw new IllegalStateException(String.format("Can not transform %s into delete", opKind)); + } + } + } + + public static class GeneratedDeleteColumnsOp extends BaseOperation implements ReplayingVisitor.DeleteColumnsOp + { + private final long cd; + private final BitSet columnMask; + + public GeneratedDeleteColumnsOp(long lts, long pd, long cd, long opId, OpSelectors.OperationKind opKind, BitSet columnMask) + { + super(lts, pd, opId, opKind); + this.cd = cd; + this.columnMask = columnMask; + } + + public long cd() + { + return cd; + } + + @Override + public harry.util.BitSet columns() Review Comment: Unnecessary qualified class name ########## harry-core/src/harry/visitors/MutatingRowVisitor.java: ########## @@ -26,108 +26,90 @@ import harry.operations.CompiledStatement; import harry.operations.DeleteHelper; import harry.operations.WriteHelper; -import harry.operations.Query; -import harry.operations.QueryGenerator; import harry.util.BitSet; public class MutatingRowVisitor implements OperationExecutor { protected final SchemaSpec schema; protected final OpSelectors.MonotonicClock clock; - protected final OpSelectors.DescriptorSelector descriptorSelector; - protected final QueryGenerator rangeSelector; protected final MetricReporter metricReporter; public MutatingRowVisitor(Run run) { this(run.schemaSpec, run.clock, - run.descriptorSelector, - run.rangeSelector, run.metricReporter); } @VisibleForTesting public MutatingRowVisitor(SchemaSpec schema, OpSelectors.MonotonicClock clock, - OpSelectors.DescriptorSelector descriptorSelector, - QueryGenerator rangeSelector, MetricReporter metricReporter) { this.metricReporter = metricReporter; this.schema = schema; this.clock = clock; - this.descriptorSelector = descriptorSelector; - this.rangeSelector = rangeSelector; } - public CompiledStatement insert(long lts, long pd, long cd, long opId) + public CompiledStatement insert(VisitExecutor.WriteOp op) Review Comment: A very welcome improvement! ########## harry-core/src/harry/model/sut/DoubleWritingSut.java: ########## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package harry.model.sut; + +import java.util.concurrent.CompletableFuture; +import java.util.regex.Pattern; + +public class DoubleWritingSut implements SystemUnderTest +{ + private final SystemUnderTest primary; + private final SystemUnderTest secondary; + + public DoubleWritingSut(SystemUnderTest primary, + SystemUnderTest secondary) + { + this.primary = primary; + this.secondary = secondary; + } + public boolean isShutdown() + { + return primary.isShutdown(); + } + + public void shutdown() + { + primary.shutdown(); + } + + private static final Pattern pattern = Pattern.compile("select", Pattern.CASE_INSENSITIVE); + + public Object[][] execute(String statement, ConsistencyLevel cl, Object... bindings) + { + if (pattern.matcher(statement).find()) + return primary.execute(statement, cl, bindings); Review Comment: Eventually could see an `Operation<SUT>` API in the future, to make this easier to express -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
