OK I removed it in the latest patch on LUCENE-5883.
On Thu, Aug 14, 2014 at 8:17 AM, Robert Muir <[email protected]> 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 <[email protected]> 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 <[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] > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
