SOLR-2111: improve facet exception handling, change exception return type

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@995253 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -460,6 +460,12 @@ Bug Fixes
 
 * SOLR-2107: MoreLikeThisHandler doesn't work with alternate qparsers. (yonik)
 
+* SOLR-2111: Change exception handling in distributed faceting to work more
+  like non-distributed faceting, change facet_counts/exception from a String
+  to a List<String> to enable listing all exceptions that happened, and
+  prevent an exception in one facet command from affecting another
+  facet command. (yonik)
+
 
 Other Changes
 ----------------------

solr/src/java/org/apache/solr/handler/component/FacetComponent.java

@@ -209,9 +209,9 @@ public class  FacetComponent extends SearchComponent
             dff.initialLimit = dff.limit;
           }
 
-          // TEST: Uncomment the following line when testing to supress over-requesting facets and
+          // Currently this is for testing only and allows overriding of the
-          // thus cause more facet refinement queries.
+          // facet.limit set to the shards
-          // if (dff.limit > 0) dff.initialLimit = dff.offset + dff.limit;
+          dff.initialLimit = rb.req.getParams().getInt("facet.shard.limit", dff.initialLimit);
 
           sreq.params.set(paramStart + FacetParams.FACET_LIMIT,  dff.initialLimit);
       }
@@ -243,6 +243,8 @@ public class  FacetComponent extends SearchComponent
       int shardNum = rb.getShardNum(srsp.getShard());
       NamedList facet_counts = (NamedList)srsp.getSolrResponse().getResponse().get("facet_counts");
 
+      fi.addExceptions((List)facet_counts.get("exception"));
+
       // handle facet queries
       NamedList facet_queries = (NamedList)facet_counts.get("facet_queries");
       if (facet_queries != null) {
@@ -257,18 +259,12 @@ public class  FacetComponent extends SearchComponent
       // step through each facet.field, adding results from this shard
       NamedList facet_fields = (NamedList)facet_counts.get("facet_fields");
     
-      // an error could cause facet_fields to come back null
+      if (facet_fields != null) {
-      if (facet_fields == null) {
-        String msg = (String)facet_counts.get("exception");
-        if (msg == null) msg = "faceting exception in sub-request - missing facet_fields";
-        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, msg);
-
-      }
-
         for (DistribFieldFacet dff : fi.facets.values()) {
           dff.add(shardNum, (NamedList)facet_fields.get(dff.getKey()), dff.initialLimit);
         }
       }
+    }
 
 
     //
@@ -337,6 +333,10 @@ public class  FacetComponent extends SearchComponent
       NamedList facet_counts = (NamedList)srsp.getSolrResponse().getResponse().get("facet_counts");
       NamedList facet_fields = (NamedList)facet_counts.get("facet_fields");
 
