lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Re: Who should close MergePolicy
Date Thu, 14 Aug 2014 05:38:56 GMT
OK I removed it in the latest patch on LUCENE-5883.


On Thu, Aug 14, 2014 at 8:17 AM, Robert Muir <rcmuir@gmail.com> wrote:

> Yeah, I agree. We should only add such semantics if necessary. But
> close() definitely seems wrong so lets just start with nuking that?
>
> On Thu, Aug 14, 2014 at 1:08 AM, Shai Erera <serera@gmail.com> wrote:
> > OK I will remove it under LUCENE-5883.
> >
> > And I like the idea of adding ref-counting semantics, but we should do it
> > separately and only when there's a real reason too. Then, we can make
> sure
> > IW inc/decRef appropriately.
> >
> > Shai
> >
> >
> > On Thu, Aug 14, 2014 at 8:05 AM, Robert Muir <rcmuir@gmail.com> wrote:
> >>
> >> Was there a reasoning for adding this method in the first place? What
> was
> >> it?
> >>
> >> Was it accidental?
> >>
> >> +1 for straightening this out: seems like if MP should support
> >> multiple indexwriter instances then it should have a reference
> >> counting API rather than abusing closeable semantics!
> >>
> >> On Wed, Aug 13, 2014 at 9:00 AM, Shai Erera <serera@gmail.com> wrote:
> >> > Hi
> >> >
> >> > While working on LUCENE-5883, I noticed that IndexWriter calls
> >> > mergePolicy.close(). But since LUCENE-5711, where we no longer wire an
> >> > MP to
> >> > a single IndexWriter instance, I don't think IW should call
> MP.close(),
> >> > e.g.
> >> > in case the app shares this MP across several IWs?
> >> >
> >> > But then, in the common case where an MP is used by a single IW
> >> > instance,
> >> > having IW close it is convenient.
> >> >
> >> > I looked at all MPs we have, none seem to implement close() in any
> >> > special
> >> > way (empty impls, or delegating impls are the only ones I found). So
> >> > question is:
> >> >
> >> > 1) Is it a bug that IW calls mp.close()? I think so, but would like to
> >> > confirm with others.
> >> >
> >> > 2) If it is a bug, and we're going to say that you should close your
> MP
> >> > separately, I wonder why we need to have Closeable on MP at all. If
> >> > someone
> >> > uses an MP which should be closed, he can just close it? This stems
> from
> >> > the
> >> > fact that none of the existing MPs in trunk really implement close(),
> >> > yet we
> >> > advertise that you should close MP since it implements Closeable.
> >> >
> >> > Shai
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> >> For additional commands, e-mail: dev-help@lucene.apache.org
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Mime
View raw message