[
https://issues.apache.org/jira/browse/SOLR-2894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14054293#comment-14054293
]
Hoss Man commented on SOLR-2894:
--------------------------------
bq. ... – but as things stand right now there is still a nasty bug somewhere in
the facet.missing processing that i can't wrap my head arround...
I spent today doing some manual testing with some small amounts of data, and
looking at the shard requests triggered by each request. I then started
reading through more of the refinement code (first time i've looked at a fair
bit of this) and i think i've figured out what's going on (but i don't have a
fix for it yet)...
Basically: the {{PivotFacetField}} class, that holds a
{{List<PivotFacetValue>}} doesn't do anything special as far as keeping track
of the PivotFacetValue that represents the {{facet.missing}} value (ie: the
PivotFacetValue where {{PivotFacetValue.value==null}}). This means that in
methods like {{PivotFacetField.sort()}} and
{{PivotFacetField.queuePivotRefinementRequests(...)}} the PivotFacetValue for
{{facet.missing}} is mixed in with the other values and included in
considerations about what the cutoff "countThreshold" is for refinement, _even
though it's not affected by {{facet.limit}} and should always be returned_
This means that in the test i added that has
{{facet.limit=1&facet.missing=true}} the {{null}} vaue from {{facet.missing}}
is the only value considered in the "top 1" of the constraints, and has a count
much higher then the count for the SPECIAL value -- which means SPECIAL doesn't
even qualify for the {{processPossibleCandidateElement}} logic so it never gets
refined at all.
--
I think the best course of action is to cleanup {{PivotFacetField}} a bit, so
that in addition to the {{List<PivotFacetValue>}} of values that are subject to
the facet.limit, a specfic "missingValue" variable should be added to track the
corrisponding {{PivotFacetValue}} -- this should make the value sorting &
refinement logic in {{queuePivotRefinementRequests()}} accurate as is, at the
cost of slightly more complex (but accurately modeled) logic in
{{createFromListOfNamedLists()}} and {{convertToListOfNamedLists()}}.
What do folks think?
----
bq. The endless loop is definitely a concern and we will focus on that first if
your changes haven't already fixed that.
The root cause of the infinite loop seems to be that the formating/parsing of
of the refinment params wasn't in sync (ie: empty strings weren't being
included at all, while facet.missing values were being encoded as "null" which
owuld then be parsed as 4 character string literals) ... so that _cause_ should
be fixed in my latest patch.
What still concerns me though is that there is evidently no general sanity
check in the code to prevent the distributed logic in the coordinator from
retrying to refine values over and over again even if the shard never responses
back with a number for it (ie: if some future bug gets introduced in the refine
code that runs on the shards, or if some shard has been misconfigured to have a
hard coded invaraint of {{facet=false}}, etc...). That's the sort of edge case
that may be really hard to test for, but even if we can't explicit test it, we
should at least have some sanity check in the distrib coordination code that
says "we already asked shardX for refinementY and still don't have it, throw
5xx error!" instead of "still need refinementY from shardX, ..., still need
refinementY from shardX, ..." which is what seems to be happening right now.
bq. What we are wondering is if you feel we could get a preliminary version of
this committed if we can resolve that loop?
I'm not comfortable committing features unless i know they work -- particularly
something like this, where it's adding distributed support to an existing core
feature. I don't want existing pivot users to see "oh, distributed pivot
support has been added" and upgrade to SolrCloud and then start getting
silently incorrect results.
Rest assured however: I'm dedicated to continuing to working through this
issue, and helping to fix whatever bugs we find, until it's ready to be
committed. I won't leave you hanging.
----
bq. Hey Hoss, I should have time this week and next to investigate the infinite
loop and try to implement some of your other requests.
That's great -- like i mentioned above, i think a sanity check on the infinite
loop is important, but i suspect it should be fairly trivial (i'm just not 100%
certain where it makes sense to put it yet)
I think the biggest concern right now however is addressing the bugs with how
{{facet.missing}} impacts refinement and kicks value values out of contention
due to the modeling in {{PivotFacetField}}
If you have time, and can help out with making those changes, I can go ahead
and focus on the additional tests i was describing -- which is probably the
best way to divide & conquer the problem since you guys already know the code
internals better then me. Then you can help review my tests, and i can help
review the {{PivotFacetField}} changes.
sound good?
> Implement distributed pivot faceting
> ------------------------------------
>
> Key: SOLR-2894
> URL: https://issues.apache.org/jira/browse/SOLR-2894
> Project: Solr
> Issue Type: Improvement
> Reporter: Erik Hatcher
> Assignee: Hoss Man
> Fix For: 4.9, 5.0
>
> Attachments: SOLR-2894-mincount-minification.patch,
> SOLR-2894-reworked.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch,
> SOLR-2894_cloud_test.patch, dateToObject.patch, pivot_mincount_problem.sh
>
>
> Following up on SOLR-792, pivot faceting currently only supports
> undistributed mode. Distributed pivot faceting needs to be implemented.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
