[ 
https://issues.apache.org/jira/browse/LUCENE-847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12523957
 ] 

Ning Li commented on LUCENE-847:
--------------------------------

> True, but I was thinking CMPW could be an exception to this rule.  I
> guess I would change the rule to "simple merge policies don't have to
> run their own merges"?

:) Let's see if we have to make that exception.

> Good point...  I think I could refactor this so that cascading logic
> lives entirely in one place IndexWriter.

Another problem of the current cascading in CMPW.MergeThread is, if multiple 
candidate merges are found, all of them are added to 
IndexWriter.mergingSegments. But all but the first should be removed because 
only the first merge is carried out (thus removed from mergeSegments after the 
merge is done).

How do you make cascading live entirely in IndexWriter? Just removing cascading 
from CMPW.MergeThread has one drawback. For example, segment sizes of an index 
are: 40, 20, 10, buffer size is 10 and merge factor is 2. A buffer full flush 
of 10 will trigger merge of 10 & 10, then cascade to 20 & 20, then cascade to 
40 & 40. CMPW without cascading will stop after 10 & 10 since 
IndexWriter.maybeMerge has already returned. Then we have to wait for the next 
flush to merge 20 & 20.

> How would this be used?  Ie, how would one make an IndexWriter that
> uses the ConcurrentMerger?  Would we add expert methods
> IndexWriter.set/getIndexMerger(...)?  (And presumably the mergePolicy
> is now owned by IndexMerger so it would have the
> set/getMergePolicy(...)?)
> 
> Also, how would you separate what remains in IW vs what would be in
> IndexMerger?

Maybe Merger does and only does merge (so IndexWriter still owns MergePolicy)? 
Say, base class Merger.merge simply calls IndexWriter.merge. 
ConcurrentMerger.merge creates a merge thread if possible. Otherwise it calls 
super.merge, which does non-concurrent merge. IndexWriter simply calls its 
merger's merge instead of its own merge. Everything else remains in IndexWriter.


1
> Hmm ... you're right.  This is a separate issue from merge policy,
> right?  Are you proposing buffering deletes in DocumentsWriter
> instead?

Yes, this is a separate issue. And yes if we consider DocumentsWriter as 
staging area.

2
> Good catch!  How to fix?  One thing we could do is always use
> SegmentInfo.reset(...) and never swap in clones at the SegmentInfo
> level.  This way using the default 'equals' (same instance) would
> work.  Or we could establish identity (equals) of a SegmentInfo as
> checking if the directory plus segment name are equal?  I think I'd
> lean to the 2nd option....

I think the 2nd option is better.

3
> Hmmm yes.  In fact I think we can remove synchronized from optimize
> altogether since within it we are synchronizing(this) at the right
> places?  If more than one thread calls optimize at once, externally,
> it is actually OK: they will each pick a merge that's viable
> (concurrently) and will run the merge, else return once there is no
> more concurrency left.  I'll add a unit test that confirms this.

That seems to be the case. The fact that "the same merge spec will be returned 
without changes to segmentInfos" reminds me: MergePolicy.findCandidateMerges 
finds merges which may not be eligible; but CMPW checks for eligibility when 
looking for candidate merges. Maybe we should unify the behaviour? BTW, 
MergePolicy.optimize (a rename?) doesn't check for eligibility either.

4
> Well, useCompoundFile(...) is given a single newly flushed segment and
> should decide whether it should be CFS.  Whereas
> useCompoundDocStore(...) is called when doc stores are flushed.  When
> autoCommit=false, segments can share a single set of doc stores, so
> there's no single SegmentInfo to pass down.

The reason I asked is because none of them are used right now. So they might be 
used in the future?

> Factor merge policy out of IndexWriter
> --------------------------------------
>
>                 Key: LUCENE-847
>                 URL: https://issues.apache.org/jira/browse/LUCENE-847
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Steven Parkes
>            Assignee: Steven Parkes
>             Fix For: 2.3
>
>         Attachments: concurrentMerge.patch, LUCENE-847.patch.txt, 
> LUCENE-847.patch.txt, LUCENE-847.take3.patch, LUCENE-847.take4.patch, 
> LUCENE-847.txt
>
>
> If we factor the merge policy out of IndexWriter, we can make it pluggable, 
> making it possible for apps to choose a custom merge policy and for easier 
> experimenting with merge policy variants.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to