Skip to content

Commit 72e63f2

Browse files
niyatiaggniyatiagg
authored
Delegating CachingWeightWrapper#count to internal weight object (#10543)
* adding delegation from CachingWeightWrapper#count to internal weight object and its unit test Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> * fixing checkstyle violations Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> * fixing existing tests Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> * adding test for delegating count to internal weight object Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> * Fixing failing unit tests Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> * Adding the change log Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com> --------- Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
1 parent e3c420f commit 72e63f2

4 files changed

Lines changed: 111 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
155155
- Fix some test methods in SimulatePipelineRequestParsingTests never run and fix test failure ([#10496](https://github.com/opensearch-project/OpenSearch/pull/10496))
156156
- Fix passing wrong parameter when calling newConfigurationException() in DotExpanderProcessor ([#10737](https://github.com/opensearch-project/OpenSearch/pull/10737))
157157
- Fix SuggestSearch.testSkipDuplicates by forceing refresh when indexing its test documents ([#11068](https://github.com/opensearch-project/OpenSearch/pull/11068))
158+
- Delegating CachingWeightWrapper#count to internal weight object ([#10543](https://github.com/opensearch-project/OpenSearch/pull/10543))
158159
- Fix per request latency last phase not tracked ([#10934](https://github.com/opensearch-project/OpenSearch/pull/10934))
159160

160161
### Security

‎server/src/main/java/org/opensearch/indices/IndicesQueryCache.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
188188
return in.bulkScorer(context);
189189
}
190190

191+
@Override
192+
public int count(LeafReaderContext context) throws IOException {
193+
shardKeyMap.add(context.reader());
194+
return in.count(context);
195+
}
196+
191197
@Override
192198
public boolean isCacheable(LeafReaderContext ctx) {
193199
return in.isCacheable(ctx);

‎server/src/test/java/org/opensearch/indices/IndicesQueryCacheTests.java‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public void testBasics() throws IOException {
151151
assertEquals(1L, stats.getCacheSize());
152152
assertEquals(1L, stats.getCacheCount());
153153
assertEquals(0L, stats.getHitCount());
154-
assertEquals(1L, stats.getMissCount());
154+
assertEquals(2L, stats.getMissCount());
155155
assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE);
156156

157157
for (int i = 1; i < 20; ++i) {
@@ -162,7 +162,7 @@ public void testBasics() throws IOException {
162162
assertEquals(10L, stats.getCacheSize());
163163
assertEquals(20L, stats.getCacheCount());
164164
assertEquals(0L, stats.getHitCount());
165-
assertEquals(20L, stats.getMissCount());
165+
assertEquals(40L, stats.getMissCount());
166166
assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE);
167167

168168
s.count(new DummyQuery(10));
@@ -171,7 +171,7 @@ public void testBasics() throws IOException {
171171
assertEquals(10L, stats.getCacheSize());
172172
assertEquals(20L, stats.getCacheCount());
173173
assertEquals(1L, stats.getHitCount());
174-
assertEquals(20L, stats.getMissCount());
174+
assertEquals(40L, stats.getMissCount());
175175
assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE);
176176

177177
IOUtils.close(r, dir);
@@ -181,7 +181,7 @@ public void testBasics() throws IOException {
181181
assertEquals(0L, stats.getCacheSize());
182182
assertEquals(20L, stats.getCacheCount());
183183
assertEquals(1L, stats.getHitCount());
184-
assertEquals(20L, stats.getMissCount());
184+
assertEquals(40L, stats.getMissCount());
185185
assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE);
186186

187187
cache.onClose(shard);
@@ -232,7 +232,7 @@ public void testTwoShards() throws IOException {
232232
assertEquals(1L, stats1.getCacheSize());
233233
assertEquals(1L, stats1.getCacheCount());
234234
assertEquals(0L, stats1.getHitCount());
235-
assertEquals(1L, stats1.getMissCount());
235+
assertEquals(2L, stats1.getMissCount());
236236
assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE);
237237

238238
QueryCacheStats stats2 = cache.getStats(shard2);
@@ -248,14 +248,14 @@ public void testTwoShards() throws IOException {
248248
assertEquals(1L, stats1.getCacheSize());
249249
assertEquals(1L, stats1.getCacheCount());
250250
assertEquals(0L, stats1.getHitCount());
251-
assertEquals(1L, stats1.getMissCount());
251+
assertEquals(2L, stats1.getMissCount());
252252
assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE);
253253

254254
stats2 = cache.getStats(shard2);
255255
assertEquals(1L, stats2.getCacheSize());
256256
assertEquals(1L, stats2.getCacheCount());
257257
assertEquals(0L, stats2.getHitCount());
258-
assertEquals(1L, stats2.getMissCount());
258+
assertEquals(2L, stats2.getMissCount());
259259
assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE);
260260

261261
for (int i = 0; i < 20; ++i) {
@@ -266,14 +266,14 @@ public void testTwoShards() throws IOException {
266266
assertEquals(0L, stats1.getCacheSize()); // evicted
267267
assertEquals(1L, stats1.getCacheCount());
268268
assertEquals(0L, stats1.getHitCount());
269-
assertEquals(1L, stats1.getMissCount());
269+
assertEquals(2L, stats1.getMissCount());
270270
assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE);
271271

272272
stats2 = cache.getStats(shard2);
273273
assertEquals(10L, stats2.getCacheSize());
274274
assertEquals(20L, stats2.getCacheCount());
275275
assertEquals(1L, stats2.getHitCount());
276-
assertEquals(20L, stats2.getMissCount());
276+
assertEquals(40L, stats2.getMissCount());
277277
assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE);
278278

279279
IOUtils.close(r1, dir1);
@@ -283,14 +283,14 @@ public void testTwoShards() throws IOException {
283283
assertEquals(0L, stats1.getCacheSize());
284284
assertEquals(1L, stats1.getCacheCount());
285285
assertEquals(0L, stats1.getHitCount());
286-
assertEquals(1L, stats1.getMissCount());
286+
assertEquals(2L, stats1.getMissCount());
287287
assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE);
288288

289289
stats2 = cache.getStats(shard2);
290290
assertEquals(10L, stats2.getCacheSize());
291291
assertEquals(20L, stats2.getCacheCount());
292292
assertEquals(1L, stats2.getHitCount());
293-
assertEquals(20L, stats2.getMissCount());
293+
assertEquals(40L, stats2.getMissCount());
294294
assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE);
295295

296296
cache.onClose(shard1);
@@ -307,7 +307,7 @@ public void testTwoShards() throws IOException {
307307
assertEquals(10L, stats2.getCacheSize());
308308
assertEquals(20L, stats2.getCacheCount());
309309
assertEquals(1L, stats2.getHitCount());
310-
assertEquals(20L, stats2.getMissCount());
310+
assertEquals(40L, stats2.getMissCount());
311311
assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE);
312312

313313
IOUtils.close(r2, dir2);
@@ -388,8 +388,10 @@ public void testStatsOnEviction() throws IOException {
388388
private static class DummyWeight extends Weight {
389389

390390
private final Weight weight;
391+
private final int randCount = randomIntBetween(0, Integer.MAX_VALUE);
391392
private boolean scorerCalled;
392393
private boolean scorerSupplierCalled;
394+
private boolean countCalled;
393395

394396
DummyWeight(Weight weight) {
395397
super(weight.getQuery());
@@ -413,6 +415,12 @@ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOExcepti
413415
return weight.scorerSupplier(context);
414416
}
415417

418+
@Override
419+
public int count(LeafReaderContext context) throws IOException {
420+
countCalled = true;
421+
return randCount;
422+
}
423+
416424
@Override
417425
public boolean isCacheable(LeafReaderContext ctx) {
418426
return true;
@@ -458,4 +466,26 @@ public void onUse(Query query) {}
458466
cache.onClose(shard);
459467
cache.close();
460468
}
469+
470+
public void testDelegatesCount() throws Exception {
471+
Directory dir = newDirectory();
472+
IndexWriter w = new IndexWriter(dir, newIndexWriterConfig());
473+
w.addDocument(new Document());
474+
DirectoryReader r = DirectoryReader.open(w);
475+
w.close();
476+
ShardId shard = new ShardId("index", "_na_", 0);
477+
r = OpenSearchDirectoryReader.wrap(r, shard);
478+
IndexSearcher s = new IndexSearcher(r);
479+
IndicesQueryCache cache = new IndicesQueryCache(Settings.EMPTY);
480+
s.setQueryCache(cache);
481+
Query query = new MatchAllDocsQuery();
482+
final DummyWeight weight = new DummyWeight(s.createWeight(s.rewrite(query), ScoreMode.COMPLETE_NO_SCORES, 1f));
483+
final Weight cached = cache.doCache(weight, s.getQueryCachingPolicy());
484+
assertFalse(weight.countCalled);
485+
assertEquals(weight.randCount, cached.count(s.getIndexReader().leaves().get(0)));
486+
assertTrue(weight.countCalled);
487+
IOUtils.close(r, dir);
488+
cache.onClose(shard);
489+
cache.close();
490+
}
461491
}

‎server/src/test/java/org/opensearch/indices/IndicesServiceCloseTests.java‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,16 @@
3232

3333
package org.opensearch.indices;
3434

35-
import org.apache.lucene.document.LongPoint;
35+
import org.apache.lucene.index.LeafReaderContext;
36+
import org.apache.lucene.search.ConstantScoreScorer;
37+
import org.apache.lucene.search.ConstantScoreWeight;
38+
import org.apache.lucene.search.DocIdSetIterator;
39+
import org.apache.lucene.search.IndexSearcher;
3640
import org.apache.lucene.search.Query;
41+
import org.apache.lucene.search.QueryVisitor;
42+
import org.apache.lucene.search.ScoreMode;
43+
import org.apache.lucene.search.Scorer;
44+
import org.apache.lucene.search.Weight;
3745
import org.opensearch.cluster.ClusterName;
3846
import org.opensearch.cluster.routing.allocation.DiskThresholdSettings;
3947
import org.opensearch.common.cache.RemovalNotification;
@@ -59,6 +67,7 @@
5967
import org.opensearch.test.hamcrest.OpenSearchAssertions;
6068
import org.opensearch.transport.nio.MockNioTransportPlugin;
6169

70+
import java.io.IOException;
6271
import java.nio.file.Path;
6372
import java.util.Arrays;
6473
import java.util.Collections;
@@ -73,6 +82,56 @@
7382

7483
public class IndicesServiceCloseTests extends OpenSearchTestCase {
7584

85+
private static class DummyQuery extends Query {
86+
87+
private final int id;
88+
89+
DummyQuery(int id) {
90+
this.id = id;
91+
}
92+
93+
@Override
94+
public void visit(QueryVisitor visitor) {
95+
visitor.visitLeaf(this);
96+
}
97+
98+
@Override
99+
public boolean equals(Object obj) {
100+
return sameClassAs(obj) && id == ((IndicesServiceCloseTests.DummyQuery) obj).id;
101+
}
102+
103+
@Override
104+
public int hashCode() {
105+
return 31 * classHash() + id;
106+
}
107+
108+
@Override
109+
public String toString(String field) {
110+
return "dummy";
111+
}
112+
113+
@Override
114+
public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
115+
return new ConstantScoreWeight(this, boost) {
116+
@Override
117+
public Scorer scorer(LeafReaderContext context) throws IOException {
118+
return new ConstantScoreScorer(this, score(), scoreMode, DocIdSetIterator.all(context.reader().maxDoc()));
119+
}
120+
121+
@Override
122+
public int count(LeafReaderContext context) {
123+
return -1;
124+
}
125+
126+
@Override
127+
public boolean isCacheable(LeafReaderContext ctx) {
128+
return true;
129+
}
130+
};
131+
}
132+
133+
}
134+
76135
private Node startNode() throws NodeValidationException {
77136
final Path tempDir = createTempDir();
78137
String nodeName = "node_s_0";
@@ -225,7 +284,7 @@ public void testCloseAfterRequestHasUsedQueryCache() throws Exception {
225284
Engine.Searcher searcher = shard.acquireSearcher("test");
226285
assertEquals(1, searcher.getIndexReader().maxDoc());
227286

228-
Query query = LongPoint.newRangeQuery("foo", 0, 5);
287+
Query query = new DummyQuery(1);
229288
assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize());
230289
searcher.count(query);
231290
assertEquals(1L, cache.getStats(shard.shardId()).getCacheSize());
@@ -271,7 +330,7 @@ public void testCloseWhileOngoingRequestUsesQueryCache() throws Exception {
271330
node.close();
272331
assertEquals(1, indicesService.indicesRefCount.refCount());
273332

274-
Query query = LongPoint.newRangeQuery("foo", 0, 5);
333+
Query query = new DummyQuery(1);
275334
assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize());
276335
searcher.count(query);
277336
assertEquals(1L, cache.getStats(shard.shardId()).getCacheSize());

0 commit comments

Comments
 (0)