lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hoss Man (JIRA)" <>
Subject [jira] [Commented] (LUCENE-5527) Make the Collector API work per-segment
Date Thu, 03 Apr 2014 18:51:16 GMT


Hoss Man commented on LUCENE-5527:

This isn't an area of Lucene i tend to think much about, but if we're talking about changing
the API, i have a few questions i wanted to put out there for discussion:

* is {{setNextReader}} really an appropriate name anymore? or should it be something like
* Should we bite the bullet and make {{LeafCollector}} and {{Collector}} extend {{Closable}}
** I believe this was discussed at one point before, and it was considered an obnoxious interface
change with minimal gain - but if we're going to change the interface API now anyway, it may
be worth reconsidering
** Solr works around this by always using {{DelegatingCollector}} which has a {{finish()}}
method - i can't imagine other lucene apps don't have similar wishes for {{close()}} like
* Should we go ahead and think about if/how this API should be tweaked (now or in the future)
to allow a {{Collector}} to indicate that it's {{LeafCollectors}}  can be used to collect
documents from different segments in parallel threads? If we think a marker interface would
be the best way to go, so we can easily add that later -- but i wanted to ask the question
** One possibility that just occurred to me: Document right now that {{LeafCollectors}} _may_
be processed in parallel, and that if you want to force single threaded collection you should
implement {{Collector.getLeafCollector(AtomicReaderContext)}} such that it blocks until the
previously returned {{LeafCollector}} has been closed (which should be easy enough to do in
things like {{SimpleCollector}} using a basic sync lock, right?)

(I have no strong feelings about most of this)

> Make the Collector API work per-segment
> ---------------------------------------
>                 Key: LUCENE-5527
>                 URL:
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Adrien Grand
>            Priority: Minor
>             Fix For: 5.0
>         Attachments: LUCENE-5527.patch
> Spin-off of LUCENE-5299.
> LUCENE-5229 proposes different changes, some of them being controversial, but there is
one of them that I really really like that consists in refactoring the {{Collector}} API in
order to have a different Collector per segment.
> The idea is, instead of having a single Collector object that needs to be able to take
care of all segments, to have a top-level Collector:
> {code}
> public interface Collector {
>   AtomicCollector setNextReader(AtomicReaderContext context) throws IOException;
> }
> {code}
> and a per-AtomicReaderContext collector:
> {code}
> public interface AtomicCollector {
>   void setScorer(Scorer scorer) throws IOException;
>   void collect(int doc) throws IOException;
>   boolean acceptsDocsOutOfOrder();
> }
> {code}
> I think it makes the API clearer since it is now obious {{setScorer}} and {{acceptDocsOutOfOrder}}
need to be called after {{setNextReader}} which is otherwise unclear.
> It also makes things more flexible. For example, a collector could much more easily decide
to use different strategies on different segments. In particular, it makes the early-termination
collector much cleaner since it can return different atomic collectors implementations depending
on whether the current segment is sorted or not.
> Even if we have lots of collectors all over the place, we could make it easier to migrate
by having a Collector that would implement both Collector and AtomicCollector, return {{this}}
in setNextReader and make current concrete Collector implementations extend this class instead
of directly extending Collector.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message