+      fi.addExceptions((List)facet_counts.get("exception"));
+
+      if (facet_fields == null) continue; // this can happen when there's an exception      
+
       for (int i=0; i<facet_fields.size(); i++) {
         String key = facet_fields.getName(i);
         DistribFieldFacet dff = (DistribFieldFacet)fi.facets.get(key);
@@ -364,6 +364,11 @@ public class  FacetComponent extends SearchComponent
     FacetInfo fi = rb._facetInfo;
 
     NamedList facet_counts = new SimpleOrderedMap();
+
+    if (fi.exceptionList != null) {
+      facet_counts.add("exception",fi.exceptionList);
+    }
+
     NamedList facet_queries = new SimpleOrderedMap();
     facet_counts.add("facet_queries",facet_queries);
     for (QueryFacet qf : fi.queryFacets.values()) {
@@ -460,6 +465,7 @@ public class  FacetComponent extends SearchComponent
   public static class FacetInfo {
     public LinkedHashMap<String,QueryFacet> queryFacets;
     public LinkedHashMap<String,DistribFieldFacet> facets;
+    public List exceptionList;
 
     void parse(SolrParams params, ResponseBuilder rb) {
       queryFacets = new LinkedHashMap<String,QueryFacet>();
@@ -482,6 +488,12 @@ public class  FacetComponent extends SearchComponent
         }
       }
     }
+        
+    public void addExceptions(List exceptions) {
+      if (exceptions == null) return;
+      if (exceptionList == null) exceptionList = new ArrayList();
+      exceptionList.addAll(exceptions);
+    }
   }
 
   /**
@@ -604,7 +616,8 @@ public class  FacetComponent extends SearchComponent
     }
 
     void add(int shardNum, NamedList shardCounts, int numRequested) {
-      int sz = shardCounts.size();
+      // shardCounts could be null if there was an exception
+      int sz = shardCounts == null ? 0 : shardCounts.size();
       int numReceived = sz;
 
       OpenBitSet terms = new OpenBitSet(termNum+sz);

solr/src/java/org/apache/solr/request/SimpleFacets.java

@@ -71,6 +71,8 @@ public class SimpleFacets {
   protected SolrQueryRequest req;
   protected ResponseBuilder rb;
 
+  protected SimpleOrderedMap facetResponse;
+
   public final Date NOW = new Date();
 
   // per-facet values
@@ -182,19 +184,29 @@ public class SimpleFacets {
     if (!params.getBool(FacetParams.FACET,true))
       return null;
 
-    NamedList res = new SimpleOrderedMap();
+    facetResponse = new SimpleOrderedMap();
     try {
-
+      facetResponse.add("facet_queries", getFacetQueryCounts());
-      res.add("facet_queries", getFacetQueryCounts());
+      facetResponse.add("facet_fields", getFacetFieldCounts());
-      res.add("facet_fields", getFacetFieldCounts());
+      facetResponse.add("facet_dates", getFacetDateCounts());
-      res.add("facet_dates", getFacetDateCounts());
+      facetResponse.add("facet_ranges", getFacetRangeCounts());
-      res.add("facet_ranges", getFacetRangeCounts());
 
     } catch (Exception e) {
       SolrException.logOnce(SolrCore.log, "Exception during facet counts", e);
-      res.add("exception", SolrException.toStr(e));
+      addException("Exception during facet counts", e);
     }
-    return res;
+    return facetResponse;
+  }
+
+  public void addException(String msg, Exception e) {
+    List exceptions = (List)facetResponse.get("exception");
+    if (exceptions == null) {
+      exceptions = new ArrayList();
+      facetResponse.add("exception", exceptions);
+    }
+
+    String entry = msg + '\n' + SolrException.toStr(e);
+    exceptions.add(entry);
   }
 
   /**
@@ -215,14 +227,22 @@ public class SimpleFacets {
     // SolrQueryParser qp = searcher.getSchema().getSolrQueryParser(null);
 
     String[] facetQs = params.getParams(FacetParams.FACET_QUERY);
+
     if (null != facetQs && 0 != facetQs.length) {
       for (String q : facetQs) {
+        try {
           parseParams(FacetParams.FACET_QUERY, q);
 
           // TODO: slight optimization would prevent double-parsing of any localParams
           Query qobj = QParser.getParser(q, null, req).getQuery();
           res.add(key, searcher.numDocs(qobj, base));
         }
+        catch (Exception e) {
+          String msg = "Exception during facet.query of " + q;
+          SolrException.logOnce(SolrCore.log, msg, e);
+          addException(msg , e);
+        }
+      }
     }
 
     return res;
@@ -325,6 +345,7 @@ public class SimpleFacets {
     String[] facetFs = params.getParams(FacetParams.FACET_FIELD);
     if (null != facetFs) {
       for (String f : facetFs) {
+        try {
           parseParams(FacetParams.FACET_FIELD, f);
           String termList = localParams == null ? null : localParams.get(CommonParams.TERMS);
           if (termList != null) {
@@ -332,6 +353,11 @@ public class SimpleFacets {
           } else {
             res.add(key, getTermCounts(facetValue));
           }
+        } catch (Exception e) {
+          String msg = "Exception during facet.field of " + f;
+          SolrException.logOnce(SolrCore.log, msg, e);
+          addException(msg , e);
+        }
       }
     }
     return res;
@@ -753,6 +779,7 @@ public class SimpleFacets {
    *
    * @see FacetParams#FACET_DATE
    */
+
   public NamedList getFacetDateCounts()
     throws IOException, ParseException {
 
@@ -761,10 +788,26 @@ public class SimpleFacets {
 
     if (null == fields || 0 == fields.length) return resOuter;
 
-    final IndexSchema schema = searcher.getSchema();
     for (String f : fields) {
-      parseParams(FacetParams.FACET_DATE, f);
+      try {
-      f = facetValue;
+        getFacetDateCounts(f, resOuter);
+      } catch (Exception e) {
+        String msg = "Exception during facet.date of " + f;
+        SolrException.logOnce(SolrCore.log, msg, e);
+        addException(msg , e);
+      }
+    }
+
+    return resOuter;
+  }
+
+  public void getFacetDateCounts(String dateFacet, NamedList resOuter)
+      throws IOException, ParseException {
+
+    final IndexSchema schema = searcher.getSchema();
+
+    parseParams(FacetParams.FACET_DATE, dateFacet);
+    String f = facetValue;
 
 
     final NamedList resInner = new SimpleOrderedMap();
@@ -901,9 +944,6 @@ public class SimpleFacets {
     }
   }
 
-    return resOuter;
-  }
-
   
   /**
    * Returns a list of value constraints and the associated facet
@@ -912,18 +952,33 @@ public class SimpleFacets {
    *
    * @see FacetParams#FACET_RANGE
    */
-  public NamedList getFacetRangeCounts()
-    throws IOException, ParseException {
 
+  public NamedList getFacetRangeCounts() {
     final NamedList resOuter = new SimpleOrderedMap();
     final String[] fields = params.getParams(FacetParams.FACET_RANGE);
 
     if (null == fields || 0 == fields.length) return resOuter;
 
-    final IndexSchema schema = searcher.getSchema();
     for (String f : fields) {
-      parseParams(FacetParams.FACET_RANGE, f);
+      try {
-      f = facetValue;
+        getFacetRangeCounts(f, resOuter);
+      } catch (Exception e) {
+        String msg = "Exception during facet.range of " + f;
+        SolrException.logOnce(SolrCore.log, msg, e);
+        addException(msg , e);
+      }
+    }
+
+    return resOuter;
+  }
+
+  void getFacetRangeCounts(String facetRange, NamedList resOuter)
+      throws IOException, ParseException {
+
+    final IndexSchema schema = searcher.getSchema();
+
+    parseParams(FacetParams.FACET_RANGE, facetRange);
+    String f = facetValue;
 
     final SchemaField sf = schema.getField(f);
     final FieldType ft = sf.getType();
@@ -970,9 +1025,6 @@ public class SimpleFacets {
     resOuter.add(key, getFacetRangeCounts(sf, calc));
   }
 
-    return resOuter;
-  }
-
   private <T extends Comparable<T>> NamedList getFacetRangeCounts
     (final SchemaField sf, 
      final RangeEndpointCalculator<T> calc) throws IOException {

solr/src/test/org/apache/solr/TestDistributedSearch.java

@@ -161,6 +161,7 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase {
 
     query("q","*:*", "sort",i1+" desc", "stats", "true", "stats.field", i1);
 
+    /*** TODO: the failure may come back in "exception"
     try {
       // test error produced for field that is invalid for schema
       query("q","*:*", "rows",100, "facet","true", "facet.field",invalidField, "facet.mincount",2);
@@ -168,6 +169,17 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase {
     } catch (SolrServerException ex) {
       // expected
     }
+    ***/
+
+    // Try to get better coverage for refinement queries by turning off over requesting.
+    // This makes it much more likely that we may not get the top facet values and hence
+    // we turn of that checking.
+    handle.put("facet_fields", SKIPVAL);    
+    query("q","*:*", "rows",0, "facet","true", "facet.field",t1,"facet.limit",5, "facet.shard.limit",5);
+    // check a complex key name
+   // query("q","*:*", "rows",0, "facet","true", "facet.field","{!key=a/b/c}"+t1,"facet.limit",5, "facet.shard.limit",5);
+    handle.remove("facet_fields");
+
 
     // index the same document to two servers and make sure things
     // don't blow up.