Simon, sorry about that: MP = MergePolicy, LMP = LogMergePolicy. WTF = ... well I know what that is :-).

In 3x, IW.getLogMergePolicy which is called by some public (albeit deprecated) API throws IllegalArgumentException if MP is not LMP. In trunk this API is gone now (thanks to Earwin?), so it's less of a problem. Still, IW queries instanceof LMP just to know whether it should create a compound file, which is a setting unrelated to LMP. If I write my own MP, does it mean IW will never create compound segments?

Hmm .. now that I look closely at it, MP has useCompundFile/DocStore methods, and LMP just adds getUseCompoundFile(). Why?
And IndexWriter.addIndexes(IndexReader...) queries instanceof LMP, instead of calling mp.useCompoundFile()?

So perhaps we should:

1) Fix IW to not case to LMP just to ask if it should create compound files or not. And then we can perhaps remove getLogMergePolicy from IW on 3x, and also removing the source for confusion.

2) Look at LMP and decide if there are method we believe can be placed on a general MP, such as mergeFactor or maxMergeDocs. LogMP is special in how it picks segments for merge - that is, log-based (levels). But maxMergeDocs, maxMergeSize, mergeFactor, are unrelated to log/levels. This is the sort of functionality I'd expect to find on a general MP impl.

Shai

On Thu, Dec 2, 2010 at 11:44 AM, Earwin Burrfoot <earwin@gmail.com> wrote:
Actually, in trunk IW doesn't break on anything else.

There's one private no-longer-used method I forgot to delete on my
drop-all-deprecations spree.
And there's a block in addIndexes, that explicitly checks instanceof,
and only then casts to LMP.

I'm against consolidating MP and LMP. MP is a damn interface!
We should strive to make things less coupled rather than other way around.

On Thu, Dec 2, 2010 at 12:25, Shai Erera <serera@gmail.com> wrote:
> Hi
>
> While IndexWriter declares it accepts a general MP, it will actually fail if
> the given instance is not LogMP. So I wonder if we shouldn't consolidate
> both of them into one, and pull up all of LMP features to MP. I think all of
> LMP's features are useful for any kind of MP, and if someone wants to ignore
> them he still can.
>
> This is not the sort of change that fits well in trunk. IMO it can fit well
> in 3x too since IW didn't accept anything that is not LMP. So even if it
> will appear we're breaking back-compat, we actually won't. Which is another
> reason, for me, why those two should be consolidated.
>
> What do you think?
>
> Shai
>



--
Kirill Zakharenko/Кирилл Захаренко (earwin@gmail.com)
Phone: +7 (495) 683-567-4
ICQ: 104465785

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org