You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mk...@apache.org on 2017/06/24 21:38:13 UTC

lucene-solr:branch_6x: SOLR-10824: fixing NPE ExactSharedStatsCache, avoid maxdocs skew on unique terms.

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x a3ac47af5 -> 49e230edf


SOLR-10824: fixing NPE ExactSharedStatsCache, avoid maxdocs skew on
unique terms.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/49e230ed
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/49e230ed
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/49e230ed

Branch: refs/heads/branch_6x
Commit: 49e230edf045ec162a962adc064e76161fa76e36
Parents: a3ac47a
Author: Mikhail Khludnev <mk...@apache.org>
Authored: Mon Jun 19 12:28:25 2017 +0300
Committer: Mikhail Khludnev <mk...@apache.org>
Committed: Mon Jun 12 22:26:30 2017 +0300

----------------------------------------------------------------------
 solr/CHANGES.txt                                 |  3 +++
 .../solr/search/stats/ExactSharedStatsCache.java | 12 ++++++------
 .../solr/search/stats/ExactStatsCache.java       | 19 +++++++++++--------
 .../solr/search/stats/TestDefaultStatsCache.java | 17 ++++++++++++++---
 4 files changed, 34 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/49e230ed/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 094f680..4dd8b75 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -137,6 +137,9 @@ Bug Fixes
 
 * SOLR-10763: Admin UI replication tab sometimes empty when failed replications (janhoy, Bojan Vitnik)
 
+* SOLR-10824: fix NPE ExactSharedStatsCache, fixing maxdocs skew for terms which are absent at one of shards
+when using one of Exact*StatsCache (Mikhail Khludnev)
+
 Optimizations
 ----------------------
 * SOLR-10634: JSON Facet API: When a field/terms facet will retrieve all buckets (i.e. limit:-1)

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/49e230ed/solr/core/src/java/org/apache/solr/search/stats/ExactSharedStatsCache.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/stats/ExactSharedStatsCache.java b/solr/core/src/java/org/apache/solr/search/stats/ExactSharedStatsCache.java
index fb959a4..de4f7ec 100644
--- a/solr/core/src/java/org/apache/solr/search/stats/ExactSharedStatsCache.java
+++ b/solr/core/src/java/org/apache/solr/search/stats/ExactSharedStatsCache.java
@@ -16,17 +16,17 @@
  */
 package org.apache.solr.search.stats;
 
+import java.lang.invoke.MethodHandles;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.handler.component.ResponseBuilder;
 import org.apache.solr.request.SolrQueryRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.lang.invoke.MethodHandles;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.concurrent.ConcurrentHashMap;
-
 
 public class ExactSharedStatsCache extends ExactStatsCache {
   private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -73,7 +73,7 @@ public class ExactSharedStatsCache extends ExactStatsCache {
 
   protected TermStats getPerShardTermStats(SolrQueryRequest req, String t, String shard) {
     Map<String,TermStats> cache = perShardTermStats.get(shard);
-    return cache.get(t);
+    return (cache != null) ? cache.get(t) : null; //Term doesn't exist in shard;
   }
 
   protected void addToGlobalColStats(SolrQueryRequest req,

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/49e230ed/solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java b/solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java
index 51d772c..413584d 100644
--- a/solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java
+++ b/solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java
@@ -163,6 +163,10 @@ public class ExactStatsCache extends StatsCache {
       for (Term t : terms) {
         TermContext termContext = TermContext.build(context, t);
 
+        if (!colMap.containsKey(t.field())) { // collection stats for this field
+          colMap.put(t.field(), new CollectionStats(searcher.localCollectionStatistics(t.field())));
+        }
+
         TermStatistics tst = searcher.localTermStatistics(t, termContext);
         if (tst.docFreq() == 0) { // skip terms that are not present here
           continue;
@@ -170,23 +174,22 @@ public class ExactStatsCache extends StatsCache {
 
         statsMap.put(t.toString(), new TermStats(t.field(), tst));
         rb.rsp.add(TERMS_KEY, t.toString());
-        if (!colMap.containsKey(t.field())) { // collection stats for this field
-          colMap.put(t.field(), new CollectionStats(searcher.localCollectionStatistics(t.field())));
-        }
       }
-      if (statsMap.size() != 0 && colMap.size() != 0) { //Don't add empty keys
+      if (statsMap.size() != 0) { //Don't add empty keys
         String termStatsString = StatsUtil.termStatsMapToString(statsMap);
         rb.rsp.add(TERM_STATS_KEY, termStatsString);
-
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("termStats=" + termStatsString + ", terms=" + terms + ", numDocs=" + searcher.maxDoc());
+        }
+      }
+      if (colMap.size() != 0){
         String colStatsString = StatsUtil.colStatsMapToString(colMap);
         rb.rsp.add(COL_STATS_KEY, colStatsString);
-
         if (LOG.isDebugEnabled()) {
-          LOG.debug("termStats=" + termStatsString + ", collectionStats="
+          LOG.debug("collectionStats="
               + colStatsString + ", terms=" + terms + ", numDocs=" + searcher.maxDoc());
         }
       }
-
     } catch (IOException e) {
       LOG.error("Error collecting local stats, query='" + q.toString() + "'", e);
       throw new SolrException(ErrorCode.SERVER_ERROR, "Error collecting local stats.", e);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/49e230ed/solr/core/src/test/org/apache/solr/search/stats/TestDefaultStatsCache.java
----------------------------------------------------------------------
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 955385e..88300ba 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
@@ -37,14 +37,17 @@ public class TestDefaultStatsCache extends BaseDistributedSearchTestCase {
     System.clearProperty("solr.statsCache");
   }
 
-  @Test
+  @Test 
   public void test() throws Exception {
     del("*:*");
+    String aDocId=null;
     for (int i = 0; i < clients.size(); i++) {
       int shard = i + 1;
       for (int j = 0; j <= i; j++) {
-        index_specific(i, id, docId++, "a_t", "one two three",
+        int currentId = docId++;
+        index_specific(i, id,currentId , "a_t", "one two three",
             "shard_i", shard);
+        aDocId = rarely() ? currentId+"":aDocId;
       }
     }
     commit();
@@ -52,18 +55,26 @@ public class TestDefaultStatsCache extends BaseDistributedSearchTestCase {
     handle.put("QTime", SKIPVAL);   
     handle.put("timestamp", SKIPVAL);
     
+    if (aDocId != null) {
+      dfQuery("q", "id:"+aDocId, "debugQuery", "true", "fl", "*,score");
+    }
     dfQuery("q", "a_t:one", "debugQuery", "true", "fl", "*,score");
     
     // add another document
     for (int i = 0; i < clients.size(); i++) {
       int shard = i + 1;
       for (int j = 0; j <= i; j++) {
-        index_specific(i, id, docId++, "a_t", "one two three four five",
+        int currentId = docId++;
+        index_specific(i, id, currentId, "a_t", "one two three four five",
             "shard_i", shard);
+        aDocId = rarely() ? currentId+"":aDocId;
       }
     }
     commit();
 
+    if (aDocId != null) {
+      dfQuery("q", "id:"+aDocId,"debugQuery", "true", "fl", "*,score");
+    }
     dfQuery("q", "a_t:one a_t:four", "debugQuery", "true", "fl", "*,score");
   }