[jira] Created: (LUCENE-1726) IndexWriter.readerPool create new segmentReader outside of sync block

IndexWriter.readerPool create new segmentReader outside of sync block
---------------------------------------------------------------------

                 Key: LUCENE-1726
                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
             Project: Lucene - Java
          Issue Type: Improvement
          Components: Index
    Affects Versions: 2.4.1
            Reporter: Jason Rutherglen
            Priority: Trivial
             Fix For: 2.9


I think we will want to do something like what field cache does
with CreationPlaceholder for IndexWriter.readerPool. Otherwise
we have the (I think somewhat problematic) issue of all other
readerPool.get* methods waiting for an SR to warm.

It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Rutherglen updated LUCENE-1726:
-------------------------------------

    Attachment: LUCENE-1726.patch

We don't block accessing readers in the IW.readerPool when a new
segmentReader is being warmed/instantiated. This is important
when new segmentReaders on large new segments are being accessed
for the first time. Otherwise today IW.getReader may wait while
the new SR is being created.

* IW.readerPool map values are now of type MapValue

* We synchronize on the MapValue in methods that access the SR

* synchronize for the entire readerPool.get method is removed.

* All tests pass

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Priority: Trivial
>             Fix For: 2.9
>
>         Attachments: LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless reassigned LUCENE-1726:
------------------------------------------

    Assignee: Michael McCandless

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 2.9
>
>         Attachments: LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

Can we make the MapValue strongly typed?  Eg name it SegmentReaderValue, and it has a single member "SegmentReader reader".

getIfExists has duplicate checks for null (mv != null is checked twice and mv.value != null too).

I think there is a thread hazard here, in particular a risk that one thread decrefs a reader just as another thread is trying to get it, and the reader in fact gets closed while the other thread has an mv.reader != null and illegally increfs that.  I think you'll have to do the sr.incRef inside the synchronized(this), but I don't think that entirely resolves it.

I'm going to move this out out 2.9; I don't think it should block it.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless updated LUCENE-1726:
---------------------------------------

    Fix Version/s:     (was: 2.9)
                   3.1

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Rutherglen updated LUCENE-1726:
-------------------------------------

    Attachment: LUCENE-1726.patch

* New SRMapValue is strongly typed

* All tests pass

{quote}I think there is a thread hazard here, in particular a
risk that one thread decrefs a reader just as another thread is
trying to get it, and the reader in fact gets closed while the
other thread has an mv.reader != null and illegally increfs
that. I think you'll have to do the sr.incRef inside the
synchronized(this), but I don't think that entirely resolves
it.{quote}

Are you referring to a decref on a reader outside of IW? The
asserts we have did help in catching synchronization errors.
It's unclear to me how to recreate the use case described such
that it breaks things. We need a test case that fails with the
current patch?

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

The hazard is something like this, for a non-NRT IndexWriter (ie, no
pooling):

  * Thread #1 is a merge; it checks out a reader & starts running

  * Thread #2 is applyDeletes (or opening an NRT reader); it calls
    readerPool.get, enters the first sync block to pull out the
    SRMapValue which has non-null reader, then leaves the sync block

  * Thread #1 calls release, which decRefs the reader & closes it

  * Thread #2 resumes, sees it has a non-null mv.reader and incRefs
    it, which is illegal (reader was already closed).

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Jason Rutherglen commented on LUCENE-1726:
------------------------------------------

Shouldn't we be seeing an exception in TestStressIndexing2 (or
another test class) when the mv.reader.incRef occurs and the
reader is already closed?

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

Yes, we should eventually see a failure; I think it's just rare.  Maybe try making a new test that constantly indexes docs, w/ low merge factor & maxBufferedDocs (so lots of flushing/merging happens) in one thread and constantly opens an NRT reader in another thread, to tease it out?

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Jason Rutherglen commented on LUCENE-1726:
------------------------------------------

