Mailing List Archive: Lucene: Java-Dev

[jira] Commented: (LUCENE-743) IndexReader.reopen()

1 2 3

View All

Index | Next | Previous | View Threaded

jira at apache

Nov 2, 2007, 7:38 AM

Post #51 of 71 (692 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539596 ]

Yonik Seeley commented on LUCENE-743:
-------------------------------------

It also looks like Norm.incRef is used in an unsafe manner (unsynchronized, or synchronized on the reader), and also Norm.decRef() is called inside a synchronized(norms) block, but an individual Norm may be shared across multiple Hashtables, right?

I don't think that norms even needs to be a synchronized Hashtable... it could be changed to a HashMap since it's contents never change, right?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 7:43 AM

Post #52 of 71 (691 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539598 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

OK, reviewed the latest patch:

* In this code:
{code}
// singleNormFile means multiple norms share this file
if (fileName.endsWith("." + IndexFileNames.NORMS_EXTENSION)) {
clone.singleNormStream = d.openInput(fileName, readBufferSize);
}
{code}
I think the comment should be removed (it doens't apply) and also
won't this incorrectly open the singleNormStream more than once if
more than one field does not have separate norms? I think you should
init that to null and then only reopen it, once, if it's still null?

* In MultiSegmentReader, the logic that copies over unchanged norms
from the old norms cache can be simplified. I think you can just
look up the old Norm instance & the new Norm instance and if they
are == then you can copy bytes over? This would also let you remove
"sharedNorms" entirely which is good because it's not a just a
boolean thing anymore since some Norm instances are shared and some
aren't.

* I think you also need to override decRef (and add
decRefReaderNotNorms) to SegmentReader? Because now there is a
mismatch: incRef incr's the Norm RC's, but, decRef does not. So I
think norms are not getting closed? I think we should modify the
"assertReaderClosed()" in the unit test to verify (when appropriate)
that also the RC of all Norm instances is also 0 (ie
assertTrue(SR.normsClosed())). Then, make sure SR calls
referencedSegmentReader.decRefReaderNotNorms instead of decRef. I
think you then don't need to track "closedNorms" boolean, at all.
You simply always decRef the norms whenever SR.decRef is called.
The doCloseUnsharedResources is still needed to close the
singleNormStream.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 7:47 AM

Post #53 of 71 (694 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539600 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

{quote}
This is complex enough that in addition to review, I think we need a
good multi-threaded test - 100 or 1000 threads over a ram directory,
all changing, querying, retrieving docs, reopening, closing, etc.
{quote}

+1

We should fix all the synchronization issues you've found, create this
unit test, and then iterate from there.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 7:57 AM

Post #54 of 71 (691 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539606 ]

Yonik Seeley commented on LUCENE-743:
-------------------------------------

{quote}We should fix all the synchronization issues you've found, create this
unit test, and then iterate from there.
{quote}

Or reverse it... write the test first so we have confidence that it can at least uncover some of these issues.
The test should do as little synchronization as possible of it's own so it doesn't mask a lack of synchronization in the core.
It should be possible to uncover the unsynchronized concurrent use of IndexInput at least, and hopefully some of the refcounting issues too.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 8:08 AM

Post #55 of 71 (691 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539610 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

{quote}
Or reverse it... write the test first so we have confidence that it can at least uncover some of these issues.
The test should do as little synchronization as possible of it's own so it doesn't mask a lack of synchronization in the core.
It should be possible to uncover the unsynchronized concurrent use of IndexInput at least, and hopefully some of the refcounting issues too.
{quote}

Excellent, I agree!

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 9:33 AM

Post #56 of 71 (693 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539634 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

{quote}
I just did a quick partial review of SegmentReader for thread safety only and ran across some potential issues
{quote}

OK, let's scratch my "ready to commit" comment ;)

A question about thread-safety here. I agree that we must
fix all possible problems concerning two or more
IndexReaders in *read-mode*, like the FieldsReader issue.

On the other hand: We're saying that performing write
operations on a re-opened reader results in undefined
behavior. Some of the issues you mentioned, Yonik, should
only apply in case one of the shared readers is used to
perform index modifications, right? Then the question is:
how much sense does it make to make reopen() thread-safe
in the write case then?

So I think the multi-threaded testcase should not
perform index modifications using readers involved in a
reopen()?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 2, 2007, 10:01 AM

Post #57 of 71 (692 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539637 ]

Yonik Seeley commented on LUCENE-743:
-------------------------------------

Sorry, I hadn't kept up with this issue wrt what was going to be legal (and we should definitely only test what will be legal in the MT test). So that removes the deletedDocs issue I guess.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 12, 2007, 1:35 PM

Post #58 of 71 (648 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12541955 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

I think the cause of the intermittant failure in the test is a missing
try/finally in doReopen to properly close/decRef everything on
exception.

Because of lockless commits, a commit could be in-process while you
are re-opening, in which case you could hit an IOexception and you
must therefore decRef those norms you had incRef'd (and, close eg the
newly opened FieldsReader).

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

