jgq2008303393 commented on a change in pull request #940: LUCENE-9002: Query caching leads to absurdly slow queries URL: https://github.com/apache/lucene-solr/pull/940#discussion_r333851594
########## File path: lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java ########## @@ -732,8 +741,39 @@ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOExcepti if (docIdSet == null) { if (policy.shouldCache(in.getQuery())) { - docIdSet = cache(context); - putIfAbsent(in.getQuery(), docIdSet, cacheHelper); + final ScorerSupplier supplier = in.scorerSupplier(context); + if (supplier == null) { + putIfAbsent(in.getQuery(), DocIdSet.EMPTY, cacheHelper); + return null; + } + + final long cost = supplier.cost(); + return new ScorerSupplier() { + @Override + public Scorer get(long leadCost) throws IOException { + // skip cache operation which would slow query down too much + if (cost > skipCacheCost && cost > leadCost * skipCacheFactor Review comment: We have tested different scenarios to observe the query latency with/without cacheing in an online metric ES cluster. The result is as follows: | query | latencyWithoutCaching | latencyWithCaching | leadCost | rangeQueryCost | skipCacheFactor | | ---------- | :-----------: | :-----------: | :-----------: | :-----------: | :-----------: | | ip:xxx AND time:[t-1h, t] | 10ms | 36ms(+260%) | 20528 | 878979 | 42 | | ip:xxx AND time:[t-4h, t] | 10ms | 100ms(+900%) | 20528 | 4365870 | 212 | | ip:xxx AND time:[t-8h, t] | 11ms | 200ms(+1700%) | 20528 | 8724483 | 425 | | ip:xxx AND time:[t-12h, t] | 12ms | 300ms(+2400%) | 20528 | 13083096 | 637 | | ip:xxx AND time:[t-24h, t] | 16ms | 500ms(+3000%) | 20528 | 26158936 | 1274 | | ip:xxx AND time:[t-48h, t] | 30ms | 1200ms(3900%) | 20528 | 52310616 | 2548 | As the table show, query latency without caching is low and related with the final result set, and query latency with caching is much high and mainly related with _rangeQueryCost_. We set the default value of _skipCacheFactor_ to 250, which make the query slower by no more than 10 times. In addition to _skipCacheFactor_ which is similar to _maxCostFactor_ in LUCENE-8027, we have added a new parameter _skipCacheCost_. The mainly reasons are: - control the time used for caching as the caching time is related to the cost of range query. - skip caching too large range queries which will consume too much memory and evict cache frequently. How do you think? Looking forward to your ideas. @jpountz ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org