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 <[email protected]> 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 <[email protected]> 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: [email protected] > For additional commands, e-mail: [email protected] > >
