Question about aborting background merges

[Michael Busch <bu...@gmail.com>]

Hi,

I have a question about IndexWriter.close(false) and background merges.
I was going to take a look at the code, but I'm sure that Mike knows the
answer :-). Let's assume that a long background merge is going on and
close(false) is called. Then the merges are marked as aborted and
IndexWriter.close() returns after flushing DocumentsWriter's buffers.
The background merge threads keep going. Now a new IndexWriter is opened
and optimize() is called. Can it happen that optimize() tries to create
a segment with the same name the background threads are still working
on? Then the new IndexWriter would probably hit an IOException? Or would
the new IndexWriter use different file names for the merged segments?

-Michael

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

Re: Question about aborting background merges

[Michael Busch <bu...@gmail.com>]

Michael McCandless wrote:
> 
> Well that certainly sounds spooky!  You mean this test is using the
> one of IndexWriter's ctors that relies on IndexReader.indexExists() to
> decide whether to pass create=true?
> 

Not sure, but I'll try to find out today.

-Michael

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

Re: Question about aborting background merges

[Michael McCandless <lu...@mikemccandless.com>]

"Michael Busch" <bu...@gmail.com> wrote:
> Michael McCandless wrote:
> 
> > Hmm ... looking at the code, I think we should also check whether a
> > merge was aborted in mergeInit.  It looks like there is a window from
> > when a merge is handed out until when it is inited such that if the
> > writer is closed in that window it could result in incorrect re-use of a
> > segment name.  Michael are you seeing such a case?
> > 
> 
> Thanks for the explanation! I'm not seeing this particular issue.
> However, a very weird thing happened in one of our stress tests. I
> believe in that test a script would push a bunch of docs into the
> IndexWriter, then close(false) the writer and open a new one immediately
> afterwards. This test ran for quite a while without any problems (up to
> 8M docs). Then the weird thing happened: suddenly the index was totally
> wiped out, meaning only the segments.gen and segments_1 were in the
> index directory. There was no IOException in the log.
> 
> I must say that I didn't write the stress test and haven't seen it yet
> either, I'm planning to take a look next week if I can. It looks like it
> might be an application bug, such that it might open an IndexWriter with
> create=true, because the segments file has the initial generation 1.
> 
> I started looking if the constructors of IndexWriter that automatically
> try to determine if the index has to be created or not might have a
> problem, maybe even in combination with aborted, but still running
> background merge threads. I think the question is if
> SegmentInfos.getCurrentSegmentGeneration() could ever falsely return -1,
> but it doesn't look like this is possible.
> 
> I'm just really guessing here, it might not be a Lucene problem at all,
> but at this moment I can't rule this possibility out.

Well that certainly sounds spooky!  You mean this test is using the
one of IndexWriter's ctors that relies on IndexReader.indexExists() to
decide whether to pass create=true?

Keep up posted!

Mike

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

Re: Question about aborting background merges

[Michael Busch <bu...@gmail.com>]

Michael McCandless wrote:

> Hmm ... looking at the code, I think we should also check whether a
> merge was aborted in mergeInit.  It looks like there is a window from
> when a merge is handed out until when it is inited such that if the
> writer is closed in that window it could result in incorrect re-use of a
> segment name.  Michael are you seeing such a case?
> 

Thanks for the explanation! I'm not seeing this particular issue.
However, a very weird thing happened in one of our stress tests. I
believe in that test a script would push a bunch of docs into the
IndexWriter, then close(false) the writer and open a new one immediately
afterwards. This test ran for quite a while without any problems (up to
8M docs). Then the weird thing happened: suddenly the index was totally
wiped out, meaning only the segments.gen and segments_1 were in the
index directory. There was no IOException in the log.

I must say that I didn't write the stress test and haven't seen it yet
either, I'm planning to take a look next week if I can. It looks like it
might be an application bug, such that it might open an IndexWriter with
create=true, because the segments file has the initial generation 1.

I started looking if the constructors of IndexWriter that automatically
try to determine if the index has to be created or not might have a
problem, maybe even in combination with aborted, but still running
background merge threads. I think the question is if
SegmentInfos.getCurrentSegmentGeneration() could ever falsely return -1,
but it doesn't look like this is possible.

I'm just really guessing here, it might not be a Lucene problem at all,
but at this moment I can't rule this possibility out.

-Michael

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

Re: Question about aborting background merges

[Michael McCandless <lu...@mikemccandless.com>]

"Michael Busch" <bu...@gmail.com> wrote:

> I have a question about IndexWriter.close(false) and background
> merges. I was going to take a look at the code, but I'm sure that Mike
> knows the answer :-). Let's assume that a long background merge is
> going on and close(false) is called. Then the merges are marked as
> aborted and IndexWriter.close() returns after flushing
> DocumentsWriter's buffers. The background merge threads keep going.
> Now a new IndexWriter is opened and optimize() is called. Can it
> happen that optimize() tries to create a segment with the same name
> the background threads are still working on? Then the new IndexWriter
> would probably hit an IOException? Or would the new IndexWriter use
> different file names for the merged segments?

We should be fine here.  When a merge kicks off, it gets a segment name
by calling newSegmentName().  That method gives the merge the next
segment name, and marks commitPending, for exactly this reason (actually
there's a comment in that method explaining this).

When you then close(false), the segments_N that's flushed records the
fact that this name is "in use" and will not re-assign that name when
you next open a writer on the index.

This was in fact a bug at one point (failing to mark commitPending on
giving out a new segmentName), which one of the unit tests exposed.

Hmm ... looking at the code, I think we should also check whether a
merge was aborted in mergeInit.  It looks like there is a window from
when a merge is handed out until when it is inited such that if the
writer is closed in that window it could result in incorrect re-use of a
segment name.  Michael are you seeing such a case?

Mike

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