<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi there - all looks good. Some comments:<div><br></div><div><ul class="MailOutline"><li>Summary documentation - is this going to be published on a wiki page or something somewhere? Especially the Infinispan bit? I think people will find this info very useful... </li><li>CacheKey - if this class is what everything is going to be used in the cache, for performance you should cache the hashcode. Calculate it once and then cache it as an instance variable. If this class is immutable it can be done on construction, even. Infinispan uses hashcode() a lot. :-) But then again, depending on how many entries live in the cache, the overhead of an extra int for every entry may be heavy ...</li><li>LockCacheKey - is probably more performant if this is implemented as a boolean flag on CacheKey. Then you won't need to look at the class type when working out hashcodes</li><li>Have you written any stress or performance tests?</li></ul><div><br></div><div>Cheers</div><div>Manik</div><div><br></div><div><div>On 23 Aug 2009, at 22:53, Łukasz Moreń wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi,<div><br></div><div>Yes, I can adjust the patch next days. I've just noticed that I send summary in not friendly format :), better one is now attached.</div><div><br></div><div>There is explanation for yours questions below.</div> <div> <br><br><div class="gmail_quote">2009/8/23 Emmanuel Bernard <span dir="ltr"><<a href="mailto:emmanuel@hibernate.org" target="_blank">emmanuel@hibernate.org</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div style="word-wrap:break-word">Hey Lukasz,<div>Your patch looks quite good and pass tests on my side.</div><div><br></div><div>I encourage others to check out the patch before we apply it (ideally another person form HSearch and one person from infinispan.</div> <div><br></div><div><br></div><div>Lukasz, I have a few questions/remarks though before applying it. Can you answer / adjust the patch?</div><div><br></div><div>IndexWriterSetting</div><div><div>Why move to return Object in parsing from the initial int?</div> <div></div></div></div></blockquote><div><br></div><div>IndexWriterSetting has to set up MergeScheduler in IndexWriter. Before, parsing was responsible for number conversion from String to int. Now I have to parse class name, and build/return MergeScheduler from it.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br></div><div>Move DPHelper#createInfinispanCacheManager to IDP</div> <div>this is not something that can be shared as it creates a hard dependency on infinispan otherwise.</div><div><br></div><div>in createInfinispanCacheManager</div> <div>Don't log in error the fact that xml is not used if a default config is used. Just log in trace at best.</div><div><br></div><div>Rename InfinispanCacheManagerConfigurationImpl to DefaultInfinispanCacheManagerConfiguration or even better with a name describing nicely the behavior of the infinispan config.</div> <div><br></div><div>in InfinispanIndexOutput, is it possible to get writeBytes bigger than buffer size? If yes, does newCheck creates the appropriate numbers of chunks?</div></div></div></blockquote><div><br></div><div>Yes it is possible. Writing process is divided into stages, during every stage can be written max: buffer_size bytes. At the end of the stage its checked if necessary is new chunk, if so new chunk is created.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div><div><br></div><div>InfinispanDirectoryProvider</div> <div>put the configuration proeprties available in the InfinispanDirectoryProvider javadoc.</div><div><br></div><div>I think the default cache name should be "Hibernate Search" instead of "HSInfinispanCache". We know it's in infinispan :)</div> <div><br></div><div>what's the try catch opening and closing an IW about? It looks weird.</div></div></div></blockquote><div><br></div><div>IW is opened with create=true parameter, first index have to be initialized/created. Always next IW is opened with create=false parameter, then data is appended to exisitng index. Similar things are done in other DP's.</div> <div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>in stop()</div><div>you don't close the CacheManager? How is that?</div> </div></div></blockquote><div><br></div><div>Yes. Should be closed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"> <div><div></div><div><br></div><div>InfinispanCacheManagerConfigurationImpl</div> <div>What does "Infinispan-Cluster" correspond to? Why this name? Shouldn't it be "Hibernate Search cluster"?</div><div>Is it safe to override the GlobalConfiguration? What if JBoss AS use infinispan to run?</div> </div></div></blockquote><div><br></div><div>This name is used to distinguish cluster used by HSearch - All nodes with the same name form a group. Yes, rather "Hibernate Search cluster" is better name. It is safe to modify GlobalConfiguration, there can be set up configuration for CacheManager like communication way (JGroups or something else), stack configuration for JGroups, etc.; where Configuration is used to configure specific cache. I think just the infinispan cluster name on JBoss AS have to be different from HSearch, then they will be independent. </div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div> <div><br></div><div><div>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.</div> </div></div></div></blockquote><div><br></div><div>Yes right, I was testing it with DummyTM and forgot to change it later.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div style="word-wrap:break-word"><div><div><div></div> <div><br></div></div><div>InfinispanCacheManagerConfiguration</div><div>some javadoc on the methods would be useful. I don't know what do implement here.</div><div><br></div><div><br></div><div>Is there a better name for Metadata? Like FileMetadata maybe?</div> </div></div></blockquote><div><br></div><div>Better FileMetadata or maybe FileHeader.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"> <div><div></div> <div><br></div><div>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"?</div></div></div></blockquote><div> <br></div><div>Yes in tests only so far. However it can be used as a provided default configuration. I will send maybe question to infinispan group about best configuration parameters. "It's a movie cache" it's the name for cache configured in ispn-cache-default-conf.xml. In tests in this cache are stored indexes for entity Movie. Indexes for all other entities are stored in default HSInfinispanCache.</div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div><div><br></div><div>Emmanuel</div> <div><br></div></div><div><br><div><br><div>Begin forwarded message:</div><br><blockquote type="cite"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font face="Helvetica" size="3" color="#000000" style="font:12.0px Helvetica;color:#000000"><b>From: </b></font><font face="Helvetica" size="3" style="font:12.0px Helvetica">Łukasz Moreń <<a href="mailto:lukasz.moren@gmail.com" target="_blank">lukasz.moren@gmail.com</a>></font></div> <div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font face="Helvetica" size="3" color="#000000" style="font:12.0px Helvetica;color:#000000"><b>Date: </b></font><font face="Helvetica" size="3" style="font:12.0px Helvetica">21 août 2009 02:11:03 HAEC</font></div> <div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font face="Helvetica" size="3" color="#000000" style="font:12.0px Helvetica;color:#000000"><b>To: </b></font><font face="Helvetica" size="3" style="font:12.0px Helvetica">Emmanuel Bernard <<a href="mailto:emmanuel@hibernate.org" target="_blank">emmanuel@hibernate.org</a>></font></div> <div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><font face="Helvetica" size="3" color="#000000" style="font:12.0px Helvetica;color:#000000"><b>Subject: </b></font><font face="Helvetica" size="3" style="font:12.0px Helvetica"><b>GSoC patch with Infinispan Directory Provider</b></font></div> <div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px;min-height:14px"><br></div> </div><div>I'm sending patch and piece of documentation - not much but necessary information are included. </div> <div>There are some todos but I didn't manage to finish it yet.</div><div>I changed maven jgroups dependency to 2.8.beta2, before version was clashed with used by infinispan.</div> <div>In pom file there was dependency on hibernate common annotations 3.2.shapshot. It should't be 3.5?</div> <div><br></div>Cheers,<div>Lukasz</div> </blockquote></div></div></div><br><div style="word-wrap:break-word"><div><div><blockquote type="cite"></blockquote></div></div></div><br><div style="word-wrap:break-word"><div><div> <blockquote type="cite"></blockquote></div><br></div></div><br></blockquote></div><br></div> <span><GSoC2009_summary.pdf></span>_______________________________________________<br>infinispan-dev mailing list<br><a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>https://lists.jboss.org/mailman/listinfo/infinispan-dev</blockquote></div><br><div apple-content-edited="true"> <span class="Apple-style-span" style="font-size: 12px; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>--</div><div>Manik Surtani</div><div><a href="mailto:manik@jboss.org">manik@jboss.org</a></div><div>Lead, Infinispan</div><div>Lead, JBoss Cache</div><div><a href="http://www.infinispan.org">http://www.infinispan.org</a></div><div><a href="http://www.jbosscache.org">http://www.jbosscache.org</a></div><div><br></div></div></span><br class="Apple-interchange-newline"></div></span><br class="Apple-interchange-newline"> </div><br></div></body></html>