SOLR-17173: throw exception on attempting enable distrib IDF with LocalStatsCache (#2332)

follow up SOLR-17058  (#2046)

Author: mkhludnev

solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java

@@ -115,6 +115,7 @@
 import org.apache.solr.search.grouping.endresulttransformer.GroupedEndResultTransformer;
 import org.apache.solr.search.grouping.endresulttransformer.MainEndResultTransformer;
 import org.apache.solr.search.grouping.endresulttransformer.SimpleEndResultTransformer;
+import org.apache.solr.search.stats.LocalStatsCache;
 import org.apache.solr.search.stats.StatsCache;
 import org.apache.solr.util.SolrPluginUtils;
 import org.apache.solr.util.SolrResponseUtil;
@@ -146,6 +147,18 @@ public void prepare(ResponseBuilder rb) throws IOException {
         // Generate Query ID
         rb.queryID = generateQueryID(req);
       }
+      // set the flag for distributed stats
+      if (req.getSearcher().getStatsCache().getClass().equals(LocalStatsCache.class)) {
+        if (params.getPrimitiveBool(CommonParams.DISTRIB_STATS_CACHE)) {
+          throw new SolrException(
+              SolrException.ErrorCode.BAD_REQUEST,
+              "Explicitly set "
+                  + CommonParams.DISTRIB_STATS_CACHE
+                  + "=true is not supported with "
+                  + LocalStatsCache.class.getSimpleName());
+        }
+      }
+      rb.setDistribStatsDisabled(!params.getBool(CommonParams.DISTRIB_STATS_CACHE, true));
     }
 
     // Set field flags
@@ -168,9 +181,6 @@ public void prepare(ResponseBuilder rb) throws IOException {
       rb.setQueryString(queryString);
     }
 
-    // set the flag for distributed stats
-    rb.setEnableDistribStats(params.getBool(CommonParams.DISTRIB_STATS_CACHE, true));
-
     try {
       QParser parser = QParser.getParser(rb.getQueryString(), defType, req);
       Query q = parser.getQuery();
@@ -368,7 +378,7 @@ public void process(ResponseBuilder rb) throws IOException {
     QueryCommand cmd = rb.createQueryCommand();
     cmd.setTimeAllowed(timeAllowed);
     cmd.setMinExactCount(getMinExactCount(params));
-    cmd.setEnableDistribStats(rb.isEnableDistribStats());
+    cmd.setDistribStatsDisabled(rb.isDistribStatsDisabled());
 
     boolean isCancellableQuery = params.getBool(CommonParams.IS_QUERY_CANCELLABLE, false);
 
@@ -740,7 +750,7 @@ protected void regularFinishStage(ResponseBuilder rb) {
 
   protected void createDistributedStats(ResponseBuilder rb) {
     StatsCache cache = rb.req.getSearcher().getStatsCache();
-    if (rb.isEnableDistribStats()
+    if (!rb.isDistribStatsDisabled()
         && ((rb.getFieldFlags() & SolrIndexSearcher.GET_SCORES) != 0
             || rb.getSortSpec().includesScore())) {
       ShardRequest sreq = cache.retrieveStatsRequest(rb);

solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java

@@ -74,7 +74,7 @@ public class ResponseBuilder {
   private String cancellationUUID;
   private String taskStatusCheckUUID;
   private boolean isTaskListRequest;
-  private boolean isEnableDistribStats = true;
+  private boolean isDistribStatsDisabled;
 
   private QParser qparser = null;
   private String queryString = null;
@@ -519,11 +519,11 @@ public String getTaskStatusCheckUUID() {
     return taskStatusCheckUUID;
   }
 
-  public void setEnableDistribStats(boolean isEnableDistribStats) {
-    this.isEnableDistribStats = isEnableDistribStats;
+  public void setDistribStatsDisabled(boolean isEnableDistribStats) {
+    this.isDistribStatsDisabled = isEnableDistribStats;
   }
 
-  public boolean isEnableDistribStats() {
-    return isEnableDistribStats;
+  public boolean isDistribStatsDisabled() {
+    return isDistribStatsDisabled;
   }
 }

solr/core/src/java/org/apache/solr/search/QueryCommand.java

@@ -39,7 +39,7 @@ public class QueryCommand {
   private long timeAllowed = -1;
   private int minExactCount = Integer.MAX_VALUE;
   private CursorMark cursorMark;
-  private boolean enableDistribStats = true;
+  private boolean distribStatsDisabled;
 
   public CursorMark getCursorMark() {
     return cursorMark;
@@ -222,11 +222,11 @@ public boolean isQueryCancellable() {
     return isQueryCancellable;
   }
 
-  public void setEnableDistribStats(boolean enableDistribStats) {
-    this.enableDistribStats = enableDistribStats;
+  public void setDistribStatsDisabled(boolean distribStatsDisabled) {
+    this.distribStatsDisabled = distribStatsDisabled;
   }
 
-  public boolean isEnableDistribStats() {
-    return enableDistribStats;
+  public boolean isDistribStatsDisabled() {
+    return distribStatsDisabled;
   }
 }

solr/core/src/java/org/apache/solr/search/QueryResultKey.java

@@ -40,17 +40,17 @@ public final class QueryResultKey implements Accountable {
   final List<Query> filters;
   final int nc_flags; // non-comparable flags... ignored by hashCode and equals
   final int minExactCount;
-  final boolean enableDistribStats;
+  final boolean distribStatsDisabled;
   private final int hc; // cached hashCode
   private final long ramBytesUsed; // cached
 
   public QueryResultKey(Query query, List<Query> filters, Sort sort, int nc_flags) {
-    this(query, filters, sort, nc_flags, Integer.MAX_VALUE, true);
+    this(query, filters, sort, nc_flags, Integer.MAX_VALUE, false);
   }
 
   public QueryResultKey(
       Query query, List<Query> filters, Sort sort, int nc_flags, int minExactCount) {
-    this(query, filters, sort, nc_flags, minExactCount, true);
+    this(query, filters, sort, nc_flags, minExactCount, false);
   }
 
   public QueryResultKey(
@@ -59,12 +59,12 @@ public QueryResultKey(
       Sort sort,
       int nc_flags,
       int minExactCount,
-      boolean enableDistribStats) {
+      boolean distribStatsDisabled) {
     this.query = query;
     this.sort = sort;
     this.nc_flags = nc_flags;
     this.minExactCount = minExactCount;
-    this.enableDistribStats = enableDistribStats;
+    this.distribStatsDisabled = distribStatsDisabled;
 
     int h = query.hashCode();
 
@@ -124,7 +124,7 @@ public boolean equals(Object o) {
     if (!this.query.equals(other.query)) return false;
     if (!unorderedCompare(this.filters, other.filters)) return false;
     if (this.minExactCount != other.minExactCount) return false;
-    if (this.enableDistribStats != other.enableDistribStats) return false;
+    if (this.distribStatsDisabled != other.distribStatsDisabled) return false;
 
     for (int i = 0; i < sfields.size(); i++) {
       SortField sf1 = this.sfields.get(i);

solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java

@@ -1567,7 +1567,7 @@ private void getDocListC(QueryResult qr, QueryCommand cmd) throws IOException {
               cmd.getSort(),
               flags,
               cmd.getMinExactCount(),
-              cmd.isEnableDistribStats());
+              cmd.isDistribStatsDisabled());
       if ((flags & NO_CHECK_QCACHE) == 0) {
         superset = queryResultCache.get(key);
 

solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java

@@ -214,16 +214,16 @@ public void testDisableDistribStats() {
     int[] nums = smallArrayOfRandomNumbers();
     final Query base = new FlatHashTermQuery("base");
     assertKeyEquals(
-        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, Integer.MAX_VALUE, true),
+        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, Integer.MAX_VALUE, false),
         new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0));
     assertKeyEquals(
-        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 10, true),
+        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 10, false),
         new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 10));
     assertKeyNotEquals(
-        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, Integer.MAX_VALUE, false),
+        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, Integer.MAX_VALUE, true),
         new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0));
     assertKeyNotEquals(
-        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 20, false),
+        new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 20, true),
         new QueryResultKey(base, buildFiltersFromNumbers(nums), null, 0, 20));
   }
 

solr/core/src/test/org/apache/solr/search/stats/TestBaseStatsCache.java

@@ -64,4 +64,9 @@ protected void checkResponse(QueryResponse controlRsp, QueryResponse shardRsp) {
       assertEquals(controlDoc.getFieldValue("score"), shardDoc.getFieldValue("score"));
     }
   }
+
+  @Override
+  protected void checkDistribStatsException() {
+    // doing nothing on distrib stats
+  }
 }

solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java

@@ -20,6 +20,7 @@
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.junit.Test;
 
@@ -79,6 +80,8 @@ public void test() throws Exception {
       dfQuery("q", "{!cache=false}id:" + aDocId, "debugQuery", "true", "fl", "*,score");
     }
     dfQuery("q", "a_t:one a_t:four", "debugQuery", "true", "fl", "*,score");
+
+    checkDistribStatsException();
   }
 
   // in this case, as the number of shards increases, per-shard scores begin to
@@ -111,4 +114,16 @@ protected void dfQuery(Object... q) throws Exception {
     QueryResponse rsp = client.query(params);
     checkResponse(controlRsp, rsp);
   }
+
+  protected void checkDistribStatsException() throws Exception {
+    final ModifiableSolrParams params = new ModifiableSolrParams();
+    params.set("shards", shards);
+    params.set("distrib.statsCache", "true");
+    int which = r.nextInt(clients.size());
+    SolrClient client = clients.get(which);
+    SolrException e = assertThrows(SolrException.class, () -> client.query(params));
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+    assertTrue(e.getMessage().contains("distrib.statsCache"));
+    assertTrue(e.getMessage().contains(LocalStatsCache.class.getSimpleName()));
+  }
 }