Adaptive federation timeout

9 files changed, 277 insertions, 260 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java b/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java
index 8ea62c30692..85f1a185acf 100644
--- a/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java
+++ b/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java
@@ -1,71 +1,117 @@
 package com.yahoo.search.federation;
 
 import com.google.common.collect.ImmutableList;
+import com.yahoo.search.Result;
 import com.yahoo.search.searchchain.FutureResult;
-import com.yahoo.search.searchchain.model.federation.FederationOptions;
 
-import java.util.Collection;
+import java.time.Clock;
+import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 /**
- * The result of a federation to targets which knowd how to wait for the result from each target.
+ * The result of a federation to targets which knows how to wait for the result from each target.
+ * This thread handles multiple threads producing target results but only a single thread may use an instance of this.
  *
  * @author bratseth
  */
 class FederationResult {
 
+    /** All targets of this */
     private final List<TargetResult> targetResults;
     
+    /** 
+     * The remaining targets to wait for. 
+     * Other targets are either complete, or should only be included if they are available when we complete
+     */
+    private List<TargetResult> targetsToWaitFor;
+    
     private FederationResult(ImmutableList<TargetResult> targetResults) {
         this.targetResults = targetResults;
+
+        if (targetResults.stream().anyMatch(TargetResult::isMandatory))
+            targetsToWaitFor = targetResults.stream().filter(TargetResult::isMandatory).collect(Collectors.toList());
+        else
+            targetsToWaitFor = new ArrayList<>(targetResults);
     }
-    
-    public void waitForAll(long queryTimeout) {
-        waitForMandatoryTargets(queryTimeout); // For now ...
+
+    /**
+     * Wait on each target for that targets timeout
+     * On the worst case this is the same as waiting for the max target timeout,
+     * in the average case it may be much better because lower timeout sources do not get to
+     * drive the timeout above their own timeout value.
+     * When this completes, results can be accessed from the TargetResults with no blocking
+     * (i.e getOrTimeout) without breaking any contract.
+     */
+    public void waitForAll(int queryTimeout, Clock clock) {
+        long startTime = clock.millis();
+        while ( ! targetsToWaitFor.isEmpty()) {
+            TargetResult nextToWaitFor = targetWithSmallestTimeout(targetsToWaitFor, queryTimeout);
+            long timeLeftOfNextTimeout = nextToWaitFor.timeout(queryTimeout) - ( clock.millis() - startTime );
+            nextToWaitFor.getIfAvailable(timeLeftOfNextTimeout);
+            targetsToWaitFor.remove(nextToWaitFor);
+        }
     }
     
     /** Returns an immutable list of the results of this */
     public List<TargetResult> all() { return targetResults; }
-    
-    private void waitForMandatoryTargets(long queryTimeout) {
-        FutureWaiter futureWaiter = new FutureWaiter();
-
-        boolean hasMandatoryTargets = false;
-        for (TargetResult targetResult : targetResults) {
-            if (targetResult.isMandatory()) {
-                futureWaiter.add(targetResult.futureResult, targetResult.getSearchChainExecutionTimeoutMs(queryTimeout));
-                hasMandatoryTargets = true;
-            }
-        }
 
-        if ( ! hasMandatoryTargets) {
-            for (TargetResult targetResult : targetResults) {
-                futureWaiter.add(targetResult.futureResult,
-                                 targetResult.getSearchChainExecutionTimeoutMs(queryTimeout));
-            }
-        }
-
-        futureWaiter.waitForFutures();
+    private TargetResult targetWithSmallestTimeout(List<TargetResult> results, int queryTimeout) {
+        TargetResult smallest = null;
+        for (TargetResult result : results)
+            if (smallest == null || result.timeout(queryTimeout) < smallest.timeout(queryTimeout))
+                smallest = result;
+        return smallest;        
     }
-
+    
     static class TargetResult {
 
         final FederationSearcher.Target target;
-        final FutureResult futureResult;
+        private final FutureResult futureResult;
+
+        /** 
+         * Single threaded access to result already returned from futureResult, if any.
+         * To avoid unnecessary synchronization with the producer thread.
+         */
+        private Optional<Result> availableResult = Optional.empty();
 
         private TargetResult(FederationSearcher.Target target, FutureResult futureResult) {
             this.target = target;
             this.futureResult = futureResult;
         }
 
+        private boolean isMandatory() { return ! target.federationOptions().getOptional(); }
+
+        /**
+         * Returns the result of this by blocking until timeout if necessary. 
+         * 
+         * @return the result if available, or null otherwise
+         */
+        public Optional<Result> getIfAvailable(long timeout) {
+            if (availableResult.isPresent()) return availableResult;
+            availableResult = futureResult.getIfAvailable(timeout, TimeUnit.MILLISECONDS);
+            return availableResult;
+        }
+        
+        /** Returns a result without blocking; if the result is not available one with a timeout error is produced */
+        public Result getOrTimeoutError() {
+            // The else part is to offload creation of the timeout error
+            return getIfAvailable(0).orElse(futureResult.get(0, TimeUnit.MILLISECONDS));
+        }
+        
         public boolean successfullyCompleted() {
             return futureResult.isDone() && ! futureResult.isCancelled();
         }
 
-        private boolean isMandatory() { return ! target.federationOptions().getOptional(); }
-
-        private long getSearchChainExecutionTimeoutMs(long queryTimeout) {
-            return target.federationOptions().getSearchChainExecutionTimeoutInMilliseconds(queryTimeout);
+        private int timeout(long queryTimeout) {
+            return (int)target.federationOptions().getSearchChainExecutionTimeoutInMilliseconds(queryTimeout);
+        }
+        
+        @Override
+        public String toString() {
+            return "result for " + target;
         }
 
     }
@@ -84,27 +130,4 @@ class FederationResult {
         
     }
     
-    /** Returns the max mandatory timeout, or 0 if there are no mandatory sources */
-    /*
-    private long calculateMandatoryTimeout(Query query, Collection<FederationSearcher.Target> targets) {
-        long mandatoryTimeout = 0;
-        long queryTimeout = query.getTimeout();
-        for (FederationSearcher.Target target : targets) {
-            if (target.federationOptions().getOptional()) continue;
-            mandatoryTimeout = Math.min(mandatoryTimeout,
-                                        target.federationOptions().getSearchChainExecutionTimeoutInMilliseconds(queryTimeout));
-        }
-        return mandatoryTimeout;
-    }
-
-        if (query.requestHasProperty("timeout")) {
-        mandatoryTimeout = query.getTimeLeft();
-    } else {
-        mandatoryTimeout = calculateMandatoryTimeout(query, targetHandlers);
-    }
-
-        if (mandatoryTimeout < 0)
-            return new Result(query, ErrorMessage.createTimeout("Timed out when about to federate"));
-    
-    */
 }
diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java
index c10a7f600e3..621f0fbe090 100644
--- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java
+++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java
@@ -45,6 +45,7 @@ import static com.yahoo.collections.CollectionUtil.first;
 import static com.yahoo.container.util.Util.quote;
 import static com.yahoo.search.federation.StrictContractsConfig.PropagateSourceProperties;
 
+import java.time.Clock;
 import java.util.*;
 import java.util.Map.Entry;
 import java.util.concurrent.TimeUnit;
@@ -55,6 +56,7 @@ import java.util.logging.Logger;
  *
  * @author Arne Bergene Fossaa
  * @author tonytv
+ * @author bratseth
  */
 @Provides(FederationSearcher.FEDERATION)
 @After("*")
@@ -68,7 +70,6 @@ public class FederationSearcher extends ForkingSearcher {
     public final static CompoundName SOURCENAME = new CompoundName("sourceName");
     public final static CompoundName PROVIDERNAME = new CompoundName("providerName");
 
-
     /** Logging field name constants */
     public static final String LOG_COUNT_PREFIX = "count_";
 
@@ -80,6 +81,7 @@ public class FederationSearcher extends ForkingSearcher {
     private final boolean strictSearchchain;
     private final TargetSelector<?> targetSelector;
 
+    private final Clock clock = Clock.systemUTC();
 
     @Inject
     public FederationSearcher(FederationConfig config, StrictContractsConfig strict,
@@ -197,7 +199,7 @@ public class FederationSearcher extends ForkingSearcher {
 
     private void search(Query query, Execution execution, Collection<Target> targets, Result mergedResults) {
         FederationResult results = search(query, execution, targets);
-        results.waitForAll(query.getTimeLeft());
+        results.waitForAll((int)query.getTimeLeft(), clock);
 
         HitOrderer s = null;
         for (FederationResult.TargetResult targetResult : results.all()) {
@@ -205,10 +207,9 @@ public class FederationSearcher extends ForkingSearcher {
                 addSearchChainTimedOutError(query, targetResult.target.getId());
             } else {
                 if (s == null) {
-                    s = dirtyCopyIfModifiedOrderer(mergedResults.hits(), targetResult.futureResult.get().hits().getOrderer());
+                    s = dirtyCopyIfModifiedOrderer(mergedResults.hits(), targetResult.getOrTimeoutError().hits().getOrderer());
                 }
-                mergeResult(query, targetResult.target, mergedResults, targetResult.futureResult.get());
-
+                mergeResult(query, targetResult.target, mergedResults, targetResult.getOrTimeoutError());
             }
         }
     }
diff --git a/container-search/src/main/java/com/yahoo/search/federation/FutureWaiter.java b/container-search/src/main/java/com/yahoo/search/federation/FutureWaiter.java
deleted file mode 100644
index af90d015621..00000000000
--- a/container-search/src/main/java/com/yahoo/search/federation/FutureWaiter.java
+++ /dev/null
@@ -1,60 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.search.federation;
-
-import com.yahoo.search.searchchain.FutureResult;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.List;
-import java.util.concurrent.TimeUnit;
-
-/**
- * @author tonytv
- */
-class FutureWaiter {
-
-    private class Future {
-        final FutureResult result;
-        final long timeoutInMilliseconds;
-
-        public Future(FutureResult result, long timeoutInMilliseconds) {
-            this.result = result;
-            this.timeoutInMilliseconds = timeoutInMilliseconds;
-        }
-    }
-
-    private List<Future> futures = new ArrayList<>();
-
-    public void add(FutureResult futureResult, long timeoutInMilliseconds) {
-        futures.add(new Future(futureResult, timeoutInMilliseconds));
-    }
-
-    public void waitForFutures() {
-        sortFuturesByTimeoutDescending();
-
-        long startTime = System.currentTimeMillis();
-
-        for (Future future : futures) {
-            long timeToWait = startTime + future.timeoutInMilliseconds - System.currentTimeMillis();
-            if (timeToWait <= 0)
-                break;
-
-            future.result.get(timeToWait, TimeUnit.MILLISECONDS);
-        }
-    }
-
-    private void sortFuturesByTimeoutDescending() {
-        Collections.sort(futures, new Comparator<Future>() {
-            @Override
-            public int compare(Future lhs, Future rhs) {
-                return -compareLongs(lhs.timeoutInMilliseconds, rhs.timeoutInMilliseconds);
-            }
-
-            private int compareLongs(long lhs, long rhs) {
-                return new Long(lhs).compareTo(rhs);
-            }
-        });
-    }
-
-}
diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/AsyncExecution.java b/container-search/src/main/java/com/yahoo/search/searchchain/AsyncExecution.java
index e1794a73a93..739337add14 100644
--- a/container-search/src/main/java/com/yahoo/search/searchchain/AsyncExecution.java
+++ b/container-search/src/main/java/com/yahoo/search/searchchain/AsyncExecution.java
@@ -40,7 +40,7 @@ import java.util.concurrent.*;
  * </p>
  *
  * @see com.yahoo.search.searchchain.Execution
- * @author <a href="mailto:arnebef@yahoo-inc.com">Arne Bergene Fossaa</a>
+ * @author Arne Bergene Fossaa
  */
 public class AsyncExecution {
 
@@ -174,9 +174,8 @@ public class AsyncExecution {
      * collection
      */
     public static List<Result> waitForAll(Collection<FutureResult> tasks, long timeoutMs) {
-
         // Copy the list in case it is modified while we are waiting
-        final List<FutureResult> workingTasks = new ArrayList<>(tasks);
+        List<FutureResult> workingTasks = new ArrayList<>(tasks);
         try {
             runTask(() -> {
                 for (FutureResult task : workingTasks)
@@ -186,15 +185,13 @@ public class AsyncExecution {
             // Handle timeouts below
         }
 
-        final List<Result> results = new ArrayList<>(tasks.size());
+        List<Result> results = new ArrayList<>(tasks.size());
         for (FutureResult atask : workingTasks) {
             Result result;
             if (atask.isDone() && !atask.isCancelled()) {
-                result = atask.get(); // Since isDone() = true, this won't
-                                      // block.
+                result = atask.get(); // Since isDone() = true, this won't block.
             } else { // Not done and no errors thrown
-                result = new Result(atask.getQuery(),
-                        atask.createTimeoutError());
+                result = new Result(atask.getQuery(), atask.createTimeoutError());
             }
             results.add(result);
         }
diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/FutureResult.java b/container-search/src/main/java/com/yahoo/search/searchchain/FutureResult.java
index 877252f07e6..ec87305da3b 100644
--- a/container-search/src/main/java/com/yahoo/search/searchchain/FutureResult.java
+++ b/container-search/src/main/java/com/yahoo/search/searchchain/FutureResult.java
@@ -1,6 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.searchchain;
 
+import java.util.Optional;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.FutureTask;
@@ -16,6 +17,8 @@ import com.yahoo.search.result.ErrorMessage;
 
 /**
  * Extends a {@code FutureTask<Result>}, with some added error handling
+ * 
+ * @author bratseth
  */
 public class FutureResult extends FutureTask<Result> {
 
@@ -26,51 +29,56 @@ public class FutureResult extends FutureTask<Result> {
 
     private final static Logger log = Logger.getLogger(FutureResult.class.getName());
 
-    FutureResult(Callable<Result> callable, Execution execution, final Query query) {
+    protected FutureResult(Callable<Result> callable, Execution execution, Query query) {
         super(callable);
         this.query = query;
         this.execution = execution;
     }
 
+    /** 
+     * Returns a Result containing the hits returned from this source, or an error otherwise.
+     * This will block for however long it takes to get the result: Using this is a bad idea.
+     */
     @Override
     public Result get() {
-        Result result;
         try {
-            result = super.get();
+            return super.get();
         }
         catch (InterruptedException e) {
-            result = new Result(getQuery(), ErrorMessage.createUnspecifiedError(
-                    "'" + execution + "' was interrupted while executing: " + Exceptions.toMessageString(e)));
+            return new Result(getQuery(), createInterruptedError(e));
         }
         catch (ExecutionException e) {
-            log.log(Level.WARNING,"Exception on executing " + execution + " for " + query,e);
-            result = new Result(getQuery(), ErrorMessage.createErrorInPluginSearcher(
-                    "Error in '" + execution + "': " + Exceptions.toMessageString(e),
-                    e.getCause()));
+            return new Result(getQuery(), createExecutionError(e));
         }
-        return result;
     }
 
+    /** 
+     * Returns a Result containing the hits returned from this source, or an error otherwise.
+     * This blocks for at most the given timeout and returns a Result containing a timeout error
+     * if the result is not available within this time.
+     */
     @Override
     public Result get(long timeout, TimeUnit timeunit) {
-        Result result;
+        return getIfAvailable(timeout, timeunit).orElse(new Result(getQuery(), createTimeoutError()));
+    }
+
+    /**
+     * Same as get(timeout, timeunit) but returns Optiona.empty instead of a result with error if the result is 
+     * not available in time
+     */
+    public Optional<Result> getIfAvailable(long timeout, TimeUnit timeunit) {
         try {
-            result = super.get(timeout, timeunit);
+            return Optional.of(super.get(timeout, timeunit));
         }
         catch (InterruptedException e) {
-            result = new Result(getQuery(), ErrorMessage.createUnspecifiedError(
-                    "'" + execution + "' was interrupted while executing: " + Exceptions.toMessageString(e)));
+            return Optional.of(new Result(getQuery(), createInterruptedError(e)));
         }
         catch (ExecutionException e) {
-            log.log(Level.WARNING,"Exception on executing " + execution + " for " + query, e);
-            result = new Result(getQuery(), ErrorMessage.createErrorInPluginSearcher(
-                    "Error in '" + execution + "': " + Exceptions.toMessageString(e),
-                    e.getCause()));
+            return Optional.of(new Result(getQuery(), createExecutionError(e)));
         }
         catch (TimeoutException e) {
-            result = new Result(getQuery(), createTimeoutError());
+            return Optional.empty();
         }
-        return result;
     }
 
     /** Returns the query used in this execution, never null */
@@ -78,9 +86,19 @@ public class FutureResult extends FutureTask<Result> {
         return query;
     }
 
-    ErrorMessage createTimeoutError() {
-        return ErrorMessage.createTimeout(
-                "Error executing '" + execution + "': " + " Chain timed out.");
+    private ErrorMessage createInterruptedError(Exception e) {
+        return ErrorMessage.createUnspecifiedError("'" + execution + "' was interrupted while executing: " + 
+                                                   Exceptions.toMessageString(e));
+    }
+    
+    private ErrorMessage createExecutionError(Exception e) {
+        log.log(Level.WARNING,"Exception on executing " + execution + " for " + query,e);
+        return ErrorMessage.createErrorInPluginSearcher("Error in '" + execution + "': " + Exceptions.toMessageString(e),
+                                                        e.getCause());
+    }
 
+    ErrorMessage createTimeoutError() {
+        return ErrorMessage.createTimeout("Error executing '" + execution + "': " + " Chain timed out.");
     }
+
 }
diff --git a/container-search/src/test/java/com/yahoo/search/federation/FederationResultTest.java b/container-search/src/test/java/com/yahoo/search/federation/FederationResultTest.java
new file mode 100644
index 00000000000..eb17cf27db9
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/federation/FederationResultTest.java
@@ -0,0 +1,146 @@
+package com.yahoo.search.federation;
+
+import com.google.common.collect.ImmutableSet;
+import com.yahoo.component.chain.Chain;
+import com.yahoo.search.Query;
+import com.yahoo.search.Result;
+import com.yahoo.search.Searcher;
+import com.yahoo.search.result.ErrorMessage;
+import com.yahoo.search.searchchain.Execution;
+import com.yahoo.search.searchchain.FutureResult;
+import com.yahoo.search.searchchain.model.federation.FederationOptions;
+import org.junit.Test;
+
+import java.time.Clock;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author bratseth
+ */
+public class FederationResultTest {
+    
+    private static final FederationSearcher.Target organic = new MockTarget("organic", 250);
+    private static final FederationSearcher.Target dsp1 = new MockTarget("dsp1", 120);
+    private static final FederationSearcher.Target dsp2 = new MockTarget("dsp2", 100);
+
+    private final Clock clock = Clock.systemUTC();
+
+    @Test
+    public void testFederationResult() {
+        assertTimeout(ImmutableSet.of(),                50, 100,  90);
+        assertTimeout(ImmutableSet.of(),               240, 200, 200);
+        assertTimeout(ImmutableSet.of("dsp1"),         130, 140, 110);
+        assertTimeout(ImmutableSet.of("organic"),      260,  80,  80);
+        assertTimeout(ImmutableSet.of("dsp2"),         100, 110, 115);
+        assertTimeout(ImmutableSet.of(),               100, 110, 105);
+        assertTimeout(ImmutableSet.of("dsp1", "dsp2"), 100, 130, 130);
+        assertTimeout(ImmutableSet.of("organic"),      260, 130, 130);
+    }
+    
+    private void assertTimeout(Set<String> expectedTimeoutNames, int ... responseTimes) {
+        FederationResult.Builder builder = new FederationResult.Builder();
+        builder.add(organic, resultAfter(responseTimes[0]));
+        builder.add(dsp1,    resultAfter(responseTimes[1]));
+        builder.add(dsp2,    resultAfter(responseTimes[2]));
+        FederationResult federationResult = builder.build();
+        federationResult.waitForAll(50, clock);
+        assertEquals(3, federationResult.all().size());
+        for (FederationResult.TargetResult targetResult : federationResult.all()) {
+            Result result = targetResult.getOrTimeoutError();
+            if (expectedTimeoutNames.contains(targetResult.target.getId().toString()))
+                assertTrue(targetResult.target.getId() + " timed out", timedOut(result));
+            else
+                assertTrue(targetResult.target.getId() + " did not time out", ! timedOut(result));
+        }
+    }
+    
+    private MockFutureResult resultAfter(int time) {
+        return new MockFutureResult(new Query(), time);        
+    }
+    
+    private boolean timedOut(Result result) {
+        ErrorMessage error = result.hits().getError();
+        if (error == null) return false;
+        return error.getCode() == ErrorMessage.timeoutCode;
+    }
+
+    private class MockFutureResult extends FutureResult {
+        
+        private final int responseTime;
+        private final Query query;
+        private final long startTime;
+        
+        MockFutureResult(Query query, int responseTime) {
+            super(() -> new Result(query), new Execution(Execution.Context.createContextStub()), query);
+            this.responseTime = responseTime;
+            this.query = query;
+            startTime = clock.millis();
+        }
+
+        @Override
+        public Result get() { throw new RuntimeException(); }
+
+        @Override
+        public Optional<Result> getIfAvailable(long timeout, TimeUnit timeunit) {
+            if (timeunit != TimeUnit.MILLISECONDS) throw new RuntimeException();
+
+            long elapsedTime = clock.millis() - startTime;
+            long leftUntilResponse = responseTime - elapsedTime;
+            if (leftUntilResponse > timeout) {
+                sleepUntil(timeout);
+                return Optional.empty();
+            }
+            else {
+                sleepUntil(leftUntilResponse);
+                return Optional.of(new Result(query));
+            }
+        }
+        
+        private void sleepUntil(long time) {
+            if (time <= 0) return;
+            try {
+                Thread.sleep(time);
+            }
+            catch (InterruptedException e) {
+            }
+        }
+
+        @Override
+        public Query getQuery() {
+            return query;
+        }
+
+    }
+    
+    private static class MockTarget extends FederationSearcher.Target {
+
+        private final Chain<Searcher> chain;
+        private final int timeout;
+        
+        MockTarget(String id, int timeout) {
+            this.chain = new Chain<>(id);
+            this.timeout = timeout;
+        }
+
+        @Override
+        Chain<Searcher> getChain() { return chain; }
+
+        @Override
+        void modifyTargetQuery(Query query) { }
+
+        @Override
+        void modifyTargetResult(Result result) { }
+
+        @Override
+        public FederationOptions federationOptions() {
+            return new FederationOptions(false, timeout, true);
+        }
+
+    }
+    
+}
diff --git a/container-search/src/test/java/com/yahoo/search/federation/FutureWaiterTest.java b/container-search/src/test/java/com/yahoo/search/federation/FutureWaiterTest.java
deleted file mode 100644
index 37969e12399..00000000000
--- a/container-search/src/test/java/com/yahoo/search/federation/FutureWaiterTest.java
+++ /dev/null
@@ -1,109 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.search.federation;
-
-
-/**
- * @author tonytv
- */
-// TODO: Fix or remove!
-public class FutureWaiterTest {
-
-/*
-
-    @MockClass(realClass = System.class)
-    public static class MockSystem {
-
-        private static long currentTime;
-        private static boolean firstTime;
-
-        private static final long startTime = 123;
-
-        @Mock
-        public static synchronized  long currentTimeMillis() {
-            if (firstTime) {
-                firstTime = false;
-                return startTime;
-            }
-            return currentTime;
-        }
-
-        static synchronized void setElapsedTime(long elapsedTime) {
-            firstTime = true;
-            currentTime = elapsedTime + startTime;
-        }
-    }
-
-    @Mocked()
-    FutureResult result1;
-
-    @Mocked()
-    FutureResult result2;
-
-    @Mocked()
-    FutureResult result3;
-
-    @Mocked()
-    FutureResult result4;
-
-    @Before
-    public void before() {
-        Mockit.setUpMock(FutureWaiterTest.MockSystem.class);
-    }
-
-    @After
-    public void after() {
-        Mockit.tearDownMocks();
-    }
-
-    @Test
-    public void require_time_to_wait_is_adjusted_for_elapsed_time() {
-        MockSystem.setElapsedTime(300);
-
-        FutureWaiter futureWaiter = new FutureWaiter();
-        futureWaiter.add(result1, 350);
-        futureWaiter.waitForFutures();
-
-        new FullVerifications() {
-            {
-                result1.get(350 - 300, TimeUnit.MILLISECONDS);
-            }
-        };
-    }
-
-    @Test
-    public void require_do_not_wait_for_expired_timeouts() {
-        MockSystem.setElapsedTime(300);
-
-        FutureWaiter futureWaiter = new FutureWaiter();
-        futureWaiter.add(result1, 300);
-        futureWaiter.add(result2, 290);
-
-        futureWaiter.waitForFutures();
-
-        new FullVerifications() {
-            {}
-        };
-    }
-
-    @Test
-    public void require_wait_for_largest_timeout_first() throws InterruptedException {
-        MockSystem.setElapsedTime(600);
-
-        FutureWaiter futureWaiter = new FutureWaiter();
-        futureWaiter.add(result1, 500);
-        futureWaiter.add(result4, 800);
-        futureWaiter.add(result2, 600);
-        futureWaiter.add(result3, 700);
-
-        futureWaiter.waitForFutures();
-
-        new FullVerifications() {
-            {
-                result4.get(800 - 600, TimeUnit.MILLISECONDS);
-                result3.get(700 - 600, TimeUnit.MILLISECONDS);
-            }
-        };
-    }
-
-    */
-}
diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
index e4176bb6679..11fae387739 100644
--- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
@@ -38,7 +38,7 @@ import static com.yahoo.search.federation.StrictContractsConfig.PropagateSourceP
  * Test for federation searcher. The searcher is also tested in
  * com.yahoo.prelude.searcher.test.BlendingSearcherTestCase.
  *
- * @author <a href="mailto:arnebef@yahoo-inc.com">Arne Bergene Fossaa</a>
+ * @author Arne Bergene Fossaa
  */
 @SuppressWarnings("deprecation")
 public class FederationSearcherTestCase {
diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java
index 7b0451a01ba..7aa73c64d3a 100644
--- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java
+++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationTester.java
@@ -18,6 +18,7 @@ import java.util.Collections;
 * @author tonytv
 */
 class FederationTester {
+
     SearchChainResolver.Builder builder = new SearchChainResolver.Builder();
     SearchChainRegistry registry = new SearchChainRegistry();