Hi Michael,<div><br></div><div>another aspect I noticed are the public methods to create an IndexWriter.</div><div>Can you verify which parts in Yarep/Yanel really need that?</div><div>This is very dangerous because you have no control anymore by when the writer gets closed. I would keep all modifying index logic inside the Indexer class.</div>
<div><br></div><div>What do you think?<br><br>Cheers</div><div>Balz<br><br><div class="gmail_quote">On Wed, Mar 23, 2011 at 10:59 AM, Balz Schreier <span dir="ltr"><<a href="mailto:balz.schreier@gmail.com">balz.schreier@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Hi Michael,<div><br></div><div>I had to use the removeFromIndex(node) method from the LuceneIndexer class.</div><div>I also had a quick look at the LuceneIndexerV2.</div>
<div><br></div><div>Don't you think that the following is crucial to have it in place? :</div>
<div><br></div><div>1) When an IndexWriter gets created it must get closed under any circumstances.</div><div>I would recommend to use the TCFTC pattern here (try catch finally try catch):</div><div>try : create, do stuff, close</div>
<div>catch: throwable (not exception)</div><div>finally: try to close, catch throwable (do nothing in there), </div><div><br></div><div>e.g.:</div><div><br></div><div><font face="'courier new', monospace"><b>try</b> {</font></div>
<div><font face="'courier new', monospace"> create</font></div><div><font face="'courier new', monospace"> do stuff</font></div><div><font face="'courier new', monospace"><b> <font color="#FF0000">close</font></b></font></div>
<div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace">} <b>catch</b> (Throwable t) {</font></div><div><font face="'courier new', monospace"> logging</font></div>
<div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace">} <b>finally</b> {</font></div><div><font face="'courier new', monospace"> <b>try</b> {</font></div>
<div><font face="'courier new', monospace"><b> <font color="#FF0000">close</font></b></font></div><div><font face="'courier new', monospace"><br>
</font></div><div><font face="'courier new', monospace"> } <b>catch</b> (Throwable t) {</font></div><div><font face="'courier new', monospace"> }</font></div>
<div><font face="'courier new', monospace">}</font></div><div><br></div><div><br></div><div>This way, you are absolutely sure that it gets closed whatever happens.</div><div>I found no try catch in updateDocument(). Maybe I'm wrong, I just quickly had a look at it.</div>
<div><br></div><div>2)</div><div><br></div><div>If you have heavy traffic on your website, you want to avoid that your instance of indexwriter can not be created (because another writer has acquired the lock already).</div>
<div><br></div><div>Therefore I would introduce a central LOCK in the LuceneIndexer class:</div><div><br></div><div><font face="'courier new', monospace"><b>private static final String LOCK = "lock";</b></font></div>
<div><br></div><div>Then I would update the code from 1) like this:</div><div><br></div><div><div><font face="'courier new', monospace">try {</font></div><div><font face="'courier new', monospace"><b> synchronized (LOCK) {</b></font></div>
<div><font face="'courier new', monospace"> create</font></div><div><font face="'courier new', monospace"> do stuff</font></div>
<div><font face="'courier new', monospace"> close</font></div><div><font face="'courier new', monospace"><b> }</b></font></div><div><font face="'courier new', monospace"><br>
</font></div><div><font face="'courier new', monospace">} catch (Throwable t) {</font></div><div><font face="'courier new', monospace"> logging</font></div>
<div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace">} finally {</font></div><div><font face="'courier new', monospace"> try {</font></div>
<div><font face="'courier new', monospace"> close</font></div><div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace"> } catch (Throwable t) {</font></div>
<div><font face="'courier new', monospace"> }</font></div><div><font face="'courier new', monospace">}</font></div></div><div><br></div><div>What do you think?</div>
<div>This way, you are absolutely sure (as long as only one JVM is accessing the index, so no Yanel cluster is running), that IndexWriter objects can be created all the time, no exceptions!</div><div><br></div><div>You can also synchronize the whole method, but I think that it is much more performant if you only synchronize the code sections where you really do index modifications.</div>
<div><br></div><div>Cheers</div><div>Balz</div><font color="#888888"><div><br></div>
</font></blockquote></div><br></div>