Skip to content

Commit 43c0ef4

Browse files
author
Peter Alfonsi
committed
Fix IRC timeout bug
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
1 parent 093c060 commit 43c0ef4

File tree

3 files changed

+74
-6
lines changed

3 files changed

+74
-6
lines changed

server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@
3434

3535
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
3636

37+
import org.apache.lucene.index.Term;
38+
import org.apache.lucene.search.IndexSearcher;
39+
import org.apache.lucene.search.Query;
40+
import org.apache.lucene.search.ScoreMode;
41+
import org.apache.lucene.search.TermQuery;
42+
import org.apache.lucene.search.Weight;
3743
import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
3844
import org.opensearch.action.admin.cluster.node.stats.NodeStats;
3945
import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
@@ -56,7 +62,10 @@
5662
import org.opensearch.env.NodeEnvironment;
5763
import org.opensearch.index.IndexSettings;
5864
import org.opensearch.index.cache.request.RequestCacheStats;
65+
import org.opensearch.index.query.QueryBuilder;
5966
import org.opensearch.index.query.QueryBuilders;
67+
import org.opensearch.index.query.QueryShardContext;
68+
import org.opensearch.index.query.TermQueryBuilder;
6069
import org.opensearch.search.aggregations.bucket.global.GlobalAggregationBuilder;
6170
import org.opensearch.search.aggregations.bucket.histogram.DateHistogramInterval;
6271
import org.opensearch.search.aggregations.bucket.histogram.Histogram;
@@ -65,6 +74,7 @@
6574
import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
6675
import org.opensearch.test.hamcrest.OpenSearchAssertions;
6776

77+
import java.io.IOException;
6878
import java.nio.file.Files;
6979
import java.nio.file.Path;
7080
import java.time.ZoneId;
@@ -768,6 +778,61 @@ public void testDeleteAndCreateSameIndexShardOnSameNode() throws Exception {
768778
assertTrue(stats.getMemorySizeInBytes() == 0);
769779
}
770780

781+
public void testTimedOutQuery() throws Exception {
782+
// A timed out query should be cached and then invalidated
783+
Client client = client();
784+
String index = "index";
785+
assertAcked(
786+
client.admin()
787+
.indices()
788+
.prepareCreate(index)
789+
.setMapping("k", "type=keyword")
790+
.setSettings(
791+
Settings.builder()
792+
.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true)
793+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
794+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
795+
// Disable index refreshing to avoid cache being invalidated mid-test
796+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueMillis(-1))
797+
)
798+
.get()
799+
);
800+
indexRandom(true, client.prepareIndex(index).setSource("k", "hello"));
801+
ensureSearchable(index);
802+
// Force merge the index to ensure there can be no background merges during the subsequent searches that would invalidate the cache
803+
forceMerge(client, index);
804+
805+
QueryBuilder timeoutQueryBuilder = new TermQueryBuilder("k", "hello") {
806+
@Override
807+
protected Query doToQuery(QueryShardContext context) {
808+
return new TermQuery(new Term("k", "hello")) {
809+
@Override
810+
public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
811+
// Create the weight before sleeping. Otherwise, TermStates.build() (in the call to super.createWeight()) will
812+
// sometimes throw an exception on timeout, rather than timing out gracefully.
813+
Weight result = super.createWeight(searcher, scoreMode, boost);
814+
try {
815+
// Pick 500 ms as it's the same duration used in SearchTimeoutIT.testSimpleTimeout() to ensure a timeout
816+
// (We can't directly reuse their ScriptQuery-based logic as it isn't cacheable)
817+
Thread.sleep(500);
818+
} catch (InterruptedException ignored) {}
819+
return result;
820+
}
821+
};
822+
}
823+
};
824+
825+
SearchResponse resp = client.prepareSearch(index)
826+
.setRequestCache(true)
827+
.setQuery(timeoutQueryBuilder)
828+
.setTimeout(TimeValue.ZERO)
829+
.get();
830+
assertTrue(resp.isTimedOut());
831+
RequestCacheStats requestCacheStats = getRequestCacheStats(client, index);
832+
// The cache should be empty as the timed-out query was invalidated
833+
assertEquals(0, requestCacheStats.getMemorySizeInBytes());
834+
}
835+
771836
private Path[] shardDirectory(String server, Index index, int shard) {
772837
NodeEnvironment env = internalCluster().getInstance(NodeEnvironment.class, server);
773838
final Path[] paths = env.availableShardPaths(new ShardId(index, shard));

server/src/main/java/org/opensearch/indices/IndicesRequestCache.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -310,12 +310,11 @@ BytesReference getOrCompute(
310310
* @param cacheKey the cache key to invalidate
311311
*/
312312
void invalidate(IndicesService.IndexShardCacheEntity cacheEntity, DirectoryReader reader, BytesReference cacheKey) {
313-
assert reader.getReaderCacheHelper() != null;
314-
String readerCacheKeyId = null;
315-
if (reader instanceof OpenSearchDirectoryReader) {
316-
IndexReader.CacheHelper cacheHelper = ((OpenSearchDirectoryReader) reader).getDelegatingCacheHelper();
317-
readerCacheKeyId = ((OpenSearchDirectoryReader.DelegatingCacheHelper) cacheHelper).getDelegatingCacheKey().getId();
318-
}
313+
assert reader.getReaderCacheHelper() instanceof OpenSearchDirectoryReader.DelegatingCacheHelper;
314+
OpenSearchDirectoryReader.DelegatingCacheHelper delegatingCacheHelper = (OpenSearchDirectoryReader.DelegatingCacheHelper) reader
315+
.getReaderCacheHelper();
316+
String readerCacheKeyId = delegatingCacheHelper.getDelegatingCacheKey().getId();
317+
319318
IndexShard indexShard = (IndexShard) cacheEntity.getCacheIdentity();
320319
cache.invalidate(getICacheKey(new Key(indexShard.shardId(), cacheKey, readerCacheKeyId, System.identityHashCode(indexShard))));
321320
}

server/src/main/java/org/opensearch/indices/IndicesService.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import org.opensearch.common.io.stream.BytesStreamOutput;
6969
import org.opensearch.common.lease.Releasable;
7070
import org.opensearch.common.lifecycle.AbstractLifecycleComponent;
71+
import org.opensearch.common.lucene.index.OpenSearchDirectoryReader;
7172
import org.opensearch.common.settings.IndexScopedSettings;
7273
import org.opensearch.common.settings.Setting;
7374
import org.opensearch.common.settings.Setting.Property;
@@ -1754,6 +1755,9 @@ public boolean canCache(ShardSearchRequest request, SearchContext context) {
17541755
if (context.getQueryShardContext().isCacheable() == false) {
17551756
return false;
17561757
}
1758+
if (!(context.searcher().getIndexReader().getReaderCacheHelper() instanceof OpenSearchDirectoryReader.DelegatingCacheHelper)) {
1759+
return false;
1760+
}
17571761
return true;
17581762

17591763
}

0 commit comments

Comments
 (0)