When I moved the sync block around in readerPool.get, tests
would fail and/or hang. I'm not sure yet where we'd add the
sync(this) block.

I'll work on reproducing the above mentioned issue, thanks for
the advice.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Rutherglen updated LUCENE-1726:
-------------------------------------

    Attachment: LUCENE-1726.patch

Added a test case (for now a separate test class) that runs for
5 minutes, mergeFactor 2, maxBufferedDocs 10, 4 threads
alternately adding and deleting docs. I haven't seen the error
we're looking for yet. CPU isn't maxing out (probably should,
indicating possible blocking?) and may need to allow it run
longer?

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Rutherglen updated LUCENE-1726:
-------------------------------------

    Attachment: LUCENE-1726.patch

Each thread in the test only performs adds or deletes (rather than both) and now we get a "MockRAMDirectory: cannot close: there are still open files" exception.  

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Rutherglen updated LUCENE-1726:
-------------------------------------

    Attachment: LUCENE-1726.trunk.test.patch

I tried the test on trunk and get the same error. They're all
docstore related files so maybe extra doc stores are being
opened?

{code}
    [junit] MockRAMDirectory: cannot close: there are still open files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2, _g2.tvf=2, _dw.fdx=2, _g2.tvd=2, _g2.tvx=2, _ks.tvf=2, _n9.tvx=2, _ks.tvx=2, _n9.fdx=2, _ks.fdx=2, _dw.cfx=1, _n9.tvf=2, _cp.cfx=1, _s4.tvf=2, _dw.tvx=2, _87.fdx=2, _fr.tvx=2, _87.tvf=2, _fr.tvd=2, _87.fdt=2, _ks.tvd=2, _s4.tvd=2, _dw.tvd=2, _n9.fdt=2, _g2.fdt=2, _87.tvd=2, _fr.fdt=2, _dw.fdt=2, _dj.cfx=1, _s4.tvx=2, _ks.fdt=2, _n9.tvd=2, _fr.tvf=2, _fr.fdx=2, _dw.tvf=2, _87.tvx=2}
    [junit] java.lang.RuntimeException: MockRAMDirectory: cannot close: there are still open files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2, _g2.tvf=2, _dw.fdx=2, _g2.tvd=2, _g2.tvx=2, _ks.tvf=2, _n9.tvx=2, _ks.tvx=2, _n9.fdx=2, _ks.fdx=2, _dw.cfx=1, _n9.tvf=2, _cp.cfx=1, _s4.tvf=2, _dw.tvx=2, _87.fdx=2, _fr.tvx=2, _87.tvf=2, _fr.tvd=2, _87.fdt=2, _ks.tvd=2, _s4.tvd=2, _dw.tvd=2, _n9.fdt=2, _g2.fdt=2, _87.tvd=2, _fr.fdt=2, _dw.fdt=2, _dj.cfx=1, _s4.tvx=2, _ks.fdt=2, _n9.tvd=2, _fr.tvf=2, _fr.fdx=2, _dw.tvf=2, _87.tvx=2}
    [junit] at org.apache.lucene.store.MockRAMDirectory.close(MockRAMDirectory.java:278)
    [junit] at org.apache.lucene.index.Test1726.testIndexing(Test1726.java:48)
    [junit] at org.apache.lucene.util.LuceneTestCase.runTest(LuceneTestCase.java:88)
{code}

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Jason Rutherglen edited comment on LUCENE-1726 at 7/8/09 9:47 AM:
------------------------------------------------------------------

I tried the test on trunk and get the same error. They're all
docstore related files so maybe extra doc stores are being
opened?

{code}
   [junit] MockRAMDirectory: cannot close: there are still open
