I agree w/ two variants for addIndexes. About the optimize concerns - I somehow doubt there are apps with tens or hundreds of segments in the target directory, to which they would like to add yet another index. Might be, but hard to believe. Anyway, apps could still call optimize() before that. Note that on the issue I've mentioned that the jdoc of addIndexes is wrong, saying after the method completes the index is optimized.
Maybe what we should do is add just the external indexes, as you suggest on the issue, and don't touch the local ones. Then, apps could call maybeMerge following that, or optimize, whatever they prefer. I think we can move the discussion to the issue. Shai On Tue, May 11, 2010 at 12:55 PM, Michael McCandless < luc...@mikemccandless.com> wrote: > On Mon, May 10, 2010 at 11:38 PM, Shai Erera <ser...@gmail.com> wrote: > >> Hmm addDirectories feels a bit too low level > > > > I don't mind calling it addIndexes(Directory...), but I don't think it's > too > > low level - whoever executes the method passes Directory... and that's > > exactly what the method does :). Two addIndexes force you to go read the > > jdoc, but so will addDirectories. I don't mind either way. > > I also like addIndexes because it better reflects what it does, ie you > are in fact adding indexes, it's just that the indexes are delivered > via a Directory vs via an IndexReader instance. Also, putting the > type of the params seems sort of redundant since it's already > declared/visible in the method's sig. > > >> But advertise this in back compat breaks. > > > > I don't think it's a bw break? More of a runtime change IMO. True it can > > Sorry, you're right, it's a runtime change. But for users who > actually "rely" on it, this is a big change. > > > affect performance, but did we ever measure addIndexes w/ and w/o > > optimize(). Are we sure that optimize() first, then SM merges that follow > > perform better? I mean, on paper it should. But since we do it only for > the > > Really the question is perf effect of changing mergeFactor, because > this method adds in current index's segment, and all incoming > segments, and does one giant merge. > > If we remove the optimize, and there are too many segments in the > index, and the app doesn't use CFS, they can run out of file > descriptors due to this. > > > target index, we don't really know what's happening in users' apps. It's > > only documentation in CHANGES, so it can go into both sections (BACKWARDS > or > > RUNTIME). I prefer the latter. It can also go like that into trunk's > > CHANGES. > > I agree, it should go into RUNTIME and CHANGES. > > > > >> Good! > > > > I'll open an issue to track this. > > Thanks! > > Mike > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > >
