lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bernhard Messer <Bernhard.Mes...@intrafind.de>
Subject Re: DO NOT REPLY [Bug 30736] - [PATCH] to remove synchronized code from TermVectorsReader
Date Thu, 19 Aug 2004 21:44:25 GMT
Otis,

sorry to say, the implementation i with provided with that patch would 
work, but looking in detail on it, it's simply bullshit.

It's not just the IOException which is caught and not passed to the 
caller. The current implementation would open InputStreams for each 
thread which never get closed (thanks Christoph for the tip). Every 
thread calling the ThreadLocal.get() method is creating a new 
TermVectorsReader within the anonymous inner class. The opened 
InputStreams in TermVectorsReader will never get closed again (correct 
me please if I'm wrong). So the only way i see, is to make the 
TermVectorReader class cloneable and put a clone of the original into 
the ThreadLocal.

The implementation i have in mind, would look very similar to the one 
Doug introduced in TermInfosReader, handling the SegmentTermEnum objects.

So please, simply forget that bad shot. I'll gonna try to correct it and 
add the new files to the current patch in Bugzilla.

thx
Bernhard

Otis Gospodnetic wrote:

>Ah, I see English.java.  I didn't check test/ directories.
>
>IOException - yes, let the caller deal with it.
>
>Please just attach new diffs to existing Bugzilla entry.  I'll ignore
>the old ones.
>
>Thanks,
>Otis
>
>
>--- Bernhard Messer <Bernhard.Messer@intrafind.de> wrote:
>
>  
>
>>Otis,
>>
>>the English class is in cvs, that's where i found it. It is also used
>>by 
>>other test classes like TestTermVectors e.g.
>>
>>The IOException was something where i wasn't sure how to process. I 
>>think you're right, the best idea would be to pop it up to the
>>caller. 
>>Looking at the original code, the IOException wasn't caught in 
>>TermVectors constructor.
>>
>>Sorry about the tabs, this are my settings in exlipse, inserting tabs
>>
>>instead of blanks.
>>
>>Shall i create a new patch send it to the list, or do i have to
>>create a 
>>new bugzilla issue for that. Is it possible to update attachments in 
>>bugzilla ? Don't think so.
>>
>>regards
>>Bernhard
>>
>>bugzilla@apache.org wrote:
>>
>>    
>>
>>>DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
>>>RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
>>><http://issues.apache.org/bugzilla/show_bug.cgi?id=30736>.
>>>ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
>>>INSERTED IN THE BUG DATABASE.
>>>
>>>http://issues.apache.org/bugzilla/show_bug.cgi?id=30736
>>>
>>>[PATCH] to remove synchronized code from TermVectorsReader
>>>
>>>
>>>
>>>
>>>
>>>------- Additional Comments From otis@apache.org  2004-08-19 11:47
>>>      
>>>
>>-------
>>    
>>
>>>Bernhard,
>>>
>>>Thanks for the patch.  The unit test requires class
>>>      
>>>
>>o.a.lucene.util.English. 
>>    
>>
>>>This is not in CVS.  Is this something that should be in the CVS? 
>>>      
>>>
>>What is it?
>>    
>>
>>>I am also wondering about this piece of code:
>>>
>>>-      termVectorsReader = new TermVectorsReader(cfsDir, segment,
>>>      
>>>
>>fieldInfos);
>>    
>>
>>>+       final Directory dir = cfsDir;
>>>+       termVectorsLocal = new ThreadLocal() {
>>>+               protected synchronized Object initialValue() {
>>>+                       try {
>>>+                               return new TermVectorsReader(dir,
>>>      
>>>
>>segment,
>>    
>>
>>>fieldInfos);
>>>+                       } catch (IOException ioe) {
>>>+                               ioe.printStackTrace();
>>>+                               return null;
>>>+                       }
>>>+               }
>>>+       };
>>>
>>>Is is a good thing to 'eat' that IOException and quietly return
>>>      
>>>
>>null?  The
>>    
>>
>>>method where this code is, is already throwing IOException, so why
>>>      
>>>
>>not let the
>>    
>>
>>>IOException pop up?
>>>
>>>Finally, it looks like diffs contain tabs.  Could you please change
>>>      
>>>
>>tabs to 2
>>    
>>
>>>spaces?
>>>
>>>Thanks,
>>>Otis
>>>
>>>      
>>>
>>---------------------------------------------------------------------
>>    
>>
>>>To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
>>>For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>>>
>>> 
>>>
>>>      
>>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
>>For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>>
>>
>>    
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>
>  
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message