rengels at ix

Nov 12, 2007, 1:43 PM

Post #59 of 71 (648 views)
Permalink

Re: [jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

Why doesn't reopen get the 'read' lock, since commit has the write
lock, it should wait...

On Nov 12, 2007, at 3:35 PM, Michael McCandless (JIRA) wrote:

>
> [ https://issues.apache.org/jira/browse/LUCENE-743?
> page=com.atlassian.jira.plugin.system.issuetabpanels:comment-
> tabpanel#action_12541955 ]
>
> Michael McCandless commented on LUCENE-743:
> -------------------------------------------
>
> I think the cause of the intermittant failure in the test is a missing
> try/finally in doReopen to properly close/decRef everything on
> exception.
>
> Because of lockless commits, a commit could be in-process while you
> are re-opening, in which case you could hit an IOexception and you
> must therefore decRef those norms you had incRef'd (and, close eg the
> newly opened FieldsReader).
>
>> IndexReader.reopen()
>> --------------------
>>
>> Key: LUCENE-743
>> URL: https://issues.apache.org/jira/browse/LUCENE-743
>> Project: Lucene - Java
>> Issue Type: Improvement
>> Components: Index
>> Reporter: Otis Gospodnetic
>> Assignee: Michael Busch
>> Priority: Minor
>> Fix For: 2.3
>>
>> Attachments: IndexReaderUtils.java, lucene-743-
>> take2.patch, lucene-743-take3.patch, lucene-743-take4.patch,
>> lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-
>> take7.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch,
>> MyMultiReader.java, MySegmentReader.java, varient-no-
>> isCloneSupported.BROKEN.patch
>>
>>
>> This is Robert Engels' implementation of IndexReader.reopen()
>> functionality, as a set of 3 new classes (this was easier for him
>> to implement, but should probably be folded into the core, if this
>> looks good).
>
> --
> 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-unsubscribe [at] lucene
> For additional commands, e-mail: java-dev-help [at] lucene
>

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

yonik at apache

Nov 12, 2007, 1:47 PM

Post #60 of 71 (648 views)
Permalink

Re: [jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

On Nov 12, 2007 4:43 PM, robert engels <rengels [at] ix> wrote:
> Why doesn't reopen get the 'read' lock, since commit has the write
> lock, it should wait...

After lockless commits, there is no read lock!

-Yonik

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 12, 2007, 4:14 PM

Post #61 of 71 (646 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12541998 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

> I think the cause of the intermittant failure in the test is a missing
> try/finally in doReopen to properly close/decRef everything on
> exception.

Awesome! Thanks so much for pointing me there, Mike! I was getting a
little suicidal here already ... ;)

I should have read the comment in SegmentReader#initialize more
carefully:
{code:java}
} finally {

// With lock-less commits, it's entirely possible (and
// fine) to hit a FileNotFound exception above. In
// this case, we want to explicitly close any subset
// of things that were opened so that we don't have to
// wait for a GC to do so.
if (!success) {
doClose();
}
}
{code}

While debugging, it's easy to miss such an exception, because
SegmentInfos.FindSegmentsFile#run() ignores it. But it's good that it
logs such an exception, I just have to remember to print out the
infoStream next time.

So it seems that this was indeed the cause for the failing test case.
I made the change and so far the tests didn't fail anymore (ran it
about 10 times so far). I'll run it another few times on a different
JVM and submit an updated patch in a short while if it doesn't fail
again.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 13, 2007, 11:18 AM

Post #62 of 71 (644 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542201 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

{quote}
Awesome! Thanks so much for pointing me there, Mike! I was getting a
little suicidal here already ...
{quote}

No problem, I lost some hairs tracking that one down too!!

OK, latest patch looks good! I love the new threaded unit test.

