You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by up...@apache.org on 2021/04/01 22:44:29 UTC

[geode-benchmarks] branch feature/redis-performance-testing updated (d61b87d -> 6bc641f)

This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a change to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git.


    from d61b87d  Fixing the name of the DumpResults class in build.gradle
     new b9c9e63  gradle spA
     new 2ccd1d7  Converting all of the withXXX properties to start with benchmark.
     new 002b902  Replacing metadata.json with metadata properties files
     new 8392f66  Automatically passing properties prefixed with benchmark.system.ROLE. to JVMs
     new 6bc641f  Logging all benchmark properties during test run

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 geode-benchmarks/build.gradle                      | 66 ++++++++++++++--------
 .../geode/benchmark/parameters/GcParameters.java   |  2 +-
 .../benchmark/parameters/GedisParameters.java      |  3 +-
 .../benchmark/parameters/GeodeProperties.java      |  1 -
 .../geode/benchmark/parameters/HeapParameters.java |  2 +-
 .../geode/benchmark/tasks/ProcessControl.java      |  2 +-
 .../benchmark/tasks/redis/JedisClientManager.java  |  2 +-
 .../tasks/redis/LettuceClientManager.java          |  2 +-
 .../geode/benchmark/tests/GeodeBenchmark.java      |  2 +-
 .../benchmark/tests/redis/RedisBenchmark.java      |  4 +-
 .../benchmark/tests/redis/RedisHsetBenchmark.java  |  2 +-
 .../ClientServerTopologyWithRouterAndSniProxy.java |  4 +-
 .../topology/ClientServerTopologyWithSniProxy.java |  4 +-
 .../apache/geode/benchmark/topology/Topology.java  | 14 ++---
 .../topology/redis/ManualRedisTopology.java        |  2 +-
 .../benchmark/parameters/GcParametersTest.java     |  2 +-
 .../benchmark/parameters/HeapParametersTest.java   |  2 +-
 .../benchmark/tests/ClientServerBenchmarkTest.java | 12 ++--
 .../topology/ClientServerTopologyTest.java         | 23 ++++----
 .../ClientServerTopologyWithSniProxyTest.java      |  2 +-
 harness/build.gradle                               |  1 -
 .../apache/geode/perftest/BenchmarkProperties.java | 25 ++++++++
 .../java/org/apache/geode/perftest/TestConfig.java |  3 +-
 .../org/apache/geode/perftest/TestRunners.java     |  6 +-
 .../org/apache/geode/perftest/WorkloadConfig.java  |  6 +-
 .../apache/geode/perftest/jvms/JVMLauncher.java    |  2 +-
 .../geode/perftest/runner/DefaultTestRunner.java   | 60 +++++++-------------
 .../geode/perftest/BenchmarkPropertiesTest.java    | 24 ++++++++
 .../geode/perftest/TestRunnerIntegrationTest.java  | 10 ++++
 .../perftest/yardstick/YardstickTaskTest.java      |  5 +-
 .../geode/infrastructure/BenchmarkMetadata.java    |  2 +-
 .../geode/infrastructure/aws/LaunchCluster.java    | 20 ++++---
 32 files changed, 190 insertions(+), 127 deletions(-)
 create mode 100644 harness/src/main/java/org/apache/geode/perftest/BenchmarkProperties.java
 create mode 100644 harness/src/test/java/org/apache/geode/perftest/BenchmarkPropertiesTest.java

[geode-benchmarks] 05/05: Logging all benchmark properties during test run

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git

commit 6bc641f8615e213ef85141d515a222d7e2ddb1e9
Author: Dan Smith <da...@vmware.com>
AuthorDate: Thu Apr 1 15:17:47 2021 -0700

    Logging all benchmark properties during test run