files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2, _g2.tvf=2, _dw.fdx=2,
_g2.tvd=2, _g2.tvx=2, _ks.tvf=2, _n9.tvx=2, _ks.tvx=2,
_n9.fdx=2, _ks.fdx=2, _dw.cfx=1, _n9.tvf=2, _cp.cfx=1,
_s4.tvf=2, _dw.tvx=2, _87.fdx=2, _fr.tvx=2, _87.tvf=2,
_fr.tvd=2, _87.fdt=2, _ks.tvd=2, _s4.tvd=2, _dw.tvd=2,
_n9.fdt=2, _g2.fdt=2, _87.tvd=2, _fr.fdt=2, _dw.fdt=2,
_dj.cfx=1, _s4.tvx=2, _ks.fdt=2, _n9.tvd=2, _fr.tvf=2,
_fr.fdx=2, _dw.tvf=2, _87.tvx=2} [junit]
java.lang.RuntimeException: MockRAMDirectory: cannot close:
there are still open files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2,
_g2.tvf=2, _dw.fdx=2, _g2.tvd=2, _g2.tvx=2, _ks.tvf=2,
_n9.tvx=2, _ks.tvx=2, _n9.fdx=2, _ks.fdx=2, _dw.cfx=1,
_n9.tvf=2, _cp.cfx=1, _s4.tvf=2, _dw.tvx=2, _87.fdx=2,
_fr.tvx=2, _87.tvf=2, _fr.tvd=2, _87.fdt=2, _ks.tvd=2,
_s4.tvd=2, _dw.tvd=2, _n9.fdt=2, _g2.fdt=2, _87.tvd=2,
_fr.fdt=2, _dw.fdt=2, _dj.cfx=1, _s4.tvx=2, _ks.fdt=2,
_n9.tvd=2, _fr.tvf=2, _fr.fdx=2, _dw.tvf=2, _87.tvx=2} [junit]
        at
org.apache.lucene.store.MockRAMDirectory.close(MockRAMDirectory.j
ava:278) [junit] at
org.apache.lucene.index.Test1726.testIndexing(Test1726.java:48)
[junit] at
org.apache.lucene.util.LuceneTestCase.runTest(LuceneTestCase.java
:88)
{code}

      was (Author: jasonrutherglen):
    I tried the test on trunk and get the same error. They're all
docstore related files so maybe extra doc stores are being
opened?

{code}
    [junit] MockRAMDirectory: cannot close: there are still open files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2, _g2.tvf=2, _dw.fdx=2, _g2.tvd=2, _g2.tvx=2, _ks.tvf=2, _n9.tvx=2, _ks.tvx=2, _n9.fdx=2, _ks.fdx=2, _dw.cfx=1, _n9.tvf=2, _cp.cfx=1, _s4.tvf=2, _dw.tvx=2, _87.fdx=2, _fr.tvx=2, _87.tvf=2, _fr.tvd=2, _87.fdt=2, _ks.tvd=2, _s4.tvd=2, _dw.tvd=2, _n9.fdt=2, _g2.fdt=2, _87.tvd=2, _fr.fdt=2, _dw.fdt=2, _dj.cfx=1, _s4.tvx=2, _ks.fdt=2, _n9.tvd=2, _fr.tvf=2, _fr.fdx=2, _dw.tvf=2, _87.tvx=2}
    [junit] java.lang.RuntimeException: MockRAMDirectory: cannot close: there are still open files: {_s4.fdt=2, _g2.fdx=2, _s4.fdx=2, _g2.tvf=2, _dw.fdx=2, _g2.tvd=2, _g2.tvx=2, _ks.tvf=2, _n9.tvx=2, _ks.tvx=2, _n9.fdx=2, _ks.fdx=2, _dw.cfx=1, _n9.tvf=2, _cp.cfx=1, _s4.tvf=2, _dw.tvx=2, _87.fdx=2, _fr.tvx=2, _87.tvf=2, _fr.tvd=2, _87.fdt=2, _ks.tvd=2, _s4.tvd=2, _dw.tvd=2, _n9.fdt=2, _g2.fdt=2, _87.tvd=2, _fr.fdt=2, _dw.fdt=2, _dj.cfx=1, _s4.tvx=2, _ks.fdt=2, _n9.tvd=2, _fr.tvf=2, _fr.fdx=2, _dw.tvf=2, _87.tvx=2}
    [junit] at org.apache.lucene.store.MockRAMDirectory.close(MockRAMDirectory.java:278)
    [junit] at org.apache.lucene.index.Test1726.testIndexing(Test1726.java:48)
    [junit] at org.apache.lucene.util.LuceneTestCase.runTest(LuceneTestCase.java:88)
{code}
 

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

