[ 
https://issues.apache.org/jira/browse/LUCENE-1609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12701986#action_12701986
 ] 

Earwin Burrfoot edited comment on LUCENE-1609 at 4/23/09 9:41 AM:
------------------------------------------------------------------

The problem is not with indexState not being volatile. You can unsafely publish 
objects that have no internal state, or their state is consistent enough for 
you under any memory visibility/reordering effects. See working example of it 
in LUCENE-1607, Yonik's hash for interning strings.

The problem is that indexState guards indexTerms, indexInfos, indexPointers, 
which aren't volatile too and aren't guarded by the lock. It is possible that 
one thread does load these fields and then sets indexState = new IndexRead(), 
but another thread sees only the last write and dies with NPE.

The thing I don't get, is why do we want lazy loading here at all? Is there any 
usage for TermInfosReader that prevents it from initializing in constructor?

      was (Author: earwin):
    The problem is not with indexState not being volatile. You can unsafely 
publish objects that have no internal state, or their state is consistent 
enough for you under any memory visibility/reordering effects. See working 
example of it in LUCENE-1607, Yonik's hash for interning strings.

The problem is that indexState guards indexTerms, indexInfos, indexPointers, 
which aren't volatile too and aren't guarded by the lock. It is possible that 
one thread does load these fields and then sets indexState = new IndexRead(), 
but another thread sees only the last write and dies with NPE.
  
> Eliminate synchronization contention on initial index reading in 
> TermInfosReader ensureIndexIsRead 
> ---------------------------------------------------------------------------------------------------
>
>                 Key: LUCENE-1609
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1609
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.9
>         Environment: Solr 
> Tomcat 5.5
> Ubuntu 2.6.20-17-generic
> Intel(R) Pentium(R) 4 CPU 2.80GHz, 2Gb RAM
>            Reporter: Dan Rosher
>         Attachments: LUCENE-1609.patch
>
>
> synchronized method ensureIndexIsRead in TermInfosReader causes contention 
> under heavy load
> Simple to reproduce: e.g. Under Solr, with all caches turned off, do a simple 
> range search e.g. id:[0 TO 999999] on even a small index (in my case 28K 
> docs) and under a load/stress test application, and later, examining the 
> Thread dump (kill -3) , many threads are blocked on 'waiting for monitor 
> entry' to this method.
> Rather than using Double-Checked Locking which is known to have issues, this 
> implementation uses a state pattern, where only one thread can move the 
> object from IndexNotRead state to IndexRead, and in doing so alters the 
> objects behavior, i.e. once the index is loaded, the index nolonger needs a 
> synchronized method. 
> In my particular test, this uncreased throughput at least 30 times.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-dev-h...@lucene.apache.org

Reply via email to