Non-functional changes only

5 files changed, 36 insertions, 35 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
index 1accbd51ac7..6682f6ff1d0 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
@@ -330,6 +330,7 @@ public final class ConnectionParams {
         public Path getCertificate() { return certificate; }
         public Path getCaCertificates() { return caCertificates; }
     }
+
     private final SSLContext sslContext;
     private final Path privateKey;
     private final Path certificate;
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index d510ce4b7ea..f07ab025eb0 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -49,7 +49,7 @@ import java.util.zip.GZIPOutputStream;
  */
 class ApacheGatewayConnection implements GatewayConnection {
 
-    private static Logger log = Logger.getLogger(ApacheGatewayConnection.class.getName());
+    private static final Logger log = Logger.getLogger(ApacheGatewayConnection.class.getName());
     private static final ObjectMapper mapper = new ObjectMapper();
     private static final String PATH = "/reserved-for-internal-use/feedapi?";
     private final List<Integer> SUPPORTED_VERSIONS = new ArrayList<>();
@@ -114,15 +114,14 @@ class ApacheGatewayConnection implements GatewayConnection {
 
     @Override
     public InputStream drain() throws ServerResponseException, IOException {
-        return write(Collections.<Document>emptyList(), true /* drain */, false /* use compression */);
+        return write(Collections.<Document>emptyList(), true, false);
     }
 
     @Override
     public boolean connect() {
-        log.fine("Attempting to connect to " + endpoint);
-        if (httpClient != null) {
+        log.fine(() -> "Attempting to connect to " + endpoint);
+        if (httpClient != null)
             log.log(Level.WARNING, "Previous httpClient still exists.");
-        }
         httpClient = httpClientFactory.createClient();
         return httpClient != null;
     }
@@ -246,13 +245,9 @@ class ApacheGatewayConnection implements GatewayConnection {
     private InputStream executePost(HttpPost httpPost) throws ServerResponseException, IOException {
         HttpResponse response;
         try {
-            if (httpClient == null) {
+            if (httpClient == null)
                 throw new IOException("Trying to executePost while not having a connection/http client");
-            }
             response = httpClient.execute(httpPost);
-        } catch (IOException e) {
-            httpPost.abort();
-            throw e;
         } catch (Exception e) {
             httpPost.abort();
             throw e;
@@ -432,8 +427,6 @@ class ApacheGatewayConnection implements GatewayConnection {
             clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.currentVersion));
             clientBuilder.setDefaultHeaders(Collections.singletonList(new BasicHeader(Headers.CLIENT_VERSION, Vtag.currentVersion)));
             clientBuilder.disableContentCompression();
-            // Try to disable the disabling to see if system tests become stable again.
-            // clientBuilder.disableAutomaticRetries();
             RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
             requestConfigBuilder.setSocketTimeout(0);
             if (connectionParams.getProxyHost() != null) {
@@ -441,17 +434,16 @@ class ApacheGatewayConnection implements GatewayConnection {
             }
             clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build());
 
-            log.fine("Creating HttpClient: " + " ConnectionTimeout "
-                            + " SocketTimeout 0 secs "
-                            + " proxyhost (can be null) " + connectionParams.getProxyHost()
-                            + ":" + connectionParams.getProxyPort()
+            log.fine(() -> "Creating HttpClient:" +
+                           " ConnectionTimeout " + connectionParams.getConnectionTimeToLive().getSeconds() + " seconds" +
+                           " proxyhost (can be null) " + connectionParams.getProxyHost() + ":" + connectionParams.getProxyPort()
                             + (useSsl ? " using ssl " : " not using ssl")
             );
             return clientBuilder.build();
         }
     }
 
-    // Note: Using deprecated setSslcontext() to allow httpclient 4.4 on classpath (e.g unexpected Maven dependency resolution for test classpath)
+    // Note: Using deprecated setSslContext() to allow httpclient 4.4 on classpath (e.g unexpected Maven dependency resolution for test classpath)
     @SuppressWarnings("deprecation")
     private static void setSslContext(HttpClientBuilder builder, SSLContext sslContext) {
         builder.setSslcontext(sslContext);
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/DocumentQueue.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/DocumentQueue.java
index 16bf881963f..f6b3d1fb62a 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/DocumentQueue.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/DocumentQueue.java
@@ -11,8 +11,8 @@ import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 /**
- * Document queue that only gives you document operations on documents for which there are no
- * already in flight operations for.
+ * Shared document queue that gives clients operations on documents which do not have operations already in flight.
+ * This is multithread safe.
  *
  * @author dybis
  */
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
index cd146cf0e87..c4ee2f58b65 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/EndpointResultQueue.java
@@ -15,13 +15,19 @@ import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
 /**
+ * The shared queue of operation results.
+ * This is multithread safe.
+ *
  * @author Einar M R Rosenvinge
  */
 class EndpointResultQueue {
 
-    private static Logger log = Logger.getLogger(EndpointResultQueue.class.getName());
+    private static final Logger log = Logger.getLogger(EndpointResultQueue.class.getName());
     private final OperationProcessor operationProcessor;
+
+    /** The currently in flight operations */
     private final Map<String, TimerFuture> futureByOperation = new HashMap<>();
+
     private final Endpoint endpoint;
     private final int clusterId;
     private final ScheduledThreadPoolExecutor timer;
@@ -64,25 +70,23 @@ class EndpointResultQueue {
         TimerFuture timerFuture = futureByOperation.remove(result.getOperationId());
         if (timerFuture == null) {
             if (duplicateGivesWarning) {
-                log.warning(
-                        "Result for ID '" + result.getOperationId() + "' received from '" + endpoint
-                         + "', but we have no record of a sent operation. Either something is wrong on the server side "
-                         + "(bad VIP usage?), or we have somehow received duplicate results, "
-                         + "or operation was received _after_ client-side timeout.");
+                log.warning("Result for ID '" + result.getOperationId() + "' received from '" + endpoint +
+                            "', but we have no record of a sent operation. Either something is wrong on the server side " +
+                            "(bad VIP usage?), or we have somehow received duplicate results, " +
+                            "or operation was received _after_ client-side timeout.");
             }
             return;
         }
         timerFuture.getFuture().cancel(false);
     }
 
-    //Called only from ScheduledThreadPoolExecutor thread in DocumentTimerTask.run(), see below
+    /** Called only from ScheduledThreadPoolExecutor thread in DocumentTimerTask.run(), see below */
     private synchronized void timeout(String operationId) {
         TimerFuture timerFuture = futureByOperation.remove(operationId);
         if (timerFuture == null) {
-            log.finer(
-                    "Timeout of operation '" + operationId + "', but operation "
-                    + "not found in map. Result was probably received just-in-time from server, while timeout "
-                    + "task could not be cancelled.");
+            log.finer("Timeout of operation '" + operationId + "', but operation " +
+                      "not found in map. Result was probably received just-in-time from server, while timeout " +
+                      "task could not be cancelled.");
             return;
         }
         EndpointResult endpointResult = EndPointResultFactory.createTransientError(
@@ -108,6 +112,7 @@ class EndpointResultQueue {
     }
 
     private class DocumentTimerTask implements Runnable {
+
         private final String operationId;
 
         private DocumentTimerTask(String operationId) {
@@ -118,17 +123,21 @@ class EndpointResultQueue {
         public void run() {
             timeout(operationId);
         }
+
     }
 
-    private class TimerFuture {
+    private static class TimerFuture {
+
         private final ScheduledFuture<?> future;
 
         public TimerFuture(ScheduledFuture<?> future) {
             this.future = future;
         }
+
         private ScheduledFuture<?> getFuture() {
             return future;
         }
+
     }
 
 }
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
index 0d916002964..4853a2592e1 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
@@ -31,7 +31,7 @@ import java.util.logging.Logger;
  */
 class IOThread implements Runnable, AutoCloseable {
 
-    private static Logger log = Logger.getLogger(IOThread.class.getName());
+    private static final Logger log = Logger.getLogger(IOThread.class.getName());
     private final Endpoint endpoint;
     private final GatewayConnection client;
     private final DocumentQueue documentQueue;
@@ -76,8 +76,7 @@ class IOThread implements Runnable, AutoCloseable {
         this.maxChunkSizeBytes = maxChunkSizeBytes;
         this.maxInFlightRequests = maxInFlightRequests;
         this.gatewayThrottler = new GatewayThrottler(maxSleepTimeMs);
-        //Ensure that pollInterval is in the range [1us, 10s]
-        this.pollIntervalUS = Math.max(1, (long)(1000000.0/Math.max(0.1, idlePollFrequency)));
+        this.pollIntervalUS = Math.max(1, (long)(1000000.0/Math.max(0.1, idlePollFrequency))); // ensure range [1us, 10s]
         this.thread = new Thread(ioThreadGroup, this, "IOThread " + endpoint);
         thread.setDaemon(true);
         this.localQueueTimeOut = localQueueTimeOut;
@@ -417,7 +416,7 @@ class IOThread implements Runnable, AutoCloseable {
     }
 
     private void drainDocumentQueueWhenFailingPermanently(Exception exception) {
-        //first, clear sentOperations:
+        // first, clear sentOperations:
         resultQueue.failPending(exception);
 
         for (Document document : documentQueue.removeAllDocuments()) {