Tooktime #3
Tooktime #3
Conversation
…e in tiered caching
Signed-off-by: Peter Alfonsi <[email protected]>
| }); | ||
| } | ||
|
|
||
| public void testQuerySearchResultTookTimeCacheableRequest() throws Exception { |
There was a problem hiding this comment.
For now, we don't need write for cache as such. That will come later when we verify from our own cache tests. This took time related UTs should only be testing whether it is properly setting the value.
There was a problem hiding this comment.
Ok, I'll remove these tests. We can always reuse the logic if we want them later
| @@ -823,6 +824,118 @@ public Scroll scroll() { | |||
| } | |||
| } | |||
|
|
|||
| public void testQuerySearchResultTookTime() throws Exception { | |||
There was a problem hiding this comment.
I think the right place to add this test should be in QueryPhaseTests. U can create a QueryPhaseObject as that tests has some logic to initialize dependent classes.
See if you can add a deliberate delay X, using QueryPhaseSearcher , you could verify tookTime>=X
| @@ -1145,6 +1153,114 @@ public void testQueryTimeoutChecker() throws Exception { | |||
| createTimeoutCheckerThenWaitThenRun(timeCacheLifespan / 4, timeCacheLifespan / 2 + timeTolerance, false, true); | |||
| } | |||
|
|
|||
| public void testQuerySearchResultTookTime() throws IOException { | |||
| int sleepMillis = 3000; | |||
There was a problem hiding this comment.
U can consider to randomize this value.
Using randomInBetween
| Directory dir = newDirectory(); | ||
| final Sort sort = new Sort(new SortField("rank", SortField.Type.INT)); | ||
| IndexWriterConfig iwc = newIndexWriterConfig().setIndexSort(sort); | ||
| RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); | ||
| Document doc = new Document(); | ||
| for (int i = 0; i < 10; i++) { | ||
| doc.add(new StringField("foo", Integer.toString(i), Store.NO)); | ||
| } | ||
| w.addDocument(doc); | ||
| w.close(); | ||
| IndexReader reader = DirectoryReader.open(dir); | ||
|
|
||
| QueryShardContext queryShardContext = mock(QueryShardContext.class); | ||
| when(queryShardContext.fieldMapper("user")).thenReturn( | ||
| new NumberFieldType("user", NumberType.INTEGER, true, false, true, false, null, Collections.emptyMap()) | ||
| ); | ||
|
|
||
| Index index = new Index("IndexName", "UUID"); | ||
| ShardId shardId = new ShardId(index, 0); | ||
| long nowInMillis = System.currentTimeMillis(); | ||
| String clusterAlias = randomBoolean() ? null : randomAlphaOfLengthBetween(3, 10); | ||
| SearchRequest searchRequest = new SearchRequest(); | ||
| searchRequest.allowPartialSearchResults(randomBoolean()); | ||
| ShardSearchRequest request = new ShardSearchRequest( | ||
| OriginalIndices.NONE, | ||
| searchRequest, | ||
| shardId, | ||
| 1, | ||
| AliasFilter.EMPTY, | ||
| 1f, | ||
| nowInMillis, | ||
| clusterAlias, | ||
| Strings.EMPTY_ARRAY | ||
| ); | ||
| TestSearchContextWithRequest searchContext = new TestSearchContextWithRequest( | ||
| queryShardContext, | ||
| indexShard, | ||
| newEarlyTerminationContextSearcher(reader, 0, executor), | ||
| request | ||
| ); |
There was a problem hiding this comment.
I don't think u need this change now? Remove it?
There was a problem hiding this comment.
execute(), which is what the timer wraps, needs to be passed a SearchContext with an actual request in it. I tried passing a dummy request without functionality but it didn't work. So I think we need all of this to set up the request, plus the reader and queryShardContext.
Description
Adds time taken in nanoseconds to QuerySearchResult in the query phase. This will be used as part of tiered caching, to decide whether or not to move cached entries to the disk tier based on how long it would take to recompute them.
Tested using unit tests in QuerySearchResultTests.java and SearchServiceTests.java. Tests checked the code paths for both cacheable and non-cacheable requests, and checked deserialized queries from the cache have the correct took time. Also, manually tested by temporarily adding a delay into the query phase and checking the resulting tookTime was larger than this delay.
Related Issues
N/A (Part of tiered caching project)
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.