From cfa507663d5d383fd344729423ad246982f4c853 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 10 Dec 2024 12:18:29 -0800 Subject: [PATCH] javadoc Signed-off-by: Peter Alfonsi --- .../cache/common/query/DummyQuery.java | 8 ++- .../cache/common/query/QuerySerializer.java | 14 +++++- .../cache/common/query/TieredQueryCache.java | 50 +++++++++++++++++++ .../cache/common/query/package-info.java | 12 +++++ 4 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 modules/cache-common/src/main/java/org/opensearch/cache/common/query/package-info.java diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/DummyQuery.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/DummyQuery.java index a0a86026eabca..59a66d6e28d2f 100644 --- a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/DummyQuery.java +++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/DummyQuery.java @@ -21,13 +21,15 @@ import java.io.IOException; +/** + * A dummy query class for testing. + */ public class DummyQuery extends Query { // This is mostly for testing, copied from IndicesQueryCacheTests.java. Just want to make sure everything works before testing on real // queries. // And to do this, the serializer must know about DummyQuery, so it can't live in test module private final int id; - DummyQuery(int id) { this.id = id; } @@ -67,6 +69,10 @@ public boolean isCacheable(LeafReaderContext ctx) { }; } + /** + * Get id + * @return the id + */ public int getId() { return id; } diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/QuerySerializer.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/QuerySerializer.java index 1ebffb9b16fc8..bd24cf1be2dea 100644 --- a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/QuerySerializer.java +++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/QuerySerializer.java @@ -21,6 +21,9 @@ import java.io.IOException; import java.util.Arrays; +/** + * A class to serialize Query objects. Not all Query objects are supported. You can check with isAllowed(). + */ public class QuerySerializer implements Serializer { // TODO - will have to basically do one query type at a time. Not TermQuery ever lol. @@ -31,6 +34,10 @@ public class QuerySerializer implements Serializer { // TODO: Used as part of gross hack to determine which impl of PointRangeQuery comes in. static final Query longPointRangeQuery = LongPoint.newRangeQuery("", 0, 1); + /** + * Required for javadocs. + */ + public QuerySerializer() {} @Override public byte[] serialize(Query object) { if (object == null) return null; @@ -133,9 +140,12 @@ private boolean isLongPointRangeQuery(PointRangeQuery query) { return query.getClass() == longPointRangeQuery.getClass(); } + /** + * Checks whether the query is serializable. + * @param query The query to serialize. + * @return Whether it's serializable. + */ public boolean isAllowed(Query query) { - // TODO - report true for serializable, false for not serializable. Feed this into a policy into the TSC to control disk tier - // access. return getClassByte(query) != INVALID_QUERY_BYTE; } diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/TieredQueryCache.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/TieredQueryCache.java index bdbb18041df35..696e2bef98501 100644 --- a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/TieredQueryCache.java +++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/TieredQueryCache.java @@ -124,6 +124,10 @@ Looks like LRUQC controls the eviction logic, and just tells LeafCache to remove or is it actually the case there's lots of duplicated queries in different leaves? If the latter, we may want to use it. Check on this Monday. I dont really understand why - as the comments say - there would be multiple copies. */ + +/** + * A pluggable version of the query cache which uses an ICache internally to store values. Proof of concept only! Incomplete + */ public class TieredQueryCache implements QueryCache, OpenSearchQueryCache { private final ICache innerCache; // This should typically be a TieredSpilloverCache but theoretically can @@ -144,10 +148,20 @@ public class TieredQueryCache implements QueryCache, OpenSearchQueryCache { private static final Logger logger = LogManager.getLogger(TieredQueryCache.class); + /** + * The shard id dimension name. + */ public static final String SHARD_ID_DIMENSION_NAME = "shards"; // Is there any need for locks? The underlying TSC is threadsafe. I think the need for locks in original was due to LeafCache impl. + /** + * Construct a TieredQueryCache. + * @param cacheService Cache service. + * @param settings Settings. + * @param clusterService Cluster service. + * @param nodeEnvironment Node env. + */ public TieredQueryCache(CacheService cacheService, Settings settings, ClusterService clusterService, NodeEnvironment nodeEnvironment) { // Following IQC, hardcode leavesToCache and skipFactor @@ -183,6 +197,12 @@ public TieredQueryCache(CacheService cacheService, Settings settings, ClusterSer // For example, if we just do an OpenSearchOnHeapCache of the same size, how much worse does it perform? } + /** + * Get a value from the inner cache if present. Returns null if not present. Does not add anything to the cache. + * @param query The query + * @param cacheHelper Cache helper from the query context (?) + * @return The doc ids matching the query + count, if present in the cache + */ protected CacheAndCount get(Query query, IndexReader.CacheHelper cacheHelper) { // TODO: Mostly same as LRUQC.get(), but skipping the singleton map stuff - dont think this is necessary here? But could be wrong assert query instanceof BoostQuery == false; @@ -259,9 +279,17 @@ private void putIfAbsent(Query query, CacheAndCount cached, IndexReader.CacheHel } /** + * + */ + /** + * Cache the matching doc ids into a BitSet or RoaringDocIdSet. * Duplicated from LRUQC. I think this is a bad and confusing name, but let's keep it for consistency. * Default cache implementation: uses {@link RoaringDocIdSet} for sets that have a density < 1% * and a {@link BitDocIdSet} over a {@link FixedBitSet} otherwise. + * @param scorer f + * @param maxDoc f + * @return f + * @throws IOException f */ protected CacheAndCount cacheImpl(BulkScorer scorer, int maxDoc) throws IOException { if (scorer.cost() * 100 >= maxDoc) { @@ -645,7 +673,14 @@ ICacheKey getFinalKey(Query query, String shardIdName) { } // Duplicated from LRUQC with no changes + + /** + * Doc ids and count for a query. + */ protected static class CacheAndCount implements Accountable { + /** + * An empty CacheAndCount. + */ protected static final CacheAndCount EMPTY = new CacheAndCount(DocIdSet.EMPTY, 0, 0); private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(CacheAndCount.class); @@ -653,16 +688,31 @@ protected static class CacheAndCount implements Accountable { private final int count; private final int maxDoc; // TODO: this value is needed for serialization here, but wasn't needed in LRUQC. + /** + * Constructor. + * @param cache f + * @param count f + * @param maxDoc f + */ public CacheAndCount(DocIdSet cache, int count, int maxDoc) { this.cache = cache; this.count = count; this.maxDoc = maxDoc; } + /** + * It iterates! + * @return f + * @throws IOException f + */ public DocIdSetIterator iterator() throws IOException { return cache.iterator(); } + /** + * The count + * @return count + */ public int count() { return count; } diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/query/package-info.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/package-info.java new file mode 100644 index 0000000000000..8f6c62cb932d0 --- /dev/null +++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/query/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * A package for pluggable query cache. + */ +package org.opensearch.cache.common.query;