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(a)gmail.com&gt;: > 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >

Hi Sanne Thanks for the advice! I'll take a look at it. Cheers Amin On Sat, Jan 9, 2010 at 1:34 PM, Sanne Grinovero <sanne.grinovero(a)gmail.com&gt;wrote: > 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(a)gmail.com&gt;: >> 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(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev