[hibernate-dev] Hibernate Search: Master/Slave DirectoryProviders improvements proposal

[Hardy Ferentschik]

Hi Sanne,


> A) ... but all comunication beetween the client thread calling  
> "getDirectory()" and the CopyDirectory
> which defines the current directory is unprotected; also volatile isn't  
> enough
> to fix it, IMHO an AtomicReference would be more appropriate.

Is this really required? Is synchronisation not ensured via  
SearchFactoryImplementor.getLockableDirectoryProviders()?

> B) ... Also I would like to address the use of finalize() to stop the  
> Timer,
> there is a TODO about it already;

Fair point. Initially a non daemon thread was used. Once we switched to a  
deamon thread we
could probably have got rid of the call to timer.canel().


> C) FileHelper not listening to Thread.interrupt() requests
> As FileHelper.synchronize is used in the mentioned Timers to copy
> potentially big files,
> it would be nice to check for Thread.isInterrupted() and also catch
> ClosedByInterruptException in file-copy operations so to abort all I/O
> operations and
> not just the current one to abort faster on JVM shutdown.
> (It doesn't look like the copy safety was guaranteed anyway).

Sounds reasonable.


> D) Master and Slave share configuration properties.
> Wouldn't it be natural to have two different property keys to manage
> the shared index path? I imagine the typical scenario would be to have  
> the two
> configured on different servers and copy through some NFS, but it looks  
> like
> we are forcing people to use exactly the same path on both machines as  
> the two
> classes read the same props.

The properties are different. Yes, the master and the slave both define:
hibernate.search.default.sourceBase and  
hibernate.search.default.indexBase, but remember
that these are in two different configuration files. Master and slave are  
two seperate machines
or at the very least two different JVMs. Nothing stops you to have  
different mount points. I guess the
docuemtnation might suggest this.

You can definitely create a Jira  issue for B and D. Not sure about A  
though.
Maybe someone else can have another look at the code.

--Hardy