You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/12/09 09:56:53 UTC

[GitHub] [cassandra-harry] ifesdjeen commented on a change in pull request #5: Enable execution against external cluster / Cleanup

ifesdjeen commented on a change in pull request #5:
URL: https://github.com/apache/cassandra-harry/pull/5#discussion_r539131065



##########
File path: conf/example.yaml
##########
@@ -31,9 +31,9 @@ truncate_table: false
 # to map it back to the logical timestamp of the operation that wrote this value.
 clock:
   approximate_monotonic:
-    historySize: 7300
-    epochPeriod: 1
-    epochTimeUnit: "SECONDS"
+    history_size: 7300
+    epoch_length: 1
+    epoch_time_unit: "SECONDS"

Review comment:
       well spotted! However, if this has happened, we need to rename java props as well, since jackson will attempt to dump properties with their field names.

##########
File path: harry-runner/src/harry/runner/HarryRunner.java
##########
@@ -36,21 +38,13 @@
 import harry.corruptor.QueryResponseCorruptor;
 import harry.model.ExhaustiveChecker;
 import harry.model.OpSelectors;
-import harry.model.sut.InJvmSut;
-import org.apache.cassandra.distributed.impl.IsolatedExecutor;
-import org.apache.cassandra.distributed.test.TestBaseImpl;
 
-public class HarryRunner extends TestBaseImpl
+public interface HarryRunner

Review comment:
       In this case, should we move inteface to `core`? 

##########
File path: harry-runner/src/harry/runner/HarryRunner.java
##########
@@ -59,37 +53,7 @@ public void runWithInJvmDtest() throws Throwable
         ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1);
         executor.setRemoveOnCancelPolicy(true);
 
-        Configuration.ConfigurationBuilder configuration = new Configuration.ConfigurationBuilder();
-
-        long seed = System.currentTimeMillis();
-        configuration.setSeed(seed)
-                     .setSUT(new InJvmSut.InJvmSutConfiguration(3,
-                                                                10,
-                                                                System.getProperty("harry.root", "/tmp/cassandra/harry/") + System.currentTimeMillis()))
-                     .setPartitionDescriptorSelector(new Configuration.DefaultPDSelectorConfiguration(10, 100))
-                     .setClusteringDescriptorSelector((builder) -> {
-                         builder.setNumberOfModificationsDistribution(new Configuration.ConstantDistributionConfig(10))
-                                .setRowsPerModificationDistribution(new Configuration.ConstantDistributionConfig(10))
-                                .setMaxPartitionSize(100)
-                                .setOperationKindWeights(new Configuration.OperationKindSelectorBuilder()
-                                                         .addWeight(OpSelectors.OperationKind.DELETE_ROW, 1)
-                                                         .addWeight(OpSelectors.OperationKind.DELETE_COLUMN, 1)
-                                                         .addWeight(OpSelectors.OperationKind.DELETE_RANGE, 1)
-                                                         .addWeight(OpSelectors.OperationKind.WRITE, 97)
-                                                         .build());
-                     })
-                     .setRowVisitor(new Configuration.DefaultRowVisitorConfiguration())
-                     .setClock(new Configuration.ApproximateMonotonicClockConfiguration((int) TimeUnit.HOURS.toSeconds(2) + 100,
-                                                                                        1,
-                                                                                        TimeUnit.SECONDS))
-                     .setRunTime(2, TimeUnit.HOURS)
-                     .setCreateSchema(true)
-                     .setTruncateTable(false)
-                     .setDropSchema(false)
-                     .setModel(ExhaustiveChecker::new)
-                     .setRunner(new Configuration.ConcurrentRunnerConfig(1, 1, 1));
-
-        Runner runner = configuration.build().createRunner();

Review comment:
       This is great. I should've done it this way in the first place, but this code existed long before there was even any configuration. Great job!

##########
File path: conf/example.yaml
##########
@@ -31,9 +31,9 @@ truncate_table: false
 # to map it back to the logical timestamp of the operation that wrote this value.
 clock:
   approximate_monotonic:
-    historySize: 7300
-    epochPeriod: 1
-    epochTimeUnit: "SECONDS"
+    history_size: 7300
+    epoch_length: 1
+    epoch_time_unit: "SECONDS"

Review comment:
       turns out there are more places like this. If you like you can just leave it and i'll fix it later, as you wish.

##########
File path: harry-runner/src/harry/runner/HarryRunner.java
##########
@@ -107,7 +71,8 @@ public void runWithInJvmDtest() throws Throwable
                 return a;
             }).get(run.snapshot.run_time_unit.toSeconds(run.snapshot.run_time) + 30,
                    TimeUnit.SECONDS);
-            ((Throwable) result).printStackTrace();

Review comment:
       We can (and probably should) switch to `logger.error` here?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org