diff --git a/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java b/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java index d675a93b691..34ad29a7054 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java +++ b/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java @@ -138,8 +138,8 @@ public class SearchShardTarget implements Writeable, Comparab @Override public String toString() { if (nodeId == null) { - return "[_na_][" + index + "][" + shardId + "]"; + return "[_na_]" + shardId; } - return "[" + nodeId + "][" + index + "][" + shardId + "]"; + return "[" + nodeId + "]" + shardId; } } diff --git a/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java b/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java index 4022366c1d0..d99d3ffb155 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java +++ b/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java @@ -19,6 +19,7 @@ package org.elasticsearch.search; +import org.elasticsearch.action.search.SearchType; import org.elasticsearch.cache.recycler.PageCacheRecycler; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; @@ -41,7 +42,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; public class MockSearchService extends SearchService { - public static class TestPlugin extends Plugin { @Override public String name() { @@ -59,13 +59,42 @@ public class MockSearchService extends SearchService { private static final Map ACTIVE_SEARCH_CONTEXTS = new ConcurrentHashMap<>(); /** Throw an {@link AssertionError} if there are still in-flight contexts. */ - public static void assertNoInFLightContext() { + public static void assertNoInFlightContext() { final Map copy = new HashMap<>(ACTIVE_SEARCH_CONTEXTS); if (copy.isEmpty() == false) { - throw new AssertionError("There are still " + copy.size() + " in-flight contexts", copy.values().iterator().next()); + Map.Entry firstOpen = copy.entrySet().iterator().next(); + SearchContext context = firstOpen.getKey(); + StringBuilder message = new StringBuilder().append(context.shardTarget()); + if (context.searchType() != SearchType.DEFAULT) { + message.append("searchType=[").append(context.searchType()).append("]"); + } + if (context.scrollContext() != null) { + message.append("scroll=[").append(context.scrollContext().scroll.keepAlive()).append("]"); + } + message.append(" query=[").append(context.query()).append("]"); + RuntimeException cause = new RuntimeException(message.toString()); + cause.setStackTrace(firstOpen.getValue().getStackTrace()); + throw new AssertionError( + "There are still " + copy.size() + + " in-flight contexts. The first one's creation site is listed as the cause of this exception.", + cause); } } + /** + * Add an active search context to the list of tracked contexts. Package private for testing. + */ + static void addActiveContext(SearchContext context) { + ACTIVE_SEARCH_CONTEXTS.put(context, new RuntimeException()); + } + + /** + * Clear an active search context from the list of tracked contexts. Package private for testing. + */ + static void removeActiveContext(SearchContext context) { + ACTIVE_SEARCH_CONTEXTS.remove(context); + } + @Inject public MockSearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService, IndicesService indicesService, ThreadPool threadPool, ScriptService scriptService, PageCacheRecycler pageCacheRecycler, @@ -78,14 +107,14 @@ public class MockSearchService extends SearchService { @Override protected void putContext(SearchContext context) { super.putContext(context); - ACTIVE_SEARCH_CONTEXTS.put(context, new RuntimeException()); + addActiveContext(context); } @Override protected SearchContext removeContext(long id) { final SearchContext removed = super.removeContext(id); if (removed != null) { - ACTIVE_SEARCH_CONTEXTS.remove(removed); + removeActiveContext(removed); } return removed; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 10616415693..342b0d03626 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -185,12 +185,7 @@ public abstract class ESTestCase extends LuceneTestCase { // this must be a separate method from other ensure checks above so suite scoped integ tests can call...TODO: fix that @After public final void ensureAllSearchContextsReleased() throws Exception { - assertBusy(new Runnable() { - @Override - public void run() { - MockSearchService.assertNoInFLightContext(); - } - }); + assertBusy(() -> MockSearchService.assertNoInFlightContext()); } // mockdirectorywrappers currently set this boolean if checkindex fails diff --git a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java new file mode 100644 index 00000000000..ec45c01eca5 --- /dev/null +++ b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java @@ -0,0 +1,67 @@ +/* + * 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.search; + +import org.apache.lucene.search.Query; +import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.lucene.search.Queries; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.TestSearchContext; + +public class MockSearchServiceTests extends ESTestCase { + public void testAssertNoInFlightContext() { + SearchContext s = new TestSearchContext(new QueryShardContext(new IndexSettings(IndexMetaData.PROTO, Settings.EMPTY), null, null, + null, null, null, null, null, null)) { + @Override + public SearchShardTarget shardTarget() { + return new SearchShardTarget("node", new Index("idx", "ignored"), 0); + } + + @Override + public SearchType searchType() { + return SearchType.DEFAULT; + } + + @Override + public Query query() { + return Queries.newMatchAllQuery(); + } + }; + MockSearchService.addActiveContext(s); + try { + Throwable e = expectThrows(AssertionError.class, () -> MockSearchService.assertNoInFlightContext()); + assertEquals("There are still 1 in-flight contexts. The first one's creation site is listed as the cause of this exception.", + e.getMessage()); + e = e.getCause(); + // The next line with throw an exception if the date looks wrong + assertEquals("[node][idx][0] query=[*:*]", e.getMessage()); + assertEquals(MockSearchService.class.getName(), e.getStackTrace()[0].getClassName()); + assertEquals(MockSearchServiceTests.class.getName(), e.getStackTrace()[1].getClassName()); + } finally { + MockSearchService.removeActiveContext(s); + } + } +}