Hmm... I'll dig into this test case.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Jason Rutherglen commented on LUCENE-1726:
------------------------------------------

Mike,

I was wondering if you can recommend techniques or tools for
debugging this type of multithreading issue? (i.e. how do you go
about figuring this type of issue out?)

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

I don't have any particular tools...

First I simplify the test as much as possible while still hitting the
failure (eg this failure happens w/ only 2 threads), then see if the
error will happen if I turn on IndexWriter's infoStream (it doesn't
for this, so far).  If so, I scrutinize the series of events to find
the hazard; else, I turn off infoStream and add back in a small number
of prints, as long as failure still happens.

Often I use a simple Python script that runs the test over & over
until a failure happens, saving the log, and then scrutinize that.

It's good to start with a rough guess, eg this failure is w/ only doc
stores so it seems likely the merging logic that opens doc stores just
before kicking off the merge may be to blame.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael McCandless updated LUCENE-1726:
---------------------------------------

    Attachment: LUCENE-1726.patch

OK the problem happens when a segment is first opened by a merge that
doesn't need to merge the doc stores; later, an NRT reader is opened
that separately opens the doc stores of the same [pooled]
SegmentReader, but then it's the merge that closes the read-only clone
of the reader.

In this case the separately opened (by the NRT reader) doc stores are
not closed by the merge thread.  It's the mirror image of LUCENE-1639.

I've fixed it by pulling all shared readers in a SegmentReader into a
separate static class (CoreReaders).  Cloned SegmentReaders share the
same instance of this class so that if a clone later opens the doc
stores, any prior ancestor (that the clone was created from) would
also close those readers if it's the reader to decRef to 0.

I did something similar for LUCENE-1609 (which I'll now hit conflicts
on after committing this... sigh).

I plan to commit in a day or so.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Jason Rutherglen commented on LUCENE-1726:
------------------------------------------

The test now passes, needs to go in the patch, perhaps in
TestIndexWriterReader? Great work on this, it's easier to
understand SegmentReader now that all the shared objects are in
one object (CoreReaders). It should make debugging go more
smoothly.

Is there a reason we're not synchronizing on SR.core in
openDocStores? Couldn't we synchronize on core for the cloning
methods?

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...

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

Michael McCandless commented on LUCENE-1726:
--------------------------------------------

bq. Is there a reason we're not synchronizing on SR.core in openDocStores?

I was going to say "because IW sychronizes" but in fact it doesn't,
properly, because when merging we go and open doc stores in
unsynchronized context.  So I'll synchronize(core) in
SR.openDocStores.

bq. Couldn't we synchronize on core for the cloning methods?

I don't think that's needed?  The core is simply carried over to the
newly cloned reader.

> IndexWriter.readerPool create new segmentReader outside of sync block
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-1726
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1726
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 2.4.1
>            Reporter: Jason Rutherglen
>            Assignee: Michael McCandless
>            Priority: Trivial
>             Fix For: 3.1
>
>         Attachments: LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.patch, LUCENE-1726.trunk.test.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> I think we will want to do something like what field cache does
> with CreationPlaceholder for IndexWriter.readerPool. Otherwise
> we have the (I think somewhat problematic) issue of all other
> readerPool.get* methods waiting for an SR to warm.
> It would be good to implement this for 2.9.

--
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@...
For additional commands, e-mail: java-dev-help@...