Skip to content

Disable caching when queries are profiled #48195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,11 @@ public boolean canCache(ShardSearchRequest request, SearchContext context) {
return false;
}

// Profiled queries should not use the cache
if (request.source() != null && request.source().profile()) {
return false;
}

IndexSettings settings = context.indexShard().indexSettings();
// if not explicitly set in the request, use the index setting, if not, use the request
if (request.requestCache() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public Weight createWeight(Query query, ScoreMode scoreMode, float boost) throws
timer.start();
final Weight weight;
try {
weight = super.createWeight(query, scoreMode, boost);
weight = query.createWeight(this, scoreMode, boost);
} finally {
timer.stop();
profiler.pollLastElement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void extractTerms(Set<Term> set) {

@Override
public boolean isCacheable(LeafReaderContext ctx) {
return subQueryWeight.isCacheable(ctx);
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,49 @@ public void testCacheWithFilteredAlias() {
assertCacheState(client, "index", 2, 2);
}

public void testProfileDisableCache() throws Exception {
Client client = client();
assertAcked(
client.admin().indices().prepareCreate("index")
.addMapping("_doc", "k", "type=keyword")
.setSettings(
Settings.builder()
.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
)
.get()
);
indexRandom(true, client.prepareIndex("index").setSource("k", "hello"));
ensureSearchable("index");

int expectedHits = 0;
int expectedMisses = 0;
for (int i = 0; i < 5; i++) {
boolean profile = i % 2 == 0;
SearchResponse resp = client.prepareSearch("index")
.setRequestCache(true)
.setProfile(profile)
.setQuery(QueryBuilders.termQuery("k", "hello"))
.get();
assertSearchResponse(resp);
ElasticsearchAssertions.assertAllSuccessful(resp);
assertThat(resp.getHits().getTotalHits().value, equalTo(1L));
if (profile == false) {
if (i == 1) {
expectedMisses ++;
} else {
expectedHits ++;
}
}
assertCacheState(client, "index", expectedHits, expectedMisses);
}
}

private static void assertCacheState(Client client, String index, long expectedHits, long expectedMisses) {
RequestCacheStats requestCacheStats = client.admin().indices().prepareStats(index).setRequestCache(true).get().getTotal()
.getRequestCache();
RequestCacheStats requestCacheStats = client.admin().indices().prepareStats(index)
.setRequestCache(true)
.get().getTotal().getRequestCache();
// Check the hit count and miss count together so if they are not
// correct we can see both values
assertEquals(Arrays.asList(expectedHits, expectedMisses, 0L),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public class QueryProfilerIT extends ESIntegTestCase {
* This test simply checks to make sure nothing crashes. Test indexes 100-150 documents,
* constructs 20-100 random queries and tries to profile them
*/
@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/LUCENE-8658")
public void testProfileQuery() throws Exception {
createIndex("test");
ensureGreen();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.lucene.index.Term;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.LRUQueryCache;
import org.apache.lucene.search.LeafCollector;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryCachingPolicy;
Expand All @@ -47,6 +48,7 @@
import org.elasticsearch.search.internal.ContextIndexSearcher;
import org.elasticsearch.search.profile.ProfileResult;
import org.elasticsearch.test.ESTestCase;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;

Expand Down Expand Up @@ -81,7 +83,16 @@ public static void setup() throws IOException {
reader = w.getReader();
w.close();
searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(),
IndexSearcher.getDefaultQueryCache(), MAYBE_CACHE_POLICY);
IndexSearcher.getDefaultQueryCache(), ALWAYS_CACHE_POLICY);
}

@After
public void checkNoCache() {
LRUQueryCache cache = (LRUQueryCache) searcher.getQueryCache();
assertThat(cache.getHitCount(), equalTo(0L));
assertThat(cache.getCacheCount(), equalTo(0L));
assertThat(cache.getTotalCount(), equalTo(cache.getMissCount()));
assertThat(cache.getCacheSize(), equalTo(0L));
}

@AfterClass
Expand Down Expand Up @@ -158,10 +169,6 @@ public void testUseIndexStats() throws IOException {

public void testApproximations() throws IOException {
QueryProfiler profiler = new QueryProfiler();
// disable query caching since we want to test approximations, which won't
// be exposed on a cached entry
ContextIndexSearcher searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(),
null, MAYBE_CACHE_POLICY);
searcher.setProfiler(profiler);
Query query = new RandomApproximationQuery(new TermQuery(new Term("foo", "bar")), random());
searcher.count(query);
Expand All @@ -184,7 +191,6 @@ public void testApproximations() throws IOException {

long rewriteTime = profiler.getRewriteTime();
assertThat(rewriteTime, greaterThan(0L));

}

public void testCollector() throws IOException {
Expand Down Expand Up @@ -288,17 +294,4 @@ public boolean shouldCache(Query query) throws IOException {
}

};

private static final QueryCachingPolicy NEVER_CACHE_POLICY = new QueryCachingPolicy() {

@Override
public void onUse(Query query) {}

@Override
public boolean shouldCache(Query query) throws IOException {
return false;
}

};

}