lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Re: ConcurrentMergeScheduler may spawn MergeThreads forever
Date Tue, 28 Apr 2009 20:00:39 GMT
I hope that I don't make a complete fool of myself, but I'm talking about
this:

  private List exceptions = new ArrayList();

and this (MergeThread.run()):

          synchronized(ConcurrentMergeScheduler.this) {
            exceptions.add(exc);
          }

Nothing seems to read this exceptions list, anywhere. That's what confused
me in the first place - it looks as if at some point saving those exceptions
was for a reason, but not anymore?

I see that you already fixed CMS to sleep for 250 ms (I'd add few lines that
explain why we do it) - thanks !

I wonder if we should remove this exceptions list? It's only accessed if an
exception is thrown, and therefore does not have any impact on performance
or anything (even though it syncs on CMS), but it's just confusing.

Shai

On Tue, Apr 28, 2009 at 4:59 PM, Michael McCandless <
lucene@mikemccandless.com> wrote:

> On Tue, Apr 28, 2009 at 9:27 AM, Shai Erera <serera@gmail.com> wrote:
> >> It's there so "anyUnhandledExceptions" can be called;
> >
> > I will check the code again, but I remember that after commenting it, the
> > only compile errors I saw were from MergeThread adding the exception ...
> > perhaps I'm missing something, so I'll re-check the code.
>
> The oal.util.LuceneTestCase wraps each test so that if any unhandled
> CMS exceptions happened during a test, the test fails.
>
> > I understand your point now - merging is an internal process to IW,
> > therefore there's no real user to notify on errors (e.g., even if IW knew
> > there is an error, what would it do exactly?), and I guess keep trying to
> > execute the merges is reasonable (while throwing the exceptions further -
> in
> > hope that the user code will catch those and do something with them).
>
> OK.
>
> > BTW, what will happen if I encounter such exceptions, that are thrown
> > repeatedly, and shutdown the JVM? I guess the index will not be in a
> corrupt
> > state, right? The next time I'll open it, it should be in a state prior
> to
> > the merge, or at least prior to the merge that failed?
>
> The index will never be corrupt (except for bugs ;) ) -- it will
> contain whatever the last commit was (which is typically well before
> the merges started).  EG w/ autoCommit=false, the index as of when you
> last called commit (or as of when you opened it, if you have not
> called commit) will remain intact on an abnormal shutdown.
>
> > I think that sleeping in case of exceptions makes sense .. in case of IO
> > errors that are temporary, this will not spawn threads endlessly, and
> > sleeping will give an opportunity for the IO problem to resolve. In case
> of
> > bugs, which are supposed to be detected during test time, it should give
> the
> > developer a chance to kill the process relatively quickly.
>
> OK I'll commit this.
>
> Mike
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>

Mime
View raw message