Add asserting searcher to track searcher references

This assertion module also injects an AssertingIndexSearcher that checks if our queries are all compliant with the lucene specification which is improtant for future updates and changes in the upstream project.
lmenezes · Sep 19, 2013 · 04deb80 · 04deb80
1 parent bb4f30c
commit 04deb80
Show file tree

Hide file tree

Showing 16 changed files with 273 additions and 82 deletions.
diff --git a/src/main/java/org/elasticsearch/index/engine/IndexEngineModule.java b/src/main/java/org/elasticsearch/index/engine/IndexEngineModule.java
@@ -36,6 +36,7 @@ public class IndexEngineModule extends AbstractModule implements SpawnModules {
 
     public static final class EngineSettings {
         public static final String ENGINE_TYPE = "index.engine.type";
+        public static final String INDEX_ENGINE_TYPE = "index.index_engine.type";
         public static final Class<? extends Module> DEFAULT_INDEX_ENGINE = RobinIndexEngineModule.class;
         public static final Class<? extends Module> DEFAULT_ENGINE = RobinEngineModule.class;
     }
@@ -48,7 +49,7 @@ public IndexEngineModule(Settings settings) {
 
     @Override
     public Iterable<? extends Module> spawnModules() {
-        return ImmutableList.of(createModule(settings.getAsClass(EngineSettings.ENGINE_TYPE, EngineSettings.DEFAULT_INDEX_ENGINE, "org.elasticsearch.index.engine.", "IndexEngineModule"), settings));
+        return ImmutableList.of(createModule(settings.getAsClass(EngineSettings.INDEX_ENGINE_TYPE, EngineSettings.DEFAULT_INDEX_ENGINE, "org.elasticsearch.index.engine.", "IndexEngineModule"), settings));
     }
 
     @Override

diff --git a/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java b/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java
@@ -676,16 +676,20 @@ public void delete(DeleteByQuery delete) throws EngineException {
     }
 
     @Override
-    public Searcher searcher() throws EngineException {
+    public final Searcher searcher() throws EngineException {
         SearcherManager manager = this.searcherManager;
         try {
             IndexSearcher searcher = manager.acquire();
-            return new RobinSearcher(searcher, manager);
+            return newSearcher(searcher, manager);
         } catch (IOException ex) {
             logger.error("failed to accquire searcher for shard [{}]", ex, shardId);
             throw new EngineException(shardId, ex.getMessage());
         }
     }
+
+    protected Searcher newSearcher(IndexSearcher searcher, SearcherManager manager) {
+        return new RobinSearcher(searcher, manager);
+    }
 
     @Override
     public boolean refreshNeeded() {

diff --git a/src/test/java/org/apache/lucene/util/AbstractRandomizedTest.java b/src/test/java/org/apache/lucene/util/AbstractRandomizedTest.java
@@ -19,7 +19,10 @@
 
 package org.apache.lucene.util;
 
-import com.carrotsearch.randomizedtesting.*;
+import com.carrotsearch.randomizedtesting.JUnit4MethodProvider;
+import com.carrotsearch.randomizedtesting.LifecycleScope;
+import com.carrotsearch.randomizedtesting.RandomizedContext;
+import com.carrotsearch.randomizedtesting.RandomizedTest;
 import com.carrotsearch.randomizedtesting.annotations.Listeners;
 import com.carrotsearch.randomizedtesting.annotations.TestGroup;
 import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders;
@@ -59,7 +62,6 @@
 // NOTE: this class is in o.a.lucene.util since it uses some classes that are related
 // to the test framework that didn't make sense to copy but are package private access
 public abstract class AbstractRandomizedTest extends RandomizedTest {
-
     /**
      * Annotation for integration tests
      */
@@ -305,7 +307,6 @@ protected boolean verify(Method key) {
     @Before
     public void setUp() throws Exception {
         parentChainCallRule.setupCalled = true;
-        currentSeed = SeedUtils.parseSeed(getContext().getRunnerSeedAsString());
     }
 
     /**
@@ -314,7 +315,6 @@ public void setUp() throws Exception {
     @After
     public void tearDown() throws Exception {
         parentChainCallRule.teardownCalled = true;
-        currentSeed = null;
     }
 
 
@@ -355,10 +355,4 @@ public static Class<?> getTestClass() {
     public String getTestName() {
         return threadAndTestNameRule.testMethodName;
     }
-
-    private static volatile Long currentSeed;
-
-    public static Long getCurrentSeed() {
-        return currentSeed;
-    }
 }
diff --git a/src/test/java/org/elasticsearch/AbstractSharedClusterTest.java b/src/test/java/org/elasticsearch/AbstractSharedClusterTest.java
@@ -103,6 +103,7 @@ public final void before() {
         cluster.beforeTest(getRandom());
         wipeIndices();
         wipeTemplates();
+        randomIndexTemplate();
         logger.info("[{}#{}]: before test", getTestClass().getSimpleName(), getTestName());
     }
 
@@ -116,6 +117,7 @@ public void after() throws IOException {
                 .persistentSettings().getAsMap().size(), equalTo(0));
         wipeIndices(); // wipe after to make sure we fail in the test that didn't ack the delete
         wipeTemplates();
+        ensureAllSearchersClosed();
         ensureAllFilesClosed();
         logger.info("[{}#{}]: cleaned up after test", getTestClass().getSimpleName(), getTestName());
     }
@@ -135,6 +137,15 @@ public static void afterClass() {
     public static Client client() {
         return cluster().client();
     }
+
+    private static void randomIndexTemplate() {
+        client().admin().indices().preparePutTemplate("random_index_template")
+        .setTemplate("*")
+        .setOrder(0)
+        .setSettings(ImmutableSettings.builder()
+                .put(INDEX_SEED_SETTING, getRandom().nextLong()))
+                .execute().actionGet();
+    }
 
     public static Iterable<Client> clients() {
         return cluster().clients();

diff --git a/src/test/java/org/elasticsearch/ElasticsearchTestCase.java b/src/test/java/org/elasticsearch/ElasticsearchTestCase.java
@@ -29,12 +29,14 @@
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.index.store.mock.MockDirectoryHelper;
 import org.elasticsearch.junit.listeners.LoggingListener;
+import org.elasticsearch.test.engine.MockRobinEngine;
 import org.junit.BeforeClass;
 
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
+import java.util.Map.Entry;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 
@@ -49,6 +51,7 @@ public abstract class ElasticsearchTestCase extends AbstractRandomizedTest {
     public static final String CHILD_VM_ID = System.getProperty("junit4.childvm.id", "" + System.currentTimeMillis());
 
     public static final long SHARED_CLUSTER_SEED = clusterSeed();
+    public static final String INDEX_SEED_SETTING = "index.tests.seed";
 
     private static long clusterSeed() {
         String property = System.getProperty("tests.cluster_seed");
@@ -111,6 +114,24 @@ public static void ensureAllFilesClosed() throws IOException {
         }
     }
 
+    public static void ensureAllSearchersClosed() {
+        if (MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.isEmpty()) {
+            return;
+        }
+        try {
+            RuntimeException ex = null;
+            StringBuilder builder = new StringBuilder("Unclosed Searchers instance for shards: [");
+            for (Entry<MockRobinEngine.AssertingSearcher, RuntimeException> entry : MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.entrySet()) {
+                ex = entry.getValue();
+                builder.append(entry.getKey().shardId()).append(",");
+            }
+            builder.append("]");
+            throw new RuntimeException(builder.toString(), ex);
+        } finally {
+            MockRobinEngine.INFLIGHT_ENGINE_SEARCHERS.clear();
+        }
+    }
+
     public static void forceClearMockWrappers() {
         MockDirectoryHelper.wrappers.clear();
     }
@@ -133,5 +154,12 @@ public void close() throws IOException {
                 ensureAllFilesClosed();
             }
         });
+
+        closeAfterSuite(new Closeable() {
+            @Override
+            public void close() throws IOException {
+                ensureAllSearchersClosed();
+            }
+        });
     }
   }
diff --git a/src/test/java/org/elasticsearch/TestCluster.java b/src/test/java/org/elasticsearch/TestCluster.java
@@ -40,9 +40,11 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.index.engine.IndexEngineModule;
 import org.elasticsearch.index.store.mock.MockFSIndexStoreModule;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.internal.InternalNode;
+import org.elasticsearch.test.engine.MockEngineModule;
 import org.elasticsearch.transport.TransportService;
 
 import java.io.Closeable;
@@ -114,6 +116,7 @@ private TestCluster(long clusterSeed, String clusterName, Settings defaultSettin
                 /* use RAM directories in 10% of the runs */
 //                .put("index.store.type", random.nextInt(10) == 0 ? MockRamIndexStoreModule.class.getName() : MockFSIndexStoreModule.class.getName())
                 .put("index.store.type", MockFSIndexStoreModule.class.getName()) // no RAM dir for now!
+                .put(IndexEngineModule.EngineSettings.ENGINE_TYPE, MockEngineModule.class.getName())
                 .put(defaultSettings)
                 .put("cluster.name", clusterName).build();
     }

diff --git a/src/test/java/org/elasticsearch/deleteByQuery/DeleteByQueryTests.java b/src/test/java/org/elasticsearch/deleteByQuery/DeleteByQueryTests.java
@@ -39,7 +39,7 @@ public class DeleteByQueryTests extends AbstractSharedClusterTest {
     public void testDeleteAllNoIndices() {
         client().admin().indices().prepareDelete().execute().actionGet();
         client().admin().indices().prepareRefresh().execute().actionGet();
-        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = new DeleteByQueryRequestBuilder(client());
+        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = client().prepareDeleteByQuery();
         deleteByQueryRequestBuilder.setQuery(QueryBuilders.matchAllQuery());
         DeleteByQueryResponse actionGet = deleteByQueryRequestBuilder.execute().actionGet();
         assertThat(actionGet.getIndices().size(), equalTo(0));
@@ -55,7 +55,7 @@ public void testDeleteAllOneIndex() {
 
         SearchResponse search = client().prepareSearch().setQuery(QueryBuilders.matchAllQuery()).execute().actionGet();
         assertThat(search.getHits().totalHits(), equalTo(1l));
-        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = new DeleteByQueryRequestBuilder(client());
+        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = client().prepareDeleteByQuery();
         deleteByQueryRequestBuilder.setQuery(QueryBuilders.matchAllQuery());
 
         DeleteByQueryResponse actionGet = deleteByQueryRequestBuilder.execute().actionGet();
@@ -78,7 +78,7 @@ public void testMissing() {
 
         SearchResponse search = client().prepareSearch().setQuery(QueryBuilders.matchAllQuery()).execute().actionGet();
         assertThat(search.getHits().totalHits(), equalTo(1l));
-        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = new DeleteByQueryRequestBuilder(client());
+        DeleteByQueryRequestBuilder deleteByQueryRequestBuilder = client().prepareDeleteByQuery();
         deleteByQueryRequestBuilder.setIndices("twitter", "missing");
         deleteByQueryRequestBuilder.setQuery(QueryBuilders.matchAllQuery());
 

diff --git a/src/test/java/org/elasticsearch/index/engine/robin/RobinEngineTests.java b/src/test/java/org/elasticsearch/index/engine/robin/RobinEngineTests.java
@@ -27,6 +27,7 @@
 import org.apache.lucene.index.IndexDeletionPolicy;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.TermQuery;
+import org.elasticsearch.ElasticsearchTestCase;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.lucene.Lucene;
@@ -39,6 +40,7 @@
 import org.elasticsearch.index.deletionpolicy.KeepOnlyLastDeletionPolicy;
 import org.elasticsearch.index.deletionpolicy.SnapshotDeletionPolicy;
 import org.elasticsearch.index.deletionpolicy.SnapshotIndexCommit;
+import org.elasticsearch.index.deletionpolicy.SnapshotIndexCommitExistsMatcher;
 import org.elasticsearch.index.engine.*;
 import org.elasticsearch.index.indexing.ShardIndexingService;
 import org.elasticsearch.index.indexing.slowlog.ShardSlowLogIndexingService;
@@ -57,11 +59,8 @@
 import org.elasticsearch.index.store.distributor.LeastUsedDistributor;
 import org.elasticsearch.index.store.ram.RamDirectoryService;
 import org.elasticsearch.index.translog.Translog;
-import org.elasticsearch.index.translog.fs.FsTranslog;
-import org.elasticsearch.ElasticsearchTestCase;
-import org.elasticsearch.index.deletionpolicy.SnapshotIndexCommitExistsMatcher;
-import org.elasticsearch.index.engine.EngineSearcherTotalHitsMatcher;
 import org.elasticsearch.index.translog.TranslogSizeMatcher;
+import org.elasticsearch.index.translog.fs.FsTranslog;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.hamcrest.MatcherAssert;
 import org.junit.After;
@@ -321,11 +320,12 @@ public void testSimpleOperations() throws Exception {
         assertThat(getResult.exists(), equalTo(true));
         assertThat(getResult.source().source.toBytesArray(), equalTo(B_1.toBytesArray()));
         assertThat(getResult.docIdAndVersion(), nullValue());
-
+        getResult.release();
+
         // but, not there non realtime
         getResult = engine.get(new Engine.Get(false, newUid("1")));
         assertThat(getResult.exists(), equalTo(false));
-
+        getResult.release();
         // refresh and it should be there
         engine.refresh(new Engine.Refresh().force(false));
 
@@ -339,7 +339,8 @@ public void testSimpleOperations() throws Exception {
         getResult = engine.get(new Engine.Get(false, newUid("1")));
         assertThat(getResult.exists(), equalTo(true));
         assertThat(getResult.docIdAndVersion(), notNullValue());
-
+        getResult.release();
+
         // now do an update
         document = testDocument();
         document.add(new TextField("value", "test1", Field.Store.YES));
@@ -359,7 +360,8 @@ public void testSimpleOperations() throws Exception {
         assertThat(getResult.exists(), equalTo(true));
         assertThat(getResult.source().source.toBytesArray(), equalTo(B_2.toBytesArray()));
         assertThat(getResult.docIdAndVersion(), nullValue());
-
+        getResult.release();
+
         // refresh and it should be updated
         engine.refresh(new Engine.Refresh().force(false));
 
@@ -382,7 +384,8 @@ public void testSimpleOperations() throws Exception {
         // but, get should not see it (in realtime)
         getResult = engine.get(new Engine.Get(true, newUid("1")));
         assertThat(getResult.exists(), equalTo(false));
-
+        getResult.release();
+
         // refresh and it should be deleted
         engine.refresh(new Engine.Refresh().force(false));
 
@@ -423,7 +426,7 @@ public void testSimpleOperations() throws Exception {
         assertThat(getResult.exists(), equalTo(true));
         assertThat(getResult.source(), nullValue());
         assertThat(getResult.docIdAndVersion(), notNullValue());
-
+        getResult.release();
 
         // make sure we can still work with the engine
         // now do an update

diff --git a/src/test/java/org/elasticsearch/index/store/mock/MockDirectoryHelper.java b/src/test/java/org/elasticsearch/index/store/mock/MockDirectoryHelper.java
@@ -22,8 +22,8 @@
 import com.carrotsearch.randomizedtesting.SeedUtils;
 import org.apache.lucene.store.*;
 import org.apache.lucene.store.MockDirectoryWrapper.Throttling;
-import org.apache.lucene.util.AbstractRandomizedTest;
 import org.apache.lucene.util.Constants;
+import org.elasticsearch.ElasticsearchTestCase;
 import org.elasticsearch.cache.memory.ByteBufferCache;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.settings.Settings;
@@ -44,7 +44,6 @@
 public class MockDirectoryHelper {
     public static final String RANDOM_IO_EXCEPTION_RATE = "index.store.mock.random.io_exception_rate";
     public static final String RANDOM_IO_EXCEPTION_RATE_ON_OPEN = "index.store.mock.random.io_exception_rate_on_open";
-    public static final String RANDOM_SEED = "index.store.mock.random.seed";
     public static final String RANDOM_THROTTLE = "index.store.mock.random.throttle";
     public static final String CHECK_INDEX_ON_CLOSE = "index.store.mock.check_index_on_close";
     public static final Set<MockDirectoryWrapper> wrappers = ConcurrentCollections.newConcurrentSet();
@@ -59,9 +58,7 @@ public class MockDirectoryHelper {
     public MockDirectoryHelper(ShardId shardId, Settings indexSettings, ESLogger logger) {
         randomIOExceptionRate = indexSettings.getAsDouble(RANDOM_IO_EXCEPTION_RATE, 0.0d);
         randomIOExceptionRateOnOpen = indexSettings.getAsDouble(RANDOM_IO_EXCEPTION_RATE_ON_OPEN, 0.0d);
-        final Long currentSeed = AbstractRandomizedTest.getCurrentSeed();
-        assert currentSeed != null;
-        final long seed = indexSettings.getAsLong(RANDOM_SEED, currentSeed);
+        final long seed = indexSettings.getAsLong(ElasticsearchTestCase.INDEX_SEED_SETTING, 0l);
         random = new Random(seed);
         random.nextInt(shardId.getId() + 1); // some randomness per shard
         throttle = Throttling.valueOf(indexSettings.get(RANDOM_THROTTLE, random.nextDouble() < 0.1 ? "SOMETIMES" : "NEVER"));

diff --git a/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateTests.java b/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateTests.java
@@ -116,6 +116,7 @@ public void simpleIndexTemplateTests() throws Exception {
 
     @Test
     public void testDeleteIndexTemplate() throws Exception {
+        final int existingTemplates = admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().size();
         logger.info("--> put template_1 and template_2");
         client().admin().indices().preparePutTemplate("template_1")
                 .setTemplate("te*")
@@ -136,8 +137,10 @@ public void testDeleteIndexTemplate() throws Exception {
 
         logger.info("--> explicitly delete template_1");
         admin().indices().prepareDeleteTemplate("template_1").execute().actionGet();
-        assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().size(), equalTo(1));
+        assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().size(), equalTo(1+existingTemplates));
         assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().containsKey("template_2"), equalTo(true));
+        assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().containsKey("template_1"), equalTo(false));
+
 
         logger.info("--> put template_1 back");
         client().admin().indices().preparePutTemplate("template_1")
@@ -151,7 +154,7 @@ public void testDeleteIndexTemplate() throws Exception {
 
         logger.info("--> delete template*");
         admin().indices().prepareDeleteTemplate("template*").execute().actionGet();
-        assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().size(), equalTo(0));
+        assertThat(admin().cluster().prepareState().execute().actionGet().getState().metaData().templates().size(), equalTo(existingTemplates));
 
         logger.info("--> delete * with no templates, make sure we don't get a failure");
         admin().indices().prepareDeleteTemplate("*").execute().actionGet();
@@ -224,7 +227,7 @@ public void testThatGetIndexTemplatesWithSimpleRegexWorks() throws Exception {
         assertThat(templateNames, containsInAnyOrder("template_1", "template_2"));
 
         logger.info("--> get all templates");
-        getTemplate1Response = client().admin().indices().prepareGetTemplates().execute().actionGet();
+        getTemplate1Response = client().admin().indices().prepareGetTemplates("template*").execute().actionGet();
         assertThat(getTemplate1Response.getIndexTemplates(), hasSize(3));
 
         templateNames = Lists.newArrayList();

diff --git a/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java b/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java
@@ -66,10 +66,7 @@ public void testSearchAndRelocateConcurrently() throws Exception {
         List<IndexRequestBuilder> indexBuilders = new ArrayList<IndexRequestBuilder>();
         final int numDocs = between(10, 20);
         for (int i = 0; i < numDocs; i++) {
-            indexBuilders.add(new IndexRequestBuilder(client())
-                    .setType("type")
-                    .setId(Integer.toString(i))
-                    .setIndex("test")
+            indexBuilders.add(client().prepareIndex("test", "type", Integer.toString(i))
                     .setSource(
                             jsonBuilder().startObject().field("test", "value").startObject("loc").field("lat", 11).field("lon", 21)
                                     .endObject().endObject()));