You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by mk...@apache.org on 2024/03/15 20:48:17 UTC

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

This is an automated email from the ASF dual-hosted git repository.

mkhl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 3912cf6631e SOLR-17173: throw exception on attempting enable distrib IDF with LocalStatsCache (#2332)
3912cf6631e is described below

commit 3912cf6631efe8f6c29bc8a597bd2bed657f4a3e
Author: Mikhail Khludnev <mk...@users.noreply.github.com>
AuthorDate: Fri Mar 15 23:48:11 2024 +0300

    SOLR-17173: throw exception on attempting enable distrib IDF with LocalStatsCache (#2332)
    
    follow up SOLR-17058  (#2046)
---
 .../solr/handler/component/QueryComponent.java       | 20 +++++++++++++++-----
 .../solr/handler/component/ResponseBuilder.java      | 10 +++++-----
 .../java/org/apache/solr/search/QueryCommand.java    | 10 +++++-----
 .../java/org/apache/solr/search/QueryResultKey.java  | 12 ++++++------
 .../org/apache/solr/search/SolrIndexSearcher.java    |  2 +-
 .../org/apache/solr/core/QueryResultKeyTest.java     |  8 ++++----
 .../apache/solr/search/stats/TestBaseStatsCache.java |  5 +++++
 .../solr/search/stats/TestDefaultStatsCache.java     | 15 +++++++++++++++
 8 files changed, 56 insertions(+), 26 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
index 3cde6db4adf..d30d62e1c04 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
@@ -115,6 +115,7 @@ import org.apache.solr.search.grouping.endresulttransformer.EndResultTransformer
 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 class QueryComponent extends SearchComponent {
         // 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 class QueryComponent extends SearchComponent {
       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 class QueryComponent extends SearchComponent {
     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 @@ public class QueryComponent extends SearchComponent {
 
   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);
diff --git a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java b/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java
index c5aab862978..dc58a540d1d 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java
+++ b/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 class ResponseBuilder {
     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;
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/search/QueryCommand.java b/solr/core/src/java/org/apache/solr/search/QueryCommand.java
index dce9eaa53f9..7ed6b6b9473 100755
--- a/solr/core/src/java/org/apache/solr/search/QueryCommand.java
+++ b/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 class QueryCommand {
     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;
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/search/QueryResultKey.java b/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
index 72c6b474d06..8d75c5bb6a6 100644
--- a/solr/core/src/java/org/apache/solr/search/QueryResultKey.java
+++ b/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 final class QueryResultKey implements Accountable {
       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 final class QueryResultKey implements Accountable {
     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);
diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
index 177c1476956..3b4ce7e9e6d 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -1567,7 +1567,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
               cmd.getSort(),
               flags,
               cmd.getMinExactCount(),
-              cmd.isEnableDistribStats());
+              cmd.isDistribStatsDisabled());
       if ((flags & NO_CHECK_QCACHE) == 0) {
         superset = queryResultCache.get(key);
 
diff --git a/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java b/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
index e910e99097e..2ab555de12f 100644
--- a/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
+++ b/solr/core/src/test/org/apache/solr/core/QueryResultKeyTest.java
@@ -214,16 +214,16 @@ public class QueryResultKeyTest extends SolrTestCaseJ4 {
     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));
   }
 
diff --git a/solr/core/src/test/org/apache/solr/search/stats/TestBaseStatsCache.java b/solr/core/src/test/org/apache/solr/search/stats/TestBaseStatsCache.java
index e2c6180c33c..9c7cf4e1d07 100644
--- a/solr/core/src/test/org/apache/solr/search/stats/TestBaseStatsCache.java
+++ b/solr/core/src/test/org/apache/solr/search/stats/TestBaseStatsCache.java
@@ -64,4 +64,9 @@ public abstract class TestBaseStatsCache extends TestDefaultStatsCache {
       assertEquals(controlDoc.getFieldValue("score"), shardDoc.getFieldValue("score"));
     }
   }
+
+  @Override
+  protected void checkDistribStatsException() {
+    // doing nothing on distrib stats
+  }
 }
diff --git a/solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java b/solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java
index 845154009ab..80e95607f56 100644
--- a/solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java
+++ b/solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java
@@ -20,6 +20,7 @@ import org.apache.solr.BaseDistributedSearchTestCase;
 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 class TestDefaultStatsCache extends BaseDistributedSearchTestCase {
       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 @@ public class TestDefaultStatsCache extends BaseDistributedSearchTestCase {
     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()));
+  }
 }