---
 geode-benchmarks/build.gradle                              |  6 +++---
 .../main/java/org/apache/geode/perftest/TestRunners.java   |  6 +++---
 .../apache/geode/perftest/runner/DefaultTestRunner.java    | 14 ++++++++++++--
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/geode-benchmarks/build.gradle b/geode-benchmarks/build.gradle
index 49fa625..b2f5e26 100644
--- a/geode-benchmarks/build.gradle
+++ b/geode-benchmarks/build.gradle
@@ -100,9 +100,9 @@ task benchmark(type: Test) {
   systemProperty 'org.slf4j.simpleLogger.dateTimeFormat', 'yyyy-MM-dd HH:mm:ss.SSS'
   systemProperty 'org.slf4j.simpleLogger.showThreadName', 'false'
   systemProperty 'org.slf4j.simpleLogger.showShortLogName', 'true'
-  systemProperty 'TEST_HOSTS', project.findProperty('hosts')
-  systemProperty 'TEST_METADATA', project.findProperty('metadata')
-  systemProperty 'OUTPUT_DIR', outputDir
+  systemProperty 'benchmark.TEST_HOSTS', project.findProperty('hosts')
+  systemProperty 'benchmark.TEST_METADATA', project.findProperty('metadata')
+  systemProperty 'benchmark.OUTPUT_DIR', outputDir
 
   //Set all project properties starting with "benchmark." as system properties in the test
   //JVM
diff --git a/harness/src/main/java/org/apache/geode/perftest/TestRunners.java b/harness/src/main/java/org/apache/geode/perftest/TestRunners.java
index 680071b..27fddb3 100644
--- a/harness/src/main/java/org/apache/geode/perftest/TestRunners.java
+++ b/harness/src/main/java/org/apache/geode/perftest/TestRunners.java
@@ -38,8 +38,8 @@ import org.apache.geode.perftest.runner.DefaultTestRunner;
  */
 public class TestRunners {
 
-  public static final String TEST_HOSTS = "TEST_HOSTS";
-  public static final String OUTPUT_DIR = "OUTPUT_DIR";
+  public static final String TEST_HOSTS = "benchmark.TEST_HOSTS";
+  public static final String OUTPUT_DIR = "benchmark.OUTPUT_DIR";
 
   public static final String[] JVM_ARGS_SMALL_SIZE = new String[] {
       "-XX:CMSInitiatingOccupancyFraction=60",
@@ -91,7 +91,7 @@ public class TestRunners {
   static TestRunner defaultRunner(String testHosts, File outputDir) {
     if (testHosts == null) {
       throw new IllegalStateException(
-          "You must set the TEST_HOSTS system property to a comma separated list of hosts to run the benchmarks on.");
+          "You must set the benchmark.TEST_HOSTS system property to a comma separated list of hosts to run the benchmarks on.");
     }
 
     String userName = System.getProperty("user.name");
diff --git a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
index ba4d5a8..c26b1c2 100644
--- a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
+++ b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
@@ -74,12 +74,16 @@ public class DefaultTestRunner implements TestRunner {
     }
 
     benchmarkOutput.mkdirs();
+    Properties properties = new Properties();
+    addVersionProperties(properties, getVersionProperties());
+    addSystemProperties(properties);
+    logger.info("Benchmark Properties {}", properties);
+
+
     String metadataFilename = outputDir + "/testrunner.properties";
     Path metadataOutput = Paths.get(metadataFilename);
 
     if (!metadataOutput.toFile().exists()) {
-      Properties properties = new Properties(System.getProperties());
-      addVersionProperties(properties, getVersionProperties());
       try (FileWriter writer = new FileWriter(metadataOutput.toFile().getAbsoluteFile())) {
         properties.store(writer, "Benchmark metadata generated while running tests");
       }
@@ -112,6 +116,12 @@ public class DefaultTestRunner implements TestRunner {
 
   }
 
+  private void addSystemProperties(Properties properties) {
+    System.getProperties().stringPropertyNames().stream()
+        .filter(name -> name.startsWith("benchmark."))
+        .forEach(name -> properties.setProperty(name, System.getProperty(name)));
+  }
+
   private void addVersionProperties(Properties jsonMetadata, Properties versionProperties) {
     jsonMetadata.put("benchmark.source_version", versionProperties.getProperty("Product-Version"));
     jsonMetadata.put("benchmark.source_branch", versionProperties.getProperty("Source-Repository"));

[geode-benchmarks] 01/05: gradle spA

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git

commit b9c9e637ce1f66cf74c1f7d0e363909e94b5fd71
Author: Dan Smith <da...@vmware.com>
AuthorDate: Thu Apr 1 15:32:19 2021 -0700

    gradle spA
---
 .../java/org/apache/geode/benchmark/parameters/GeodeProperties.java     | 1 -
 .../src/main/java/org/apache/geode/benchmark/tasks/ProcessControl.java  | 2 +-
 .../java/org/apache/geode/benchmark/tasks/redis/JedisClientManager.java | 2 +-
 .../org/apache/geode/benchmark/tasks/redis/LettuceClientManager.java    | 2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GeodeProperties.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GeodeProperties.java
index fa37465..bef313b 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GeodeProperties.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GeodeProperties.java
@@ -25,7 +25,6 @@ import static org.apache.geode.distributed.ConfigurationProperties.CONSERVE_SOCK
 import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
 import static org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
 import static org.apache.geode.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS;
-import static org.apache.geode.distributed.ConfigurationProperties.LOCATOR_WAIT_TIME;
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_DISK_SPACE_LIMIT;
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE_SIZE_LIMIT;
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/ProcessControl.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/ProcessControl.java
index 57905a3..3567157 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/ProcessControl.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/ProcessControl.java
@@ -65,7 +65,7 @@ public class ProcessControl {
             format("'%s' command exited with status %d", join(" ", processBuilder.command()),
                 exitStatus, System.getProperty("user.dir"));
         logger.error(msg);
-        if(System.nanoTime() - start > RETRY_TIMEOUT.toNanos()) {
+        if (System.nanoTime() - start > RETRY_TIMEOUT.toNanos()) {
           throw new RuntimeException(msg);
         }
         Thread.sleep(100);
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/JedisClientManager.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/JedisClientManager.java
index 713d987..2b33700 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/JedisClientManager.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/JedisClientManager.java
@@ -94,7 +94,7 @@ public final class JedisClientManager implements RedisClientManager {
         }
         logger.debug(clusterInfo);
       } catch (Exception e) {
-        if(System.nanoTime() - start > CONNECT_TIMEOUT.toNanos()) {
+        if (System.nanoTime() - start > CONNECT_TIMEOUT.toNanos()) {
           throw e;
         }
         Thread.sleep(50);
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/LettuceClientManager.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/LettuceClientManager.java
index df9b160..bc82546 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/LettuceClientManager.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tasks/redis/LettuceClientManager.java
@@ -93,7 +93,7 @@ public final class LettuceClientManager implements RedisClientManager {
         }
         logger.debug(clusterInfo);
       } catch (Exception e) {
-        if(System.nanoTime() - start > CONNECT_TIMEOUT.toNanos()) {
+        if (System.nanoTime() - start > CONNECT_TIMEOUT.toNanos()) {
           throw e;
         }
         logger.info("Failed connecting.", e);

[geode-benchmarks] 02/05: Converting all of the withXXX properties to start with benchmark.

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git

commit 2ccd1d762b761cab22dc04a8f6d0170dcab7ce0f
Author: Dan Smith <da...@vmware.com>
AuthorDate: Thu Apr 1 13:04:59 2021 -0700

    Converting all of the withXXX properties to start with benchmark.
    
    Converting all of the system properties we use in the benchmarks to start with the benchmark
    prefix. Changing the gradle build to copy all benchmark.* properties as system
    properties in the tst.
    
    For now, also copying the old set of withXXX properties to not break peoples scripts or CI.
---
 geode-benchmarks/build.gradle                      | 60 ++++++++++++++--------
 .../geode/benchmark/parameters/GcParameters.java   |  2 +-
 .../benchmark/parameters/GedisParameters.java      |  3 +-
 .../geode/benchmark/parameters/HeapParameters.java |  2 +-
 .../geode/benchmark/tests/GeodeBenchmark.java      |  2 +-
 .../benchmark/tests/redis/RedisBenchmark.java      |  4 +-
 .../benchmark/tests/redis/RedisHsetBenchmark.java  |  2 +-
 .../ClientServerTopologyWithRouterAndSniProxy.java |  4 +-
 .../topology/ClientServerTopologyWithSniProxy.java |  4 +-
 .../apache/geode/benchmark/topology/Topology.java  | 14 ++---
 .../topology/redis/ManualRedisTopology.java        |  2 +-
 .../benchmark/parameters/GcParametersTest.java     |  2 +-
 .../benchmark/parameters/HeapParametersTest.java   |  2 +-
 .../benchmark/tests/ClientServerBenchmarkTest.java | 12 ++---
 .../topology/ClientServerTopologyTest.java         | 23 +++++----
 .../ClientServerTopologyWithSniProxyTest.java      |  2 +-
 .../org/apache/geode/perftest/WorkloadConfig.java  |  6 +--
 .../apache/geode/perftest/jvms/JVMLauncher.java    |  2 +-
 .../perftest/yardstick/YardstickTaskTest.java      |  5 +-
 19 files changed, 88 insertions(+), 65 deletions(-)

diff --git a/geode-benchmarks/build.gradle b/geode-benchmarks/build.gradle
index 49b423f..49fa625 100644
--- a/geode-benchmarks/build.gradle
+++ b/geode-benchmarks/build.gradle
@@ -103,64 +103,80 @@ task benchmark(type: Test) {
   systemProperty 'TEST_HOSTS', project.findProperty('hosts')
   systemProperty 'TEST_METADATA', project.findProperty('metadata')
   systemProperty 'OUTPUT_DIR', outputDir
+
+  //Set all project properties starting with "benchmark." as system properties in the test
+  //JVM
+  project.properties.findAll {
+    it.key.startsWith("benchmark.")
+  }.each {
+    systemProperty(it.getKey(), it.getValue())
+  }
+
+  //------------------------------------------------------------
+  //Legacy properties - these properties were added before the benchmark.
+  //prefix convention. They will be passed on to the JVM for now to not break
+  //CI or peoples scripts. Remove these soon!
+  //------------------------------------------------------------
   if (project.hasProperty('withGc')) {
-    systemProperty 'withGc', project.findProperty('withGc')
+    systemProperty 'benchmark.withGc', project.findProperty('withGc')
   }
   if (project.hasProperty('withHeap')) {
-    systemProperty 'withHeap', project.findProperty('withHeap')
+    systemProperty 'benchmark.withHeap', project.findProperty('withHeap')
   }
   if (project.hasProperty('withThreads')) {
-    systemProperty 'withThreads', project.findProperty('withThreads')
+    systemProperty 'benchmark.withThreads', project.findProperty('withThreads')
   }
   if (project.hasProperty('withWarmup')) {
-    systemProperty 'withWarmup', project.findProperty('withWarmup')
+    systemProperty 'benchmark.withWarmup', project.findProperty('withWarmup')
   }
   if (project.hasProperty('withDuration')) {
-    systemProperty 'withDuration', project.findProperty('withDuration')
+    systemProperty 'benchmark.withDuration', project.findProperty('withDuration')
   }
 
   if (project.hasProperty('withMinKey')) {
-    systemProperty 'withMinKey', project.findProperty('withMinKey')
+    systemProperty 'benchmark.withMinKey', project.findProperty('withMinKey')
   }
   if (project.hasProperty('withMaxKey')) {
-    systemProperty 'withMaxKey', project.findProperty('withMaxKey')
+    systemProperty 'benchmark.withMaxKey', project.findProperty('withMaxKey')
   }
 
-  systemProperty 'withSsl', project.hasProperty('withSsl')
-  systemProperty 'withSslProtocols', project.findProperty('withSslProtocols')
-  systemProperty 'withSslCiphers', project.findProperty('withSslCiphers')
+  systemProperty 'benchmark.withSsl', project.hasProperty('withSsl')
+  systemProperty 'benchmark.withSslProtocols', project.findProperty('withSslProtocols')
+  systemProperty 'benchmark.withSslCiphers', project.findProperty('withSslCiphers')
 
   if (project.hasProperty('withSniProxy')) {
-    systemProperty 'withSniProxy', project.findProperty('withSniProxy')
+    systemProperty 'benchmark.withSniProxy', project.findProperty('withSniProxy')
   }
-  systemProperty 'withSniProxyImage', project.findProperty('withSniProxyImage')
+  systemProperty 'benchmark.withSniProxyImage', project.findProperty('withSniProxyImage')
 
   if (project.hasProperty('withRouter')) {
-    systemProperty 'withRouter', project.findProperty('withRouter')
+    systemProperty 'benchmark.withRouter', project.findProperty('withRouter')
   }
-  systemProperty 'withRouterImage', project.findProperty('withRouterImage')
+  systemProperty 'benchmark.withRouterImage', project.findProperty('withRouterImage')
 
   if (project.hasProperty('withRedisClient')) {
-    systemProperty 'withRedisClient', project.findProperty('withRedisClient')
+    systemProperty 'benchmark.withRedisClient', project.findProperty('withRedisClient')
   }
 
   if (project.hasProperty('withRedisCluster')) {
-    systemProperty 'withRedisCluster', project.findProperty('withRedisCluster')
+    systemProperty 'benchmark.withRedisCluster', project.findProperty('withRedisCluster')
   }
 
   if (project.hasProperty('withRedisServers')) {
-    systemProperty 'withRedisServers', project.findProperty('withRedisServers')
+    systemProperty 'benchmark.withRedisServers', project.findProperty('withRedisServers')
   }
 
   if (project.hasProperty('withReplicas')) {
-    systemProperty 'withReplicas', project.findProperty('withReplicas')
+    systemProperty 'benchmark.withReplicas', project.findProperty('withReplicas')
   }
 
-  systemProperty 'withValidation', project.hasProperty('withValidation')
-  systemProperty 'withAsyncReplication', project.hasProperty('withAsyncReplication')
+  systemProperty 'benchmark.withValidation', project.hasProperty('withValidation')
+  systemProperty 'benchmark.withAsyncReplication', project.hasProperty('withAsyncReplication')
 
-  systemProperty 'withSecurityManager', project.hasProperty('withSecurityManager')
-  systemProperty 'benchmark.profiler.argument', project.findProperty('benchmark.profiler.argument')
+  systemProperty 'benchmark.withSecurityManager', project.hasProperty('withSecurityManager')
+  //------------------------------------------------------------
+  //End legacy properties
+  //------------------------------------------------------------
 
 
   doFirst {
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GcParameters.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GcParameters.java
index 6239c0a..9398e5c 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GcParameters.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GcParameters.java
@@ -28,7 +28,7 @@ public class GcParameters {
 
   public static void configure(final TestConfig testConfig) {
     final GcImplementation gcImplementation =
-        GcImplementation.valueOf(System.getProperty("withGc", "CMS"));
+        GcImplementation.valueOf(System.getProperty("benchmark.withGc", "CMS"));
     logger.info("Configuring {} GC.", gcImplementation);
     switch (gcImplementation) {
       case CMS:
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GedisParameters.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GedisParameters.java
index c2806eb..1e641bb 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GedisParameters.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/GedisParameters.java
@@ -28,7 +28,8 @@ public class GedisParameters {
   public static void configure(final TestConfig testConfig) {
     logger.info("Configuring Gedis parameters.");
 
-    testConfig.jvmArgs(SERVER.name(), "-Dredis.replicas=" + Integer.getInteger("withReplicas", 1));
+    testConfig.jvmArgs(SERVER.name(),
+        "-Dredis.replicas=" + Integer.getInteger("benchmark.withReplicas", 1));
   }
 
 }
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/HeapParameters.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/HeapParameters.java
index fd58b6a..91b45a0 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/HeapParameters.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/parameters/HeapParameters.java
@@ -26,7 +26,7 @@ public class HeapParameters {
   private static final Logger logger = LoggerFactory.getLogger(HeapParameters.class);
 
   public static void configure(final TestConfig testConfig) {
-    final String heap = System.getProperty("withHeap", "8g");
+    final String heap = System.getProperty("benchmark.withHeap", "8g");
     logger.info("Configuring heap parameters {}.", heap);
     configureGeodeProductJvms(testConfig, "-Xmx" + heap, "-Xms" + heap);
   }
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/GeodeBenchmark.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/GeodeBenchmark.java
index 3276b72..c090ea9 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/GeodeBenchmark.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/GeodeBenchmark.java
@@ -36,7 +36,7 @@ public class GeodeBenchmark {
    */
   private static final int THREADS = Runtime.getRuntime().availableProcessors() * 10;
 
-  public static final String WITH_VALIDATION_PROPERTY = "withValidation";
+  public static final String WITH_VALIDATION_PROPERTY = "benchmark.withValidation";
 
   public static TestConfig createConfig() {
     TestConfig config = new TestConfig();
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisBenchmark.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisBenchmark.java
index f4f70b2..409b8db 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisBenchmark.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisBenchmark.java
@@ -36,8 +36,8 @@ import org.apache.geode.perftest.TestConfig;
 
 public class RedisBenchmark implements PerformanceTest {
 
-  public static final String WITH_REDIS_CLIENT_PROPERTY = "withRedisClient";
-  public static final String WITH_REDIS_CLUSTER_PROPERTY = "withRedisCluster";
+  public static final String WITH_REDIS_CLIENT_PROPERTY = "benchmark.withRedisClient";
+  public static final String WITH_REDIS_CLUSTER_PROPERTY = "benchmark.withRedisCluster";
 
   public static final String REDIS_SERVERS_ATTRIBUTE = "RedisBenchmark.Servers";
 
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisHsetBenchmark.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisHsetBenchmark.java
index 8cd1aca..9c296a6 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisHsetBenchmark.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/tests/redis/RedisHsetBenchmark.java
@@ -37,7 +37,7 @@ import org.apache.geode.perftest.TestRunners;
 public class RedisHsetBenchmark extends RedisBenchmark {
 
   private LongRange keyRange =
-      new LongRange(getLong("withMinKey", 0), getLong("withMaxKey", 1000000));
+      new LongRange(getLong("benchmark.withMinKey", 0), getLong("benchmark.withMaxKey", 1000000));
 
   @Test
   public void run() throws Exception {
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithRouterAndSniProxy.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithRouterAndSniProxy.java
index 502df52..b5b3467 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithRouterAndSniProxy.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithRouterAndSniProxy.java
@@ -35,8 +35,8 @@ import org.apache.geode.benchmark.tasks.StopRouter;
 import org.apache.geode.perftest.TestConfig;
 
 public class ClientServerTopologyWithRouterAndSniProxy extends ClientServerTopologyWithSniProxy {
-  public static final String WITH_ROUTER_PROPERTY = "withRouter";
-  public static final String WITH_ROUTER_IMAGE_PROPERTY = "withRouterImage";
+  public static final String WITH_ROUTER_PROPERTY = "benchmark.withRouter";
+  public static final String WITH_ROUTER_IMAGE_PROPERTY = "benchmark.withRouterImage";
 
   private static final int NUM_LOCATORS = 1;
   private static final int NUM_SERVERS = 2;
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxy.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxy.java
index 57af8fe..fde623f 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxy.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxy.java
@@ -40,8 +40,8 @@ import org.apache.geode.benchmark.tasks.StopSniProxy;
 import org.apache.geode.perftest.TestConfig;
 
 public class ClientServerTopologyWithSniProxy extends Topology {
-  public static final String WITH_SNI_PROXY_PROPERTY = "withSniProxy";
-  public static final String WITH_SNI_PROXY_IMAGE_PROPERTY = "withSniProxyImage";
+  public static final String WITH_SNI_PROXY_PROPERTY = "benchmark.withSniProxy";
+  public static final String WITH_SNI_PROXY_IMAGE_PROPERTY = "benchmark.withSniProxyImage";
 
   private static final int NUM_LOCATORS = 1;
   private static final int NUM_SERVERS = 2;
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/Topology.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/Topology.java
index 3e46bdf..b3336c2 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/Topology.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/Topology.java
@@ -27,14 +27,14 @@ import org.apache.geode.benchmark.parameters.ProfilerParameters;
 import org.apache.geode.perftest.TestConfig;
 
 public abstract class Topology {
-  public static final String WITH_SSL_PROPERTY = "withSsl";
-  static final String WITH_SSL_ARGUMENT = "-DwithSsl=true";
+  public static final String WITH_SSL_PROPERTY = "benchmark.withSsl";
+  static final String WITH_SSL_ARGUMENT = "-Dbenchmark.withSsl=true";
 
-  public static final String WITH_SSL_PROTOCOLS_PROPERTY = "withSslProtocols";
-  public static final String WITH_SSL_CIPHERS_PROPERTY = "withSslCiphers";
+  public static final String WITH_SSL_PROTOCOLS_PROPERTY = "benchmark.withSslProtocols";
+  public static final String WITH_SSL_CIPHERS_PROPERTY = "benchmark.withSslCiphers";
 
-  public static final String WITH_SECURITY_MANAGER_PROPERTY = "withSecurityManager";
-  static final String WITH_SECURITY_MANAGER_ARGUMENT = "-DwithSecurityManager=true";
+  public static final String WITH_SECURITY_MANAGER_PROPERTY = "benchmark.withSecurityManager";
+  static final String WITH_SECURITY_MANAGER_ARGUMENT = "-Dbenchmark.withSecurityManager=true";
 
   protected static void configureCommon(TestConfig config) {
     JvmParameters.configure(config);
@@ -48,7 +48,7 @@ public abstract class Topology {
     addToTestConfig(config, WITH_SSL_CIPHERS_PROPERTY);
     addToTestConfig(config, WITH_SECURITY_MANAGER_PROPERTY, WITH_SECURITY_MANAGER_ARGUMENT);
 
-    if (getBoolean("withAsyncReplication")) {
+    if (getBoolean("benchmark.withAsyncReplication")) {
       config.jvmArgs(SERVER.name(), "-Dgemfire.disablePartitionedRegionBucketAck=true");
     }
 
diff --git a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/redis/ManualRedisTopology.java b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/redis/ManualRedisTopology.java
index f239772..4b8eb56 100644
--- a/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/redis/ManualRedisTopology.java
+++ b/geode-benchmarks/src/main/java/org/apache/geode/benchmark/topology/redis/ManualRedisTopology.java
@@ -33,7 +33,7 @@ import org.apache.geode.perftest.TestConfig;
 public class ManualRedisTopology extends Topology {
   private static final int NUM_CLIENTS = 4;
 
-  public static final String WITH_REDIS_SERVERS_PROPERTY = "withRedisServers";
+  public static final String WITH_REDIS_SERVERS_PROPERTY = "benchmark.withRedisServers";
 
   public static void configure(TestConfig config) {
     role(config, CLIENT, NUM_CLIENTS);
diff --git a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/GcParametersTest.java b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/GcParametersTest.java
index 9e456fd..5003187 100644
--- a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/GcParametersTest.java
+++ b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/GcParametersTest.java
@@ -30,7 +30,7 @@ import org.junit.jupiter.api.Test;
 import org.apache.geode.perftest.TestConfig;
 
 class GcParametersTest {
-  private static final String WITH_GC = "withGc";
+  private static final String WITH_GC = "benchmark.withGc";
   private static final String JAVA_RUNTIME_VERSION = "java.runtime.version";
   private static final String XX_USE_ZGC = "-XX:+UseZGC";
   private static final String XX_USE_G_1_GC = "-XX:+UseG1GC";
diff --git a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/HeapParametersTest.java b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/HeapParametersTest.java
index 4303c6b..332fc43 100644
--- a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/HeapParametersTest.java
+++ b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/parameters/HeapParametersTest.java
@@ -30,7 +30,7 @@ import org.apache.geode.perftest.TestConfig;
 
 class HeapParametersTest {
 
-  private static final String WITH_HEAP = "withHeap";
+  private static final String WITH_HEAP = "benchmark.withHeap";
 
   private Properties systemProperties;
 
diff --git a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/tests/ClientServerBenchmarkTest.java b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/tests/ClientServerBenchmarkTest.java
index 58eb5c8..0663a1d 100644
--- a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/tests/ClientServerBenchmarkTest.java
+++ b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/tests/ClientServerBenchmarkTest.java
@@ -56,40 +56,40 @@ class GeodeBenchmarkTest {
 
   @AfterAll
   public static void afterAll() {
-    System.clearProperty("withSniProxy");
+    System.clearProperty("benchmark.withSniProxy");
   }
 
   @Test
   public void withoutSniProxy() {
-    System.clearProperty("withSniProxy");
+    System.clearProperty("benchmark.withSniProxy");
     config = ClientServerBenchmark.createConfig();
     assertThat(config.getBefore()).doesNotContain(startHAProxyStep, startEnvoyStep);
   }
 
   @Test
   public void withSniProxyInvalid() {
-    System.setProperty("withSniProxy", "invalid");
+    System.setProperty("benchmark.withSniProxy", "invalid");
     assertThatThrownBy(() -> ClientServerBenchmark.createConfig())
         .isInstanceOf(IllegalArgumentException.class);
   }
 
   @Test
   public void withSniProxyDefault() {
-    System.setProperty("withSniProxy", "");
+    System.setProperty("benchmark.withSniProxy", "");
     config = ClientServerBenchmark.createConfig();
     assertThat(config.getBefore()).contains(startHAProxyStep).doesNotContain(startEnvoyStep);
   }
 
   @Test
   public void withSniProxyHAProxy() {
-    System.setProperty("withSniProxy", "HAProxy");
+    System.setProperty("benchmark.withSniProxy", "HAProxy");
     config = ClientServerBenchmark.createConfig();
     assertThat(config.getBefore()).contains(startHAProxyStep);
   }
 
   @Test
   public void withSniProxyEnvoy() {
-    System.setProperty("withSniProxy", "Envoy");
+    System.setProperty("benchmark.withSniProxy", "Envoy");
     config = ClientServerBenchmark.createConfig();
     assertThat(config.getBefore()).contains(startEnvoyStep);
   }
diff --git a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyTest.java b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyTest.java
index 4fed317..63a940e 100644
--- a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyTest.java
+++ b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyTest.java
@@ -43,17 +43,18 @@ public class ClientServerTopologyTest {
 
   @Test
   public void configWithSsl() {
-    System.setProperty("withSsl", "true");
+    System.setProperty("benchmark.withSsl", "true");
     TestConfig testConfig = new TestConfig();
     ClientServerTopology.configure(testConfig);
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-DwithSsl=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-Dbenchmark.withSsl=true");
   }
 
   @Test
   public void configWithNoSsl() {
     TestConfig testConfig = new TestConfig();
     ClientServerTopology.configure(testConfig);
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).doesNotContain("-DwithSsl=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name()))
+        .doesNotContain("-Dbenchmark.withSsl=true");
   }
 
   @Test
@@ -61,27 +62,29 @@ public class ClientServerTopologyTest {
     TestConfig testConfig = new TestConfig();
     ClientServerTopology.configure(testConfig);
     assertThat(testConfig.getJvmArgs().get(CLIENT.name()))
-        .doesNotContain("-DwithSecurityManager=true");
+        .doesNotContain("-Dbenchmark.withSecurityManager=true");
   }
 
   @Test
   public void configWithSecurityManager() {
-    System.setProperty("withSecurityManager", "true");
+    System.setProperty("benchmark.withSecurityManager", "true");
     TestConfig testConfig = new TestConfig();
     ClientServerTopology.configure(testConfig);
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-DwithSecurityManager=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name()))
+        .contains("-Dbenchmark.withSecurityManager=true");
   }
 
   @Test
   public void configWithSecurityManagerAndSslAndJava11() {
-    System.setProperty("withSecurityManager", "true");
+    System.setProperty("benchmark.withSecurityManager", "true");
     System.setProperty("java.runtime.version", "11.0.4+11");
-    System.setProperty("withSsl", "true");
+    System.setProperty("benchmark.withSsl", "true");
     TestConfig testConfig = new TestConfig();
 
     ClientServerTopology.configure(testConfig);
 
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-DwithSecurityManager=true");
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-DwithSsl=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name()))
+        .contains("-Dbenchmark.withSecurityManager=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-Dbenchmark.withSsl=true");
   }
 }
diff --git a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxyTest.java b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxyTest.java
index 5e989c5..8938f21 100644
--- a/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxyTest.java
+++ b/geode-benchmarks/src/test/java/org/apache/geode/benchmark/topology/ClientServerTopologyWithSniProxyTest.java
@@ -51,7 +51,7 @@ public class ClientServerTopologyWithSniProxyTest {
     System.setProperty(WITH_SNI_PROXY_PROPERTY, sniProxyImplementation.name());
     final TestConfig testConfig = new TestConfig();
     ClientServerTopologyWithSniProxy.configure(testConfig);
-    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-DwithSsl=true");
+    assertThat(testConfig.getJvmArgs().get(CLIENT.name())).contains("-Dbenchmark.withSsl=true");
   }
 
 }
diff --git a/harness/src/main/java/org/apache/geode/perftest/WorkloadConfig.java b/harness/src/main/java/org/apache/geode/perftest/WorkloadConfig.java
index f62a90e..6517269 100644
--- a/harness/src/main/java/org/apache/geode/perftest/WorkloadConfig.java
+++ b/harness/src/main/java/org/apache/geode/perftest/WorkloadConfig.java
@@ -42,15 +42,15 @@ public class WorkloadConfig implements Serializable {
   public WorkloadConfig() {}
 
   public void durationSeconds(long durationSeconds) {
-    this.durationSeconds = Long.getLong("withDuration", durationSeconds);
+    this.durationSeconds = Long.getLong("benchmark.withDuration", durationSeconds);
   }
 
   public void warmupSeconds(long warmupSeconds) {
-    this.warmupSeconds = Long.getLong("withWarmup", warmupSeconds);
+    this.warmupSeconds = Long.getLong("benchmark.withWarmup", warmupSeconds);
   }
 
   public void threads(int threads) {
-    this.threads = Integer.getInteger("withThreads", threads);
+    this.threads = Integer.getInteger("benchmark.withThreads", threads);
   }
 
   public long getDurationSeconds() {
diff --git a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
index 0d26118..a08862b 100644
--- a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
+++ b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
@@ -92,7 +92,7 @@ class JVMLauncher {
     command.add("-D" + RemoteJVMFactory.ROLE + "=" + jvmConfig.getRole());
     command.add("-D" + RemoteJVMFactory.OUTPUT_DIR + "=" + jvmConfig.getOutputDir());
 
-    if (jvmConfig.getJvmArgs().contains("-DwithSsl=true")) {
+    if (jvmConfig.getJvmArgs().contains("-Dbenchmark.withSsl=true")) {
       command
           .add("-Dgemfire." + SSL_KEYSTORE + "=" + jvmConfig.getLibDir() + "/temp-self-signed.jks");
       command.add("-Dgemfire." + SSL_KEYSTORE_PASSWORD + "=123456");
diff --git a/harness/src/test/java/org/apache/geode/perftest/yardstick/YardstickTaskTest.java b/harness/src/test/java/org/apache/geode/perftest/yardstick/YardstickTaskTest.java
index 707ca43..4643b14 100644
--- a/harness/src/test/java/org/apache/geode/perftest/yardstick/YardstickTaskTest.java
+++ b/harness/src/test/java/org/apache/geode/perftest/yardstick/YardstickTaskTest.java
@@ -18,6 +18,7 @@
 package org.apache.geode.perftest.yardstick;
 
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
 
 import java.io.File;
 import java.nio.file.Files;
@@ -30,6 +31,7 @@ import org.apache.geode.perftest.Task;
 import org.apache.geode.perftest.TestContext;
 import org.apache.geode.perftest.WorkloadConfig;
 import org.apache.geode.perftest.benchmarks.EmptyBenchmark;
+import org.apache.geode.perftest.jvms.rmi.ControllerRemote;
 import org.apache.geode.perftest.runner.DefaultTestContext;
 import org.apache.geode.perftest.yardstick.hdrhistogram.HdrHistogramWriter;
 
@@ -45,7 +47,8 @@ public class YardstickTaskTest {
     workloadConfig.threads(1);
     Task task = new YardstickTask(benchmark, workloadConfig);
     File outputDir = folder.toFile();
-    TestContext context = new DefaultTestContext(null, outputDir, 1, role, controller);
+    ControllerRemote controller = mock(ControllerRemote.class);
+    TestContext context = new DefaultTestContext(null, outputDir, 1, "role", controller);
     task.run(context);
 
     assertTrue(1 <= benchmark.getInvocations());

[geode-benchmarks] 04/05: Automatically passing properties prefixed with benchmark.system.ROLE. to JVMs

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git

commit 8392f66a41d10ff977f90ef2375a7b5a8f66b2a2
Author: Dan Smith <da...@vmware.com>
AuthorDate: Thu Apr 1 14:43:31 2021 -0700

    Automatically passing properties prefixed with benchmark.system.ROLE. to JVMs
    
    Adding a way to automatically set system properties in test JVMs. Just add a property
    with the prefix benchmark.system.ROLE, where ROLE is the role of jvms to target. Eg
    
    benchmark.system.server.gemfire.disablePartitionedRegionBucketAck=true would set
    gemfire.disablePartitionedRegionBucketAck=true  in the server JVMs.
---
 .../apache/geode/perftest/BenchmarkProperties.java | 25 ++++++++++++++++++++++
 .../java/org/apache/geode/perftest/TestConfig.java |  3 +--
 .../geode/perftest/BenchmarkPropertiesTest.java    | 24 +++++++++++++++++++++
 .../geode/perftest/TestRunnerIntegrationTest.java  | 10 +++++++++
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/harness/src/main/java/org/apache/geode/perftest/BenchmarkProperties.java b/harness/src/main/java/org/apache/geode/perftest/BenchmarkProperties.java
new file mode 100644
index 0000000..0a7fd6e
--- /dev/null
+++ b/harness/src/main/java/org/apache/geode/perftest/BenchmarkProperties.java
@@ -0,0 +1,25 @@
+package org.apache.geode.perftest;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class BenchmarkProperties {
+
+  public static Map<String, List<String>> getDefaultJVMArgs() {
+    Map<String, List<String>> results = new HashMap<>();
+    System.getProperties().stringPropertyNames().stream()
+        .filter(name -> name.startsWith("benchmark.system."))
+        .forEach(name -> {
+          String shortName = name.replace("benchmark.system.", "");
+          String[] roleAndProperty = shortName.split("\\.", 2);
+          String role = roleAndProperty[0];
+          String property = roleAndProperty[1];
+          String value = System.getProperty(name);
+          List<String> roleProperties = results.computeIfAbsent(role, key -> new ArrayList<>());
+          roleProperties.add("-D" + property + "=" + value);
+        });
+    return results;
+  }
+}
diff --git a/harness/src/main/java/org/apache/geode/perftest/TestConfig.java b/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
index 1b66fdc..1e454e6 100644
--- a/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
+++ b/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
@@ -21,7 +21,6 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -39,7 +38,7 @@ public class TestConfig implements Serializable {
 
   private final WorkloadConfig workloadConfig = new WorkloadConfig();
   private Map<String, Integer> roles = new LinkedHashMap<>();
-  private Map<String, List<String>> jvmArgs = new HashMap<>();
+  private Map<String, List<String>> jvmArgs = BenchmarkProperties.getDefaultJVMArgs();
   private List<TestStep> before = new ArrayList<>();
   private List<TestStep> workload = new ArrayList<>();
   private List<TestStep> after = new ArrayList<>();
diff --git a/harness/src/test/java/org/apache/geode/perftest/BenchmarkPropertiesTest.java b/harness/src/test/java/org/apache/geode/perftest/BenchmarkPropertiesTest.java
new file mode 100644
index 0000000..a8a5105
--- /dev/null
+++ b/harness/src/test/java/org/apache/geode/perftest/BenchmarkPropertiesTest.java
@@ -0,0 +1,24 @@
+package org.apache.geode.perftest;
+
+import java.util.List;
+import java.util.Map;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class BenchmarkPropertiesTest {
+
+  @Test
+  public void canParsePropertiesForRoles() {
+    System.setProperty("benchmark.system.role1.p1", "v1");
+    System.setProperty("benchmark.system.role1.p2", "v2");
+
+    Map<String, List<String>> defaultArgs =
+        BenchmarkProperties.getDefaultJVMArgs();
+
+    Assertions.assertThat(defaultArgs).containsOnlyKeys("role1");
+
+    Assertions.assertThat(defaultArgs.get("role1")).containsExactlyInAnyOrder("-Dp1=v1", "-Dp2=v2");
+  }
+
+}
diff --git a/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java b/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
index f82cb6c..2bddb1a 100644
--- a/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
+++ b/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
@@ -17,6 +17,7 @@
 
 package org.apache.geode.perftest;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -26,6 +27,7 @@ import java.nio.file.Path;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -38,6 +40,7 @@ import org.apache.geode.perftest.yardstick.analysis.YardstickThroughputSensorPar
 
 public class TestRunnerIntegrationTest {
 
+  public static final String TEST_PROPERTY = "benchmark.system.all.prop3";
   private TestRunner runner;
 
   @TempDir
@@ -51,6 +54,11 @@ public class TestRunnerIntegrationTest {
         outputDir);
   }
 
+  @AfterEach()
+  void clearProperty() {
+    System.clearProperty(TEST_PROPERTY);
+  }
+
   @Test
   public void runsBeforeWorkload() throws Exception {
     runner.runTest(() -> {
@@ -93,6 +101,7 @@ public class TestRunnerIntegrationTest {
 
   @Test
   public void configuresJVMOptions() throws Exception {
+    System.setProperty(TEST_PROPERTY, "p3");
     runner.runTest(() -> {
       TestConfig testConfig = new TestConfig();
       testConfig.role("all", 1);
@@ -102,6 +111,7 @@ public class TestRunnerIntegrationTest {
             "Expecting system property to be set in launched JVM, but it was not present.");
         assertEquals(5, Integer.getInteger("prop2").intValue(),
             "Expecting system property to be set in launched JVM, but it was not present.");
+        assertThat(System.getProperty("prop3")).isEqualTo("p3");
       }, "all");
       return testConfig;
     });

[geode-benchmarks] 03/05: Replacing metadata.json with metadata properties files

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch feature/redis-performance-testing
in repository https://gitbox.apache.org/repos/asf/geode-benchmarks.git

commit 002b902f3e79ed59d41728458fa403c56d7f93d5
Author: Dan Smith <da...@vmware.com>
AuthorDate: Thu Apr 1 13:33:05 2021 -0700

    Replacing metadata.json with metadata properties files
    
    Getting rid of problematic org.json dependency, and also making
    sure we capture *all* system properties that might effect the behavior of the
    test.
---
 harness/build.gradle                               |  1 -
 .../geode/perftest/runner/DefaultTestRunner.java   | 50 +++++-----------------
 .../geode/infrastructure/BenchmarkMetadata.java    |  2 +-
 .../geode/infrastructure/aws/LaunchCluster.java    | 20 +++++----
 4 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/harness/build.gradle b/harness/build.gradle
index ab85423..a108f00 100644
--- a/harness/build.gradle
+++ b/harness/build.gradle
@@ -55,7 +55,6 @@ dependencies {
   compile(group: 'commons-io', name: 'commons-io', version: project.'commons-io.version')
   compile(group: 'org.yardstickframework', name: 'yardstick', version: project.'yardstick.version')
   compile(group: 'org.hdrhistogram', name: 'HdrHistogram', version: project.'HdrHistogram.version')
-  compile(group: 'org.json', name: 'json', version: project.'JSON.version')
   compile(group: 'org.apache.geode', name: 'geode-core', version: geodeVersion)
   testCompile(group: 'org.mockito', name: 'mockito-all', version: project.'mockito-all.version')
   testCompile(group: 'org.awaitility', name: 'awaitility', version: project.'awaitility.version')
diff --git a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
index a71b158..ba4d5a8 100644
--- a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
+++ b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
@@ -21,15 +21,12 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
-import org.json.JSONArray;
-import org.json.JSONObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -77,42 +74,17 @@ public class DefaultTestRunner implements TestRunner {
     }
 
     benchmarkOutput.mkdirs();
-    String metadataFilename = outputDir + "/metadata.json";
+    String metadataFilename = outputDir + "/testrunner.properties";
     Path metadataOutput = Paths.get(metadataFilename);
-    JSONObject JSONmetadata = new JSONObject();
-
-    if (metadataOutput.toFile().exists()) {
-      JSONmetadata = new JSONObject(new String(Files.readAllBytes(metadataOutput)));
-    } else {
-      String metadata = System.getProperty("TEST_METADATA");
-      if (!(metadata == null) && !metadata.isEmpty()) {
-        JSONObject testMetadata = new JSONObject();
-        String[] metadataEntries = metadata.split(",");
-        for (String data : metadataEntries) {
-          String[] kv = data.split(":");
-          if (kv.length == 2) {
-            testMetadata.put(kv[0], kv[1]);
-          }
-        }
-        addVersionProperties(testMetadata, getVersionProperties());
-        JSONmetadata.put("testMetadata", testMetadata);
-      }
-    }
 
-    try {
-      JSONArray testNames = JSONmetadata.getJSONArray("testNames");
-      testNames.put(testName);
-      JSONmetadata.put("testNames", testNames);
-    } catch (org.json.JSONException e) {
-      JSONArray testNames = new JSONArray();
-      testNames.put(testName);
-      JSONmetadata.put("testNames", testNames);
+    if (!metadataOutput.toFile().exists()) {
+      Properties properties = new Properties(System.getProperties());
+      addVersionProperties(properties, getVersionProperties());
+      try (FileWriter writer = new FileWriter(metadataOutput.toFile().getAbsoluteFile())) {
+        properties.store(writer, "Benchmark metadata generated while running tests");
+      }
     }
 
-    FileWriter metadataWriter = new FileWriter(metadataOutput.toFile().getAbsoluteFile());
-    metadataWriter.write(JSONmetadata.toString());
-    metadataWriter.flush();
-
     Map<String, Integer> roles = config.getRoles();
     Map<String, List<String>> jvmArgs = config.getJvmArgs();
 
@@ -140,10 +112,10 @@ public class DefaultTestRunner implements TestRunner {
 
   }
 
-  private void addVersionProperties(JSONObject jsonMetadata, Properties versionProperties) {
-    jsonMetadata.put("source_version", versionProperties.getProperty("Product-Version"));
-    jsonMetadata.put("source_branch", versionProperties.getProperty("Source-Repository"));
-    jsonMetadata.put("source_revision", versionProperties.getProperty("Source-Revision"));
+  private void addVersionProperties(Properties jsonMetadata, Properties versionProperties) {
+    jsonMetadata.put("benchmark.source_version", versionProperties.getProperty("Product-Version"));
+    jsonMetadata.put("benchmark.source_branch", versionProperties.getProperty("Source-Repository"));
+    jsonMetadata.put("benchmark.source_revision", versionProperties.getProperty("Source-Revision"));
   }
 
   private Properties getVersionProperties() throws IOException {
diff --git a/infrastructure/src/main/java/org/apache/geode/infrastructure/BenchmarkMetadata.java b/infrastructure/src/main/java/org/apache/geode/infrastructure/BenchmarkMetadata.java
index 22c5d48..42a8d3a 100644
--- a/infrastructure/src/main/java/org/apache/geode/infrastructure/BenchmarkMetadata.java
+++ b/infrastructure/src/main/java/org/apache/geode/infrastructure/BenchmarkMetadata.java
@@ -44,6 +44,6 @@ public class BenchmarkMetadata {
   }
 
   public static String benchmarkMetadataFileName(String tag) {
-    return benchmarkConfigDirectory() + "/" + tag + "-metadata.json";
+    return benchmarkConfigDirectory() + "/" + tag + "-cluster-launch.properties";
   }
 }
diff --git a/infrastructure/src/main/java/org/apache/geode/infrastructure/aws/LaunchCluster.java b/infrastructure/src/main/java/org/apache/geode/infrastructure/aws/LaunchCluster.java
index 45c126a..5a2e081 100644
--- a/infrastructure/src/main/java/org/apache/geode/infrastructure/aws/LaunchCluster.java
+++ b/infrastructure/src/main/java/org/apache/geode/infrastructure/aws/LaunchCluster.java
@@ -19,6 +19,7 @@ package org.apache.geode.infrastructure.aws;
 import static java.lang.String.format;
 import static java.lang.Thread.sleep;
 
+import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -32,11 +33,10 @@ import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Locale;
+import java.util.Properties;
 import java.util.UUID;
 import java.util.stream.Collectors;
 
-import org.json.JSONArray;
-import org.json.JSONObject;
 import software.amazon.awssdk.services.ec2.Ec2Client;
 import software.amazon.awssdk.services.ec2.model.AllocateHostsRequest;
 import software.amazon.awssdk.services.ec2.model.AllocateHostsResponse;
@@ -283,19 +283,21 @@ public class LaunchCluster {
   private static void createMetadata(String benchmarkTag, List<String> publicIps)
       throws IOException {
     UUID instanceId = UUID.randomUUID();
-    JSONObject metadataJSON = new JSONObject();
-
-    metadataJSON.put("instanceId", instanceId.toString());
-    metadataJSON.put("publicIps", new JSONArray(publicIps));
+    // TODO - Filter out only benchmark properties from system properties? Maybe not necessary.
+    Properties metadata = new Properties(System.getProperties());
+    metadata.setProperty("benchmark.instanceId", instanceId.toString());
+    metadata.setProperty("benchmark.publicIps", String.join(",", publicIps));
     Path configDirectory = Paths.get(BenchmarkMetadata.benchmarkConfigDirectory());
 
     if (!configDirectory.toFile().exists()) {
       Files.createDirectories(Paths.get(BenchmarkMetadata.benchmarkConfigDirectory()));
     }
 
-    Path metadata = Files.write(Paths.get(AwsBenchmarkMetadata.metadataFileName(benchmarkTag)),
-        metadataJSON.toString().getBytes());
-    Files.setPosixFilePermissions(metadata, PosixFilePermissions.fromString("rw-------"));
+    Path metadataPath = Paths.get(AwsBenchmarkMetadata.metadataFileName(benchmarkTag));
+    try (FileWriter writer = new FileWriter(metadataPath.toFile())) {
+      metadata.store(writer, "Benchmark metadata generated during cluster launch");
+    }
+    Files.setPosixFilePermissions(metadataPath, PosixFilePermissions.fromString("rw-------"));
   }
 
   private static void createLaunchTemplate(String benchmarkTag, Image newestImage) {