Only two smallish comments:

* You should also close fieldsReader when referencedSegmentReader !=
null, right? (in SegmentReader.doClose)

* In the new try/finally in reopenSegment: if you first setup
referencedSegmentReader, then can't that finally clause just be
clone.decRef() instead of duplicating code for decRef'ing norms,
closeNorms(), etc.?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 13, 2007, 11:54 AM

Post #63 of 71 (641 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542212 ]

Yonik Seeley commented on LUCENE-743:
-------------------------------------

So how about a public IndexReader.flush() method so that one could also reopen readers that were used for changes?

Usecase:

reader.deleteDocument()
reader.flush()
writer = new IndexWriter()
writer.addDocument()
writer.close()
reader.reopen()
reader.deleteDocument()

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 13, 2007, 10:02 PM

Post #64 of 71 (646 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542332 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

{quote}
* You should also close fieldsReader when referencedSegmentReader !=
null, right? (in SegmentReader.doClose)
{quote}

Yes, will do!

{quote}
* In the new try/finally in reopenSegment: if you first setup
referencedSegmentReader, then can't that finally clause just be
clone.decRef() instead of duplicating code for decRef'ing norms,
closeNorms(), etc.?
{quote}

Hmm, what if then in clone.close() an exception is thrown from
FieldsReader.close() or singleNormStream.close(). In that case it
would not decRef the referenced reader.

Hmm but actually we could change the order in close() so that
referencedSegmentReader.decRefReaderNotNorms() is done first even
if the following close() operations don't succeed?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 13, 2007, 10:05 PM

Post #65 of 71 (640 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542333 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

{quote}
So how about a public IndexReader.flush() method
{quote}

Since our goal is it to make IndexReader read-only in the future
(LUCENE-1030), do you really think we need to add this?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 14, 2007, 2:11 AM

Post #66 of 71 (640 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542390 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

{quote}
Hmm but actually we could change the order in close() so that
referencedSegmentReader.decRefReaderNotNorms() is done first even
if the following close() operations don't succeed?
{quote}

+1

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 14, 2007, 3:01 AM

Post #67 of 71 (640 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542411 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

{quote}
So how about a public IndexReader.flush() method
{quote}
I think also if we do decide to do this we should open a new issue for it?

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 14, 2007, 5:50 AM

Post #68 of 71 (639 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542462 ]

Yonik Seeley commented on LUCENE-743:
-------------------------------------

> Since our goal is it to make IndexReader read-only in the future
> (LUCENE-1030), do you really think we need to add this?

flush() would make reopen() useful in more cases, and LUCENE-1030 is further off (not Lucene 2.3, right?)
Anyway, flush() would be considered a write operation like setNorm() & deleteDocument() and could be deprecated along with them in the future if that's how we decide to go.

> I think also if we do decide to do this we should open a new issue for it?

Yes, that's fine.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 14, 2007, 7:57 AM

Post #69 of 71 (639 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542492 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

{quote}
I think also if we do decide to do this we should open a new issue for it?
{quote}

+1

I'll open a new issue.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 15, 2007, 6:24 AM

Post #70 of 71 (623 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542786 ]

Michael McCandless commented on LUCENE-743:
-------------------------------------------

Patch looks good. Only thing I found was this leftover
System.out.println, in SegmentReader.java:

{code}
System.out.println("refCount " + getRefCount());
{code}

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743-take9.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

jira at apache

Nov 15, 2007, 7:03 AM

Post #71 of 71 (626 views)
Permalink

[jira] Commented: (LUCENE-743) IndexReader.reopen() [In reply to]

[ https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542790 ]

Michael Busch commented on LUCENE-743:
--------------------------------------

Thanks for the review, Mike! I'll remove the println.

Ok, I think this patch has been reviewed a bunch of times and
should be ready to commit now. I'll wait another day and commit
it then if nobody objects.

> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch, lucene-743-take3.patch, lucene-743-take4.patch, lucene-743-take5.patch, lucene-743-take6.patch, lucene-743-take7.patch, lucene-743-take8.patch, lucene-743-take9.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java, MySegmentReader.java, varient-no-isCloneSupported.BROKEN.patch
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set of 3 new classes (this was easier for him to implement, but should probably be folded into the core, if this looks good).

--
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-unsubscribe [at] lucene
For additional commands, e-mail: java-dev-help [at] lucene

1 2 3

View All

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads