diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f62398e898b..7f68d8aebd5 100644 --- a/solr/CHANGES.txt +++ b/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 to enable listing all exceptions that happened, and + prevent an exception in one facet command from affecting another + facet command. (yonik) + Other Changes ---------------------- diff --git a/solr/src/java/org/apache/solr/handler/component/FacetComponent.java b/solr/src/java/org/apache/solr/handler/component/FacetComponent.java index f5f3a89b344..339f5997356 100644 --- a/solr/src/java/org/apache/solr/handler/component/FacetComponent.java +++ b/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 - // thus cause more facet refinement queries. - // if (dff.limit > 0) dff.initialLimit = dff.offset + dff.limit; + // Currently this is for testing only and allows overriding of the + // facet.limit set to the shards + 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) { @@ -256,17 +258,11 @@ 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) { - 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); + + if (facet_fields != null) { + 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 queryFacets; public LinkedHashMap facets; + public List exceptionList; void parse(SolrParams params, ResponseBuilder rb) { queryFacets = new LinkedHashMap(); @@ -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); diff --git a/solr/src/java/org/apache/solr/request/SimpleFacets.java b/solr/src/java/org/apache/solr/request/SimpleFacets.java index df962389bd0..ec20a7b8014 100644 --- a/solr/src/java/org/apache/solr/request/SimpleFacets.java +++ b/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 { - - res.add("facet_queries", getFacetQueryCounts()); - res.add("facet_fields", getFacetFieldCounts()); - res.add("facet_dates", getFacetDateCounts()); - res.add("facet_ranges", getFacetRangeCounts()); + facetResponse.add("facet_queries", getFacetQueryCounts()); + facetResponse.add("facet_fields", getFacetFieldCounts()); + facetResponse.add("facet_dates", getFacetDateCounts()); + facetResponse.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,13 +227,21 @@ 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) { - parseParams(FacetParams.FACET_QUERY, q); + 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)); + // 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); + } } } @@ -325,12 +345,18 @@ public class SimpleFacets { String[] facetFs = params.getParams(FacetParams.FACET_FIELD); if (null != facetFs) { for (String f : facetFs) { - parseParams(FacetParams.FACET_FIELD, f); - String termList = localParams == null ? null : localParams.get(CommonParams.TERMS); - if (termList != null) { - res.add(key, getListedTermCounts(facetValue, termList)); - } else { - res.add(key, getTermCounts(facetValue)); + try { + parseParams(FacetParams.FACET_FIELD, f); + String termList = localParams == null ? null : localParams.get(CommonParams.TERMS); + if (termList != null) { + res.add(key, getListedTermCounts(facetValue, termList)); + } 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); } } } @@ -753,155 +779,169 @@ public class SimpleFacets { * * @see FacetParams#FACET_DATE */ + public NamedList getFacetDateCounts() throws IOException, ParseException { final NamedList resOuter = new SimpleOrderedMap(); final String[] fields = params.getParams(FacetParams.FACET_DATE); - + if (null == fields || 0 == fields.length) return resOuter; - - final IndexSchema schema = searcher.getSchema(); + for (String f : fields) { - parseParams(FacetParams.FACET_DATE, f); - f = facetValue; - - - final NamedList resInner = new SimpleOrderedMap(); - resOuter.add(key, resInner); - final SchemaField sf = schema.getField(f); - if (! (sf.getType() instanceof DateField)) { - throw new SolrException - (SolrException.ErrorCode.BAD_REQUEST, - "Can not date facet on a field which is not a DateField: " + f); + try { + getFacetDateCounts(f, resOuter); + } catch (Exception e) { + String msg = "Exception during facet.date of " + f; + SolrException.logOnce(SolrCore.log, msg, e); + addException(msg , e); } - final DateField ft = (DateField) sf.getType(); - final String startS + } + + 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(); + resOuter.add(key, resInner); + final SchemaField sf = schema.getField(f); + if (! (sf.getType() instanceof DateField)) { + throw new SolrException + (SolrException.ErrorCode.BAD_REQUEST, + "Can not date facet on a field which is not a DateField: " + f); + } + final DateField ft = (DateField) sf.getType(); + final String startS = required.getFieldParam(f,FacetParams.FACET_DATE_START); - final Date start; - try { - start = ft.parseMath(NOW, startS); - } catch (SolrException e) { - throw new SolrException + final Date start; + try { + start = ft.parseMath(NOW, startS); + } catch (SolrException e) { + throw new SolrException (SolrException.ErrorCode.BAD_REQUEST, - "date facet 'start' is not a valid Date string: " + startS, e); - } - final String endS + "date facet 'start' is not a valid Date string: " + startS, e); + } + final String endS = required.getFieldParam(f,FacetParams.FACET_DATE_END); - Date end; // not final, hardend may change this - try { - end = ft.parseMath(NOW, endS); - } catch (SolrException e) { - throw new SolrException + Date end; // not final, hardend may change this + try { + end = ft.parseMath(NOW, endS); + } catch (SolrException e) { + throw new SolrException (SolrException.ErrorCode.BAD_REQUEST, - "date facet 'end' is not a valid Date string: " + endS, e); - } - - if (end.before(start)) { - throw new SolrException + "date facet 'end' is not a valid Date string: " + endS, e); + } + + if (end.before(start)) { + throw new SolrException (SolrException.ErrorCode.BAD_REQUEST, - "date facet 'end' comes before 'start': "+endS+" < "+startS); - } + "date facet 'end' comes before 'start': "+endS+" < "+startS); + } - final String gap = required.getFieldParam(f,FacetParams.FACET_DATE_GAP); - final DateMathParser dmp = new DateMathParser(ft.UTC, Locale.US); - dmp.setNow(NOW); + final String gap = required.getFieldParam(f,FacetParams.FACET_DATE_GAP); + final DateMathParser dmp = new DateMathParser(ft.UTC, Locale.US); + dmp.setNow(NOW); - final int minCount = params.getFieldInt(f,FacetParams.FACET_MINCOUNT, 0); + final int minCount = params.getFieldInt(f,FacetParams.FACET_MINCOUNT, 0); - final EnumSet include = FacetRangeInclude.parseParam + final EnumSet include = FacetRangeInclude.parseParam (params.getFieldParams(f,FacetParams.FACET_DATE_INCLUDE)); - try { - Date low = start; - while (low.before(end)) { - dmp.setNow(low); - String label = ft.toExternal(low); - - Date high = dmp.parseMath(gap); - if (end.before(high)) { - if (params.getFieldBool(f,FacetParams.FACET_DATE_HARD_END,false)) { - high = end; - } else { - end = high; - } + try { + Date low = start; + while (low.before(end)) { + dmp.setNow(low); + String label = ft.toExternal(low); + + Date high = dmp.parseMath(gap); + if (end.before(high)) { + if (params.getFieldBool(f,FacetParams.FACET_DATE_HARD_END,false)) { + high = end; + } else { + end = high; } - if (high.before(low)) { - throw new SolrException + } + if (high.before(low)) { + throw new SolrException (SolrException.ErrorCode.BAD_REQUEST, - "date facet infinite loop (is gap negative?)"); - } - final boolean includeLower = + "date facet infinite loop (is gap negative?)"); + } + final boolean includeLower = (include.contains(FacetRangeInclude.LOWER) || - (include.contains(FacetRangeInclude.EDGE) && low.equals(start))); - final boolean includeUpper = + (include.contains(FacetRangeInclude.EDGE) && low.equals(start))); + final boolean includeUpper = (include.contains(FacetRangeInclude.UPPER) || - (include.contains(FacetRangeInclude.EDGE) && high.equals(end))); + (include.contains(FacetRangeInclude.EDGE) && high.equals(end))); - final int count = rangeCount(sf,low,high,includeLower,includeUpper); - if (count >= minCount) { - resInner.add(label, count); - } - low = high; + final int count = rangeCount(sf,low,high,includeLower,includeUpper); + if (count >= minCount) { + resInner.add(label, count); } - } catch (java.text.ParseException e) { - throw new SolrException - (SolrException.ErrorCode.BAD_REQUEST, - "date facet 'gap' is not a valid Date Math string: " + gap, e); + low = high; } - - // explicitly return the gap and end so all the counts - // (including before/after/between) are meaningful - even if mincount - // has removed the neighboring ranges - resInner.add("gap", gap); - resInner.add("start", start); - resInner.add("end", end); + } catch (java.text.ParseException e) { + throw new SolrException + (SolrException.ErrorCode.BAD_REQUEST, + "date facet 'gap' is not a valid Date Math string: " + gap, e); + } - final String[] othersP = + // explicitly return the gap and end so all the counts + // (including before/after/between) are meaningful - even if mincount + // has removed the neighboring ranges + resInner.add("gap", gap); + resInner.add("start", start); + resInner.add("end", end); + + final String[] othersP = params.getFieldParams(f,FacetParams.FACET_DATE_OTHER); - if (null != othersP && 0 < othersP.length ) { - final Set others = EnumSet.noneOf(FacetRangeOther.class); + if (null != othersP && 0 < othersP.length ) { + final Set others = EnumSet.noneOf(FacetRangeOther.class); - for (final String o : othersP) { - others.add(FacetRangeOther.get(o)); + for (final String o : othersP) { + others.add(FacetRangeOther.get(o)); + } + + // no matter what other values are listed, we don't do + // anything if "none" is specified. + if (! others.contains(FacetRangeOther.NONE) ) { + boolean all = others.contains(FacetRangeOther.ALL); + + if (all || others.contains(FacetRangeOther.BEFORE)) { + // include upper bound if "outer" or if first gap doesn't already include it + resInner.add(FacetRangeOther.BEFORE.toString(), + rangeCount(sf,null,start, + false, + (include.contains(FacetRangeInclude.OUTER) || + (! (include.contains(FacetRangeInclude.LOWER) || + include.contains(FacetRangeInclude.EDGE)))))); } - - // no matter what other values are listed, we don't do - // anything if "none" is specified. - if (! others.contains(FacetRangeOther.NONE) ) { - boolean all = others.contains(FacetRangeOther.ALL); - - if (all || others.contains(FacetRangeOther.BEFORE)) { - // include upper bound if "outer" or if first gap doesn't already include it - resInner.add(FacetRangeOther.BEFORE.toString(), - rangeCount(sf,null,start, - false, - (include.contains(FacetRangeInclude.OUTER) || - (! (include.contains(FacetRangeInclude.LOWER) || - include.contains(FacetRangeInclude.EDGE)))))); - } - if (all || others.contains(FacetRangeOther.AFTER)) { - // include lower bound if "outer" or if last gap doesn't already include it - resInner.add(FacetRangeOther.AFTER.toString(), - rangeCount(sf,end,null, - (include.contains(FacetRangeInclude.OUTER) || - (! (include.contains(FacetRangeInclude.UPPER) || - include.contains(FacetRangeInclude.EDGE)))), - false)); - } - if (all || others.contains(FacetRangeOther.BETWEEN)) { - resInner.add(FacetRangeOther.BETWEEN.toString(), - rangeCount(sf,start,end, - (include.contains(FacetRangeInclude.LOWER) || - include.contains(FacetRangeInclude.EDGE)), - (include.contains(FacetRangeInclude.UPPER) || - include.contains(FacetRangeInclude.EDGE)))); - } + if (all || others.contains(FacetRangeOther.AFTER)) { + // include lower bound if "outer" or if last gap doesn't already include it + resInner.add(FacetRangeOther.AFTER.toString(), + rangeCount(sf,end,null, + (include.contains(FacetRangeInclude.OUTER) || + (! (include.contains(FacetRangeInclude.UPPER) || + include.contains(FacetRangeInclude.EDGE)))), + false)); + } + if (all || others.contains(FacetRangeOther.BETWEEN)) { + resInner.add(FacetRangeOther.BETWEEN.toString(), + rangeCount(sf,start,end, + (include.contains(FacetRangeInclude.LOWER) || + include.contains(FacetRangeInclude.EDGE)), + (include.contains(FacetRangeInclude.UPPER) || + include.contains(FacetRangeInclude.EDGE)))); } } } - - return resOuter; } @@ -912,65 +952,77 @@ 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); - f = facetValue; - - final SchemaField sf = schema.getField(f); - final FieldType ft = sf.getType(); - - RangeEndpointCalculator calc = null; - if (ft instanceof TrieField) { - final TrieField trie = (TrieField)ft; - - switch (trie.getType()) { - case FLOAT: + if (null == fields || 0 == fields.length) return resOuter; + + for (String f : fields) { + try { + 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(); + + RangeEndpointCalculator calc = null; + + if (ft instanceof TrieField) { + final TrieField trie = (TrieField)ft; + + switch (trie.getType()) { + case FLOAT: calc = new FloatRangeEndpointCalculator(sf); break; - case DOUBLE: + case DOUBLE: calc = new DoubleRangeEndpointCalculator(sf); break; - case INTEGER: + case INTEGER: calc = new IntegerRangeEndpointCalculator(sf); break; - case LONG: + case LONG: calc = new LongRangeEndpointCalculator(sf); break; default: throw new SolrException - (SolrException.ErrorCode.BAD_REQUEST, - "Unable to range facet on tried field of unexpected type:" + f); - } - } else if (ft instanceof DateField) { - calc = new DateRangeEndpointCalculator(sf, NOW); - } else if (ft instanceof SortableIntField) { - calc = new IntegerRangeEndpointCalculator(sf); - } else if (ft instanceof SortableLongField) { - calc = new LongRangeEndpointCalculator(sf); - } else if (ft instanceof SortableFloatField) { - calc = new FloatRangeEndpointCalculator(sf); - } else if (ft instanceof SortableDoubleField) { - calc = new DoubleRangeEndpointCalculator(sf); - } else { - throw new SolrException - (SolrException.ErrorCode.BAD_REQUEST, - "Unable to range facet on field:" + sf); + (SolrException.ErrorCode.BAD_REQUEST, + "Unable to range facet on tried field of unexpected type:" + f); } - - resOuter.add(key, getFacetRangeCounts(sf, calc)); + } else if (ft instanceof DateField) { + calc = new DateRangeEndpointCalculator(sf, NOW); + } else if (ft instanceof SortableIntField) { + calc = new IntegerRangeEndpointCalculator(sf); + } else if (ft instanceof SortableLongField) { + calc = new LongRangeEndpointCalculator(sf); + } else if (ft instanceof SortableFloatField) { + calc = new FloatRangeEndpointCalculator(sf); + } else if (ft instanceof SortableDoubleField) { + calc = new DoubleRangeEndpointCalculator(sf); + } else { + throw new SolrException + (SolrException.ErrorCode.BAD_REQUEST, + "Unable to range facet on field:" + sf); } - - return resOuter; + + resOuter.add(key, getFacetRangeCounts(sf, calc)); } private > NamedList getFacetRangeCounts diff --git a/solr/src/test/org/apache/solr/TestDistributedSearch.java b/solr/src/test/org/apache/solr/TestDistributedSearch.java index 33629713d3a..d0a22a8a106 100755 --- a/solr/src/test/org/apache/solr/TestDistributedSearch.java +++ b/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.