[hibernate-dev] Feedback on Infinispan patch

[Emmanuel Bernard]

Hey Lukasz,
Your patch looks quite good and pass tests on my side.

I encourage others to check out the patch before we apply it (ideally  
another person form HSearch and one person from infinispan.


Lukasz, I have a few questions/remarks though before applying it. Can  
you answer / adjust the patch?

IndexWriterSetting
Why move to return Object in parsing from the initial int?

Move DPHelper#createInfinispanCacheManager to IDP
this is not something that can be shared as it creates a hard  
dependency on infinispan otherwise.

in createInfinispanCacheManager
Don't log in error the fact that xml is not used if a default config  
is used. Just log in trace at best.

Rename InfinispanCacheManagerConfigurationImpl to  
DefaultInfinispanCacheManagerConfiguration or even better with a name  
describing nicely the behavior of the infinispan config.

in InfinispanIndexOutput, is it possible to get writeBytes bigger than  
buffer size? If yes, does newCheck creates the appropriate numbers of  
chunks?

InfinispanDirectoryProvider
put the configuration proeprties available in the  
InfinispanDirectoryProvider javadoc.

I think the default cache name should be "Hibernate Search" instead of  
"HSInfinispanCache". We know it's in infinispan :)

what's the try catch opening and closing an IW about? It looks weird.

in stop()
you don't close the CacheManager? How is that?

InfinispanCacheManagerConfigurationImpl
What does "Infinispan-Cluster" correspond to? Why this name? Shouldn't  
it be "Hibernate Search cluster"?
Is it safe to override the GlobalConfiguration? What if JBoss AS use  
infinispan to run?

Why the use of DummyTransactionManagerLookup. Doesn't Infinispan guess  
the right TM depending on the environment? e in JBoss As use the JBoss  
one etc?  I think GenericTransactionManagerLookup does that.

InfinispanCacheManagerConfiguration
some javadoc on the methods would be useful. I don't know what do  
implement here.


Is there a better name for Metadata? Like FileMetadata maybe?

Where is ispn-cache-default-conf.xml used? For tests only? If not: is  
it possible to use a programmatic version instead and what is "It's a  
movie cache"?

Emmanuel



Begin forwarded message:

> From: Łukasz Moreń <lukasz.moren at gmail.com>
> Date: 21 août 2009 02:11:03 HAEC
> To: Emmanuel Bernard <emmanuel at hibernate.org>
> Subject: GSoC patch with Infinispan Directory Provider
>
> I'm sending patch and piece of documentation - not much but  
> necessary information are included.
> There are some todos but I didn't manage to finish it yet.
> I changed maven jgroups dependency to 2.8.beta2, before version was  
> clashed with used by infinispan.
> In pom file there was dependency on hibernate common annotations  
> 3.2.shapshot. It should't be 3.5?
>
> Cheers,
> Lukasz

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/hibernate-dev/attachments/20090823/2a0a830b/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: GSoC2009_summary.docx
Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document
Size: 22661 bytes
Desc: not available
Url : http://lists.jboss.org/pipermail/hibernate-dev/attachments/20090823/2a0a830b/attachment.bin 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/hibernate-dev/attachments/20090823/2a0a830b/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: InfinispanDirectoryProvider_20_08_2009.patch
Type: application/octet-stream
Size: 107611 bytes
Desc: not available
Url : http://lists.jboss.org/pipermail/hibernate-dev/attachments/20090823/2a0a830b/attachment.obj 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/hibernate-dev/attachments/20090823/2a0a830b/attachment-0002.html