[hibernate-dev] Hibernate Search 421 [Exceptions happening in backend are unnoticed]

[Sanne Grinovero]

Hi Amin,
I've been looking a bit in this, but didn't take any action as we
didn't discuss any strategy, so glad you join in propose something.
I didn't think of jms, it's ok we provide some way for the end user to
override whatever we provide, but I think we should just provide
the basic stuff: logging the error or propagate it back when possible.
And then add some extensibility like plugin loading the usual way.

Originally (Hibernate Search 3.0.x) the changes made to different
indexes were serialized, so when using the "sync" backend the same
process would have applied changes to the index and, in case of
exceptions, this would have been propagated up to user's code making
it impossible to go unnoticed, or at least moving responsibility of
handling it to the developer.
When using the "async" mode for backend a different thread would have
handled the indexwriter, so in case of an exception the error would
have killed the separate thread, go unnoticed, and as we use a
ThreadPool a new thread would have been spawned to replace the failed
one.

Since Search 3.1.x both "sync" and "async" use a separate thread - to
be able to apply changes to different indexes in parallel - so
exceptions are going unnoticed by user code even in "sync" mode. Look
into "run" method of
org.hibernate.search.backend.impl.lucene.LuceneBackendQueueProcessor,
it creates a org.hibernate.search.backend.impl.lucene.QueueProcessors
where most of this logic resides.
This impl will use either runAllAsync() or runAllWaiting() depending
on backend configuration (async/sync). The Async version just
schedules the different tasks, the sync one is going to do the same
but will wait for all of them to have finished before returning
control, making sure to implement the sync behaviour even while using
several threads to perform it. I was inspired by
java.util.concurrent.AbstractExecutorService.invokeAll(Collection<Callable<T>>
tasks), as mentioned in code comments, which is ignoring
ExecutionException, so this empty catch block should be used now to do
something.

I'd suggest to provide this defaults:

* for sync backend:
   by default: rethrow the exception (like old behaviour)
   configurable alternative: log it

* for async backend:
   log it (it's not useful to rethrow as nobody is listening on it)

To handle these cases only you would add the log statement, an rethrow
it, fixing the QueueProcessors code.
To give full control to the end user of what to do, I'd suggest to let
him specify an implementationf of
java.lang.Thread.UncaughtExceptionHandler
and so we can set that on the thread.
The backend is using the default JVM "Executors.newFixedThreadPool( 1
);"  (initialized at
org.hibernate.search.backend.impl.lucene.PerDPResources:51)
but we could change that to use the Search Executor factory
"org.hibernate.search.batchindexing.Executors" I later added for the
batchbackend.
The nice thingy for this Executor is that it customizes the thread
names so you can easily spot Search's threads in monitoring/debugging
tools;
we could have a setUncaughtExceptionHandler( userImplementation ) in
org.hibernate.search.batchindexing.Executors:85.
If you change it there, the same handler would be used to manage
errors in the batch indexer, so you solve both problems at once.
Our "log the error" implementation would be the nice default for an
exception handler, but still I'd like to make sure the user code will
get the error propagated when
using async mode.

Cheers,
Sanne


2010/1/9 Amin Mohammed-Coleman <aminmc at gmail.com>:
> Hi All
>
> Emmanuel asked me to look at this issue (HSearch 421) where exceptions
> happening in backend process which are going unnoticed.  I was wondering if
> I could get some advice/thoughts on how to tackle the problem.    The issue
> mentioned providing the user the option to decide how to handle exceptions
> (for example queues, logs, etc), so I'm guessing there needs to be some
> custom option that the user will need to set up, maybe something like this?
>
> exception_handling_strategy=jms
> exception_handling_strategy_jms_queue=
>
> or if they wanted to log the exceptions:
>
> exception_handling_strategy=log
>
> or the user could create a custom class which implements a particular
> interface to handle exceptions
>
> exception_handling_strategy=custom
> exception_handling_custom_class=ExceptionHandling
>
> I could be completely wrong in the above approach and therefore would be
> grateful for any input.
>
>
> Cheers
> Amin
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>