Improve client benchmark (#19736)

* Allow to run client benchmark as an uberjar
* Busy wait to avoid accidental skew on low target throughput rates
* Trigger and wait for full GC to happen between trials
* Add missing SuppressForbidden to allow System.gc in client benchmark

client/benchmark/README.md

@@ -2,7 +2,7 @@ Steps to execute the benchmark:
 
 1. Start Elasticsearch on the target host (ideally *not* on the same machine)
 2. Create an empty index with the mapping you want to benchmark
-3. Start either the RestClientBenchmark class or the TransportClientBenchmark
+3. Build an uberjar with `gradle :client:benchmark:shadowJar` and execute it.
 4. Delete the index
 5. Repeat steps 2. - 4. for multiple iterations. The first iterations are intended as warmup for Elasticsearch itself. Always start the same benchmark in step 3!
 4. After the benchmark: Shutdown Elasticsearch and delete the data directory
@@ -17,11 +17,12 @@ Example benchmark:
 Example command line parameter list:
 
 ```
-192.168.2.2 /home/your_user_name/.rally/benchmarks/data/geonames/documents.json geonames type 8647880 5000 "{ \"query\": { \"match_phrase\": { \"name\": \"Sankt Georgen\" } } }\""
+rest 192.168.2.2 /home/your_user_name/.rally/benchmarks/data/geonames/documents.json geonames type 8647880 5000 "{ \"query\": { \"match_phrase\": { \"name\": \"Sankt Georgen\" } } }\""
 ```
 
 The parameters are in order:
 
+* Client type: Use either "rest" or "transport"
 * Benchmark target host IP (the host where Elasticsearch is running)
 * full path to the file that should be bulk indexed
 * name of the index
@@ -31,5 +32,3 @@ The parameters are in order:
 * a search request body (remember to escape double quotes). The `TransportClientBenchmark` uses `QueryBuilders.wrapperQuery()` internally which automatically adds a root key `query`, so it must not be present in the command line parameter. 
  
 You should also define a few GC-related settings `-Xms4096M -Xmx4096M  -XX:+UseConcMarkSweepGC -verbose:gc -XX:+PrintGCDetails` and keep an eye on GC activity. You can also define `-XX:+PrintCompilation` to see JIT activity.
-
-

client/benchmark/build.gradle

@@ -17,10 +17,30 @@
  * under the License.
  */
 
+buildscript {
+  repositories {
+    maven {
+      url 'https://plugins.gradle.org/m2/'
+    }
+  }
+  dependencies {
+    classpath 'com.github.jengelman.gradle.plugins:shadow:1.2.3'
+  }
+}
+
+
 apply plugin: 'elasticsearch.build'
+// build an uberjar with all benchmarks
+apply plugin: 'com.github.johnrengelman.shadow'
+// have the shadow plugin provide the runShadow task
+apply plugin: 'application'
 
 group = 'org.elasticsearch.client'
 
+archivesBaseName = 'client-benchmarks'
+mainClassName = 'org.elasticsearch.client.benchmark.BenchmarkMain'
+
+
 // never try to invoke tests on the benchmark project - there aren't any
 check.dependsOn.remove(test)
 // explicitly override the test task too in case somebody invokes 'gradle test' so it won't trip

client/benchmark/src/main/java/org/elasticsearch/client/benchmark/AbstractBenchmark.java

@@ -25,6 +25,9 @@ import org.elasticsearch.client.benchmark.ops.search.SearchRequestExecutor;
 import org.elasticsearch.common.SuppressForbidden;
 
 import java.io.Closeable;
+import java.lang.management.GarbageCollectorMXBean;
+import java.lang.management.ManagementFactory;
+import java.util.List;
 
 public abstract class AbstractBenchmark<T extends Closeable> {
     private static final int SEARCH_BENCHMARK_ITERATIONS = 10_000;
@@ -70,9 +73,8 @@ public abstract class AbstractBenchmark<T extends Closeable> {
                     System.out.println("=============");
 
                     for (int throughput = 100; throughput <= 100_000; throughput *= 10) {
-                        //request a GC between trials to reduce the likelihood of a GC occurring in the middle of a trial.
-                        System.gc();
-
+                        //GC between trials to reduce the likelihood of a GC occurring in the middle of a trial.
+                        runGc();
                         BenchmarkRunner searchBenchmark = new BenchmarkRunner(SEARCH_BENCHMARK_ITERATIONS, SEARCH_BENCHMARK_ITERATIONS,
                             new SearchBenchmarkTask(
                                 searchRequestExecutor(client, indexName), searchBody, 2 * SEARCH_BENCHMARK_ITERATIONS, throughput));
@@ -85,4 +87,31 @@ public abstract class AbstractBenchmark<T extends Closeable> {
             client.close();
         }
     }
+
+    /**
+     * Requests a full GC and checks whether the GC did actually run after a request. It retries up to 5 times in case the GC did not
+     * run in time.
+     */
+    @SuppressForbidden(reason = "we need to request a system GC for the benchmark")
+    private void runGc() {
+        long previousCollections = getTotalGcCount();
+        int attempts = 0;
+        do {
+            // request a full GC ...
+            System.gc();
+            // ... and give GC a chance to run
+            try {
+                Thread.sleep(2000);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                return;
+            }
+            attempts++;
+        } while (previousCollections == getTotalGcCount() || attempts < 5);
+    }
+
+    private long getTotalGcCount() {
+        List<GarbageCollectorMXBean> gcMxBeans = ManagementFactory.getGarbageCollectorMXBeans();
+        return gcMxBeans.stream().mapToLong(GarbageCollectorMXBean::getCollectionCount).sum();
+    }
 }

client/benchmark/src/main/java/org/elasticsearch/client/benchmark/BenchmarkMain.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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 org.elasticsearch.client.benchmark;
+
+import org.elasticsearch.client.benchmark.rest.RestClientBenchmark;
+import org.elasticsearch.client.benchmark.transport.TransportClientBenchmark;
+import org.elasticsearch.common.SuppressForbidden;
+
+import java.util.Arrays;
+
+public class BenchmarkMain {
+    @SuppressForbidden(reason = "system out is ok for a command line tool")
+    public static void main(String[] args) throws Exception {
+        String type = args[0];
+        AbstractBenchmark<?> benchmark = null;
+        switch (type) {
+            case "transport":
+                benchmark = new TransportClientBenchmark();
+                break;
+            case "rest":
+                benchmark = new RestClientBenchmark();
+                break;
+            default:
+                System.err.println("Unknown benchmark type [" + type + "]");
+                System.exit(1);
+        }
+        benchmark.run(Arrays.copyOfRange(args, 1, args.length));
+    }
+}

client/benchmark/src/main/java/org/elasticsearch/client/benchmark/ops/search/SearchBenchmarkTask.java

@@ -57,22 +57,17 @@ public class SearchBenchmarkTask implements BenchmarkTask {
 
             int waitTime = (int) Math.floor(MICROS_PER_SEC / targetThroughput - (stop - start) / NANOS_PER_MICRO);
             if (waitTime > 0) {
-                // Thread.sleep() time is not very accurate (it's most of the time around 1 - 2 ms off)
-                // so we rather busy spin for the last few microseconds. Still not entirely accurate but way closer
                 waitMicros(waitTime);
             }
         }
     }
 
     private void waitMicros(int waitTime) throws InterruptedException {
-        int millis = waitTime / 1000;
-        int micros = waitTime % 1000;
-        if (millis > 0) {
-            Thread.sleep(millis);
-        }
-        // busy spin for the rest of the time
-        if (micros > 0) {
-            long end = System.nanoTime() + 1000L * micros;
+        // Thread.sleep() time is not very accurate (it's most of the time around 1 - 2 ms off)
+        // we busy spin all the time to avoid introducing additional measurement artifacts (noticed 100% skew on 99.9th percentile)
+        // this approach is not suitable for low throughput rates (in the second range) though
+        if (waitTime > 0) {
+            long end = System.nanoTime() + 1000L * waitTime;
             while (end > System.nanoTime()) {
                 // busy spin
             }