[
https://issues.apache.org/jira/browse/SOLR-13132?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17073982#comment-17073982
]
Chris M. Hostetter commented on SOLR-13132:
-------------------------------------------
{quote}Given that we need to continue to maintain SKGSlotAcc to support
refinement requests, it's reasonable to continue to support it also as a
configurable option for initial-pass faceting (with sufficient filterCache
capacity, it could be more efficient for low-cardinality fields – analogous to
enum method terms faceting).
{quote}
Sure – but to be clear I don't think it's *just* about refinement, other cases
where the numSlots can (currently) be "1" are "otherAccs" – ie: we're only
returning the calculated value w/o being involved in sorting, or it might be
that {{prelim_sort}} has been used on some other simpler aggregation (or
{{"count"}}) but the {{relatedness()}} function is being used as the {{resort}}
collector.
{quote}The choice to use SKGSlotAcc for slot cardinality of 1 is really just an
indirect way of determining whether the SlotAcc may be used to process a
refinement request (for which sweep accumulation is not applicable – maybe
there's a better/more direct way to determine this?).
{quote}
This is the main reasons i was worried that your current code is brittle
against future changes – these assumptions inside the implementation of
{{RelatednessAgg.createSlotAcc(...)}} about how the facet processors work (and
how many slots they use for various things) that might change down the road.
Hence my suggestion that we make the Processors (that support sweeping) be
*VERY* explicit about when they want {{"Accumulation Structures To Use When
Sweeping"}} – but since there isn't neccessaily a 1-to-1 correspondence between
a {{SlotAcc}} and these accumulation strucutres (ie: a single {{relatedness()}}
function wants to sweep over both it's foreground and background) that's where
my suggestion came from to introduce a new method on {{SlotAcc}} that the
processors can call if and only if they can sweep.
I know i was kind of hand wavy before (largely because it was just stream of
consciousness) but i think what i'm suggesting would be a good API would be
something like this (psuedo-code, still not 100% thought through, but trying to
put "names" on concepts to be able to have a shared vocabulary about what is
flaoting around in my head)...
{code:java}
/** Implemented by some SlotAccs if they are capable of being used for
* sweep collecting in compatable facet processors
*/
interface SweepableSlotAcc {
/**
* called by processors if they support sweeping
* @param baseAcc - never null, where the SlotAcc should call "addChild"
* @return if true then collect methods will never be called on this SlotAcc
public boolean registerSweepingAccs(SweepingAcc baseSweepingAcc);
}
class CountSlotAcc implements SweepableSlotAcc { // TODO: not sure if it
actually needs to implement?
// in addition to implementing registerSweepingAccs (if needed) ...
//
// CountSlotAcc defines a special method for processors to get the
"baseSweepingAcc" ...
/**
* CountSlotAcc always supports being used for sweeping across the base set
* @returns never null
public SweepingAcc getBaseSweepingAcc() {
return ... // build off of fcontext.base;
}
// ...
}
class SKGSlotAcc implements SweepableSlotAcc {
// ... all existing internal strcutures and logic, like ...
private BucketData[] slotvalues;
// ...
/**
* If called, may register SweepingAccs for fg and bg set based on wether
* user indicated sweeping should be used (default)
*
* @returns true if any SweepingAccs were registered since no other
collection is needed for relatedness
*/
public boolean registerSweepingAccs(SweepingAcc baseSweepingAcc) {
if (this.agg.useSweep) { // set by param (or maybe by hueristic if no
param?)
// pass 'slotValues' to our custom SweepingAcc impls to update directly
as collection happens
baseSweepingAcc.addChild(new FGSweepingAcc(slotvalues, fgSet));
baseSweepingAcc.addChild(new BGSweepingAcc(slotvalues, bgSet));
return true;
}
}
class MultiAcc implements SweepableSlotAcc {
// ... all existing logic, plus...
public boolean registerSweepingAccs(SweepingAcc baseSweepingAcc);
// loops over subAccs
// if any return "true" it removes them from subAccs
// if subAccs is empty when it's done, then it return's true; else false
}
/**
* Abstraction used by processors that support sweeping to decide what to sweep
over and how to
* "collect" when doing the sweep.
*
* This class would encapsulate most of hte logic/structures currently in
* SweepCountAccStruct + FilterCtStruct (and maybe CountAccEntry?) and should
allow us to
* simplify the number of new methods in FacetFieldProcessor - they could
instead be encapsulated here
* (where they don't pollute/impact FacetFieldProcessors that don't know/care
about sweeping)
*/
abstract class SweepingAcc {
/** Called by SweepableSlotAccs to register their own SweepingAccs */
public addChild(SweepingAcc child) { ... }
/** Used by ... processors that want a DISI for sweeping ... */
public getSweepingDISI() { /* TODO: build up from ourself and any children */
}
// (I get more hand wavy below here because i still haven't fully groked the
changes needed in the
// the FacetFieldProcessor impls that support sweeping)
// TODO: whatever other methods are needed for knowing what to sweep over
when not DISI based...
public DocSet gettSweepingDocSet() { /* maybe? */ }
// TODO: some callback method(s) for "collecting" things during the sweep
// default impls just loop over children
}
{code}
The net result, should be, i think, that {{FacetFieldProcessor}} doesn't really
need any new logic/API changes, instead {{FacetFieldProcessorByArrayDV}} and
{{UnInvertedField}} can just start doing...
{code}
SweepingAcc sweepAcc = countAcc.getBaseSweepingAcc();
if (collectAcc.registerSweepingAccs(sweepAcc)) {
collectAcc = null;
}
// now use sweepAcc to decide what docs consider & "sweep collect"
// keep using 'collectAcc' as before if non-null
{code}
> Improve JSON "terms" facet performance when sorted by relatedness
> ------------------------------------------------------------------
>
> Key: SOLR-13132
> URL: https://issues.apache.org/jira/browse/SOLR-13132
> Project: Solr
> Issue Type: Improvement
> Components: Facet Module
> Affects Versions: 7.4, master (9.0)
> Reporter: Michael Gibney
> Priority: Major
> Attachments: SOLR-13132-with-cache-01.patch,
> SOLR-13132-with-cache.patch, SOLR-13132.patch
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> When sorting buckets by {{relatedness}}, JSON "terms" facet must calculate
> {{relatedness}} for every term.
> The current implementation uses a standard uninverted approach (either
> {{docValues}} or {{UnInvertedField}}) to get facet counts over the domain
> base docSet, and then uses that initial pass as a pre-filter for a
> second-pass, inverted approach of fetching docSets for each relevant term
> (i.e., {{count > minCount}}?) and calculating intersection size of those sets
> with the domain base docSet.
> Over high-cardinality fields, the overhead of per-term docSet creation and
> set intersection operations increases request latency to the point where
> relatedness sort may not be usable in practice (for my use case, even after
> applying the patch for SOLR-13108, for a field with ~220k unique terms per
> core, QTime for high-cardinality domain docSets were, e.g.: cardinality
> 1816684=9000ms, cardinality 5032902=18000ms).
> The attached patch brings the above example QTimes down to a manageable
> ~300ms and ~250ms respectively. The approach calculates uninverted facet
> counts over domain base, foreground, and background docSets in parallel in a
> single pass. This allows us to take advantage of the efficiencies built into
> the standard uninverted {{FacetFieldProcessorByArray[DV|UIF]}}), and avoids
> the per-term docSet creation and set intersection overhead.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
