Cleanup, no functional changes, take 2

Trying again, with fewer changes

1 files changed, 40 insertions, 38 deletions

diff --git a/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java b/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java
index 3a8d80e5ffe..43caf2d52fe 100644
--- a/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java
+++ b/config/src/main/java/com/yahoo/vespa/config/benchmark/LoadTester.java
@@ -1,4 +1,4 @@
-// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.benchmark;
 
 import com.yahoo.collections.Tuple2;
@@ -33,6 +33,8 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ThreadLocalRandom;
 
+import static com.yahoo.vespa.config.ConfigKey.createFull;
+
 /**
  * A config client for generating load against a config server or config proxy.
  * <p>
@@ -69,8 +71,7 @@ public class LoadTester {
         String configsList = parser.getBinarySwitches().get("-l");
         String defPath = parser.getBinarySwitches().get("-dd");
         debug = parser.getUnarySwitches().contains("-d");
-        LoadTester loadTester = new LoadTester();
-        loadTester.runLoad(host, port, iterations, threads, configsList, defPath);
+        new LoadTester().runLoad(host, port, iterations, threads, configsList, defPath);
     }
 
     private void runLoad(String host, int port, int iterations, int threads,
@@ -97,14 +98,17 @@ public class LoadTester {
     private Map<ConfigDefinitionKey, Tuple2<String, String[]>> readDefs(String defPath) throws IOException {
         Map<ConfigDefinitionKey, Tuple2<String, String[]>> ret = new HashMap<>();
         if (defPath == null) return ret;
+
         File defDir = new File(defPath);
         if (!defDir.isDirectory()) {
-            System.out.println("# Given def file dir is not a directory: " + defDir.getPath() + " , will not send def contents in requests.");
+            System.out.println("# Given def file dir is not a directory: " +
+                               defDir.getPath() + " , will not send def contents in requests.");
             return ret;
         }
-        final File[] files = defDir.listFiles();
+        File[] files = defDir.listFiles();
         if (files == null) {
-            System.out.println("# Given def file dir has no files: " + defDir.getPath() + " , will not send def contents in requests.");
+            System.out.println("# Given def file dir has no files: " +
+                               defDir.getPath() + " , will not send def contents in requests.");
             return ret;
         }
         for (File f : files) {
@@ -131,7 +135,7 @@ public class LoadTester {
         sb.append((metrics.failedRequests));
         sb.append("\n");
         sb.append('#').append(TransportMetrics.getInstance().snapshot().toString()).append('\n');
-        System.out.println(sb.toString());
+        System.out.println(sb);
     }
 
     private List<ConfigKey<?>> readConfigs(String configsList) throws IOException {
@@ -189,10 +193,10 @@ public class LoadTester {
 
     private class LoadThread extends Thread {
 
-        int iterations = 0;
-        String host = "";
-        int port = 0;
-        Metrics metrics = new Metrics();
+        private final int iterations;
+        private final String host;
+        private final int port;
+        private final Metrics metrics = new Metrics();
 
         LoadThread(int iterations, String host, int port) {
             this.iterations = iterations;
@@ -204,44 +208,24 @@ public class LoadTester {
         public void run() {
             Spec spec = new Spec(host, port);
             Target target = connect(spec);
-            ConfigKey<?> reqKey;
-            JRTClientConfigRequest request;
-            int totConfs = configs.size();
-            boolean reconnCycle = false; // to log reconn message only once, for instance at restart
+
             for (int i = 0; i < iterations; i++) {
-                reqKey = configs.get(ThreadLocalRandom.current().nextInt(totConfs));
+                ConfigKey<?> reqKey = configs.get(ThreadLocalRandom.current().nextInt(configs.size()));
                 ConfigDefinitionKey dKey = new ConfigDefinitionKey(reqKey);
                 Tuple2<String, String[]> defContent = defs.get(dKey);
                 if (defContent == null && defs.size() > 0) { // Only complain if we actually did run with a def dir
                     System.out.println("# No def found for " + dKey + ", not sending in request.");
                 }
-                request = getRequest(ConfigKey.createFull(reqKey.getName(), reqKey.getConfigId(), reqKey.getNamespace(), defContent.first), defContent.second);
+                ConfigKey<?> configKey = createFull(reqKey.getName(), reqKey.getConfigId(), reqKey.getNamespace(), defContent.first);
+                JRTClientConfigRequest request = createRequest(configKey, defContent.second);
                 if (debug) System.out.println("# Requesting: " + reqKey);
                 long start = System.currentTimeMillis();
                 target.invokeSync(request.getRequest(), 10.0);
                 long end = System.currentTimeMillis();
                 if (request.isError()) {
-                    if ("Connection lost".equals(request.errorMessage()) || "Connection down".equals(request.errorMessage())) {
-                        try {
-                            Thread.sleep(100);
-                        } catch (InterruptedException e) {
-                            throw new RuntimeException(e);
-                        }
-                        if (!reconnCycle) {
-                            System.out.println("# Connection lost, reconnecting...");
-                            reconnCycle = true;
-                        }
-                        target.close();
-                        target = connect(spec);
-                    } else {
-                        System.err.println(request.errorMessage());
-                    }
-                    metrics.incFailedRequests();
+                    target = handleError(request, spec, target);
                 } else {
-                    if (reconnCycle) {
-                        reconnCycle = false;
-                        System.out.println("# Connection OK");
-                    }
+                    System.out.println("# Connection OK");
                     long duration = end - start;
 
                     if (debug) {
@@ -255,7 +239,7 @@ public class LoadTester {
             }
         }
 
-        private JRTClientConfigRequest getRequest(ConfigKey<?> reqKey, String[] defContent) {
+        private JRTClientConfigRequest createRequest(ConfigKey<?> reqKey, String[] defContent) {
             if (defContent == null) defContent = new String[0];
             final long serverTimeout = 1000;
             return JRTClientConfigRequestV3.createWithParams(reqKey, DefContent.fromList(Arrays.asList(defContent)),
@@ -266,6 +250,24 @@ public class LoadTester {
         private Target connect(Spec spec) {
             return supervisor.connect(spec);
         }
+
+        private Target handleError(JRTClientConfigRequest request, Spec spec, Target target) {
+            if (List.of("Connection lost", "Connection down").contains(request.errorMessage())) {
+                try {
+                    Thread.sleep(100);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+                System.out.println("# Connection lost, reconnecting...");
+                target.close();
+                target = connect(spec);
+            } else {
+                System.err.println(request.errorMessage());
+            }
+            metrics.incFailedRequests();
+            return target;
+        }
+
     }
 
 }