From 500469fd283680ff49eff3da501f78b6dd57f7e0 Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Wed, 25 Sep 2013 14:16:11 +0200
Subject: [PATCH] Improve EngineSearcher tracking

Currently we fail tests is any searcher reference is pending. Yet,
on a slow machine the freeContext calls that are async could still be
in flight so if there are pending searchers we wait for a bit to make
sure we don't fail if a freeContext call is in flight.
The MockEngine now also contains the stack trace of the first close call
if a searcher is closed twice.
---
 .../test/ElasticSearchTestCase.java           | 21 +++++++++++++++----
 .../test/engine/MockRobinEngine.java          | 13 +++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/test/java/org/elasticsearch/test/ElasticSearchTestCase.java b/src/test/java/org/elasticsearch/test/ElasticSearchTestCase.java
index cd5aed83b1a..26471140fdd 100644
--- a/src/test/java/org/elasticsearch/test/ElasticSearchTestCase.java
+++ b/src/test/java/org/elasticsearch/test/ElasticSearchTestCase.java
@@ -49,11 +49,11 @@ public abstract class ElasticSearchTestCase extends AbstractRandomizedTest {
 
     public static final String CHILD_VM_ID = System.getProperty("junit4.childvm.id", "" + System.currentTimeMillis());
     
-    public boolean awaitBusy(Predicate<?> breakPredicate) throws InterruptedException {
+    public static boolean awaitBusy(Predicate<?> breakPredicate) throws InterruptedException {
         return awaitBusy(breakPredicate, 10, TimeUnit.SECONDS);
     }
     
-    public boolean awaitBusy(Predicate<?> breakPredicate, long maxWaitTime, TimeUnit unit) throws InterruptedException {
+    public static boolean awaitBusy(Predicate<?> breakPredicate, long maxWaitTime, TimeUnit unit) throws InterruptedException {
         long maxTimeInMillis = TimeUnit.MILLISECONDS.convert(maxWaitTime, unit);
         long iterations = Math.max(Math.round(Math.log10(maxTimeInMillis) / Math.log10(2)), 1);
         long timeInMillis = 1;
@@ -102,8 +102,21 @@ public abstract class ElasticSearchTestCase extends AbstractRandomizedTest {
     }
     
     public static void ensureAllSearchersClosed() {
-        if (MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.isEmpty()) {
-            return;
+        /* in some cases we finish a test faster than the freeContext calls make it to the
+         * shards. Let's wait for some time if there are still searchers. If the are really 
+         * pending we will fail anyway.*/
+        try {
+            if (awaitBusy(new Predicate<Object>() {
+                public boolean apply(Object o) {
+                    return MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.isEmpty();
+                }
+            }, 5, TimeUnit.SECONDS)) {
+                return;
+            }
+        } catch (InterruptedException ex) {
+            if (MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.isEmpty()) {
+                return;
+            }
         }
         try {
             RuntimeException ex = null;
diff --git a/src/test/java/org/elasticsearch/test/engine/MockRobinEngine.java b/src/test/java/org/elasticsearch/test/engine/MockRobinEngine.java
index 732a3f2ec0f..5d8e8b56172 100644
--- a/src/test/java/org/elasticsearch/test/engine/MockRobinEngine.java
+++ b/src/test/java/org/elasticsearch/test/engine/MockRobinEngine.java
@@ -95,6 +95,8 @@ public final class MockRobinEngine extends RobinEngine implements Engine {
     public static final class AssertingSearcher implements Searcher {
         private final Searcher searcher;
         private final ShardId shardId;
+        private RuntimeException firstReleaseStack;
+        private final Object lock = new Object();
 
         public AssertingSearcher(Searcher searcher, ShardId shardId) {
             this.searcher = searcher;
@@ -110,7 +112,16 @@ public final class MockRobinEngine extends RobinEngine implements Engine {
         @Override
         public boolean release() throws ElasticSearchException {
             RuntimeException remove = INFLIGHT_ENGINE_SEARCHERS.remove(this);
-            assert remove != null : "Released Searcher more than once, source [" + searcher.source() + "]";
+            synchronized (lock) { 
+                // make sure we only get this once and store the stack of the first caller!
+                if (remove == null) {
+                    assert firstReleaseStack != null;
+                    throw new AssertionError("Released Searcher more than once, source [" + searcher.source() + "]", firstReleaseStack);
+                } else {
+                    assert firstReleaseStack == null;
+                    firstReleaseStack = new RuntimeException("Searcher Released first here, source [" + searcher.source() + "]");
+                }
+            }
             return searcher.release();
         }