[jira] [Comment Edited] (LUCENE-4953) readerClosedListener is not invoked for ParallelCompositeReader's leaves

2013-04-24 Thread Uwe Schindler (JIRA)

[ 
https://issues.apache.org/jira/browse/LUCENE-4953?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13640943#comment-13640943
 ] 

Uwe Schindler edited comment on LUCENE-4953 at 4/24/13 8:58 PM:


As discussed in IRC today, like MultiReader/SlowComposite that warps another 
reader, we should no call close() on the subreaders. The problem here is: E.g., 
A wrapped slow reader still returns its delegate as core cache key, so the 
reader closed listener works as expected, when the child is closed. But PMR 
must not return the inner cache keys, as it wraps them with modifying index 
contents.

The problem here is that the leaves are atomic and the field cache does not get 
the reader closed event. The somehow best fix would be to call 
readerClosedListener for all childs (not leaves!) on doClose. Theoretically, 
all CompositeReaders that wrap ll childs of other readers should do this (only 
Parallel is currently doing this).

Another idea would be to use incRef and decRef, but that would not affect the 
wrapped atomic readers, so I prefer the previous solution.

  was (Author: thetaphi):
As discussed in IRC today, like MultiReader/SlowComposite that warps 
another reader, we should no call close() on the subreaders. The problem here 
is: A wrapped slow reader still returns its delegate as core cache key, so the 
reader closed listener works as expected.

The problem here is that the leaves are atomic and the field cache does not get 
the reader closed event. The somehow best fix would be to call 
readerClosedListener for all childs (not leaves!) on close. Theoretically, all 
CompositeReaders that wrap ll childs of other readers should do this (only 
Parallel is currently doing this).

Another idea would be to use incRef and decRef, but that would not affect the 
wrapped atomic readers, so I prefer the previous solution.
  
 readerClosedListener is not invoked for ParallelCompositeReader's leaves
 

 Key: LUCENE-4953
 URL: https://issues.apache.org/jira/browse/LUCENE-4953
 Project: Lucene - Core
  Issue Type: Bug
Reporter: Michael McCandless
 Fix For: 5.0, 4.4

 Attachments: LUCENE-4953.patch, LUCENE-4953.patch


 There was a test failure last night:
 {noformat}
 1 tests failed.
 REGRESSION:  
 org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest.testBasic
 Error Message:
 testBasic(org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest): 
 Insane FieldCache usage(s) found expected:0 but was:2
 Stack Trace:
 java.lang.AssertionError: 
 testBasic(org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest): 
 Insane FieldCache usage(s) found expected:0 but was:2
 at 
 __randomizedtesting.SeedInfo.seed([1F9C2A2AD23A8E02:B466373F0DE6082C]:0)
 at org.junit.Assert.fail(Assert.java:93)
 at org.junit.Assert.failNotEquals(Assert.java:647)
 at org.junit.Assert.assertEquals(Assert.java:128)
 at org.junit.Assert.assertEquals(Assert.java:472)
 at 
 org.apache.lucene.util.LuceneTestCase.assertSaneFieldCaches(LuceneTestCase.java:592)
 at 
 org.apache.lucene.util.TestRuleFieldCacheSanity$1.evaluate(TestRuleFieldCacheSanity.java:55)
 at 
 org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:46)
 at 
 com.carrotsearch.randomizedtesting.rules.SystemPropertiesInvariantRule$1.evaluate(SystemPropertiesInvariantRule.java:55)
 at 
 org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:49)
 at 
 org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:70)
 at 
 org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:48)
 at 
 com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
 at 
 com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:358)
 at 
 com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:782)
 at 
 com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:442)
 at 
 com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:746)
 at 
 com.carrotsearch.randomizedtesting.RandomizedRunner$3.evaluate(RandomizedRunner.java:648)
 at 
 com.carrotsearch.randomizedtesting.RandomizedRunner$4.evaluate(RandomizedRunner.java:682)
 at 
 com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:693)
 at 
 org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:46)
 at 
 

[jira] [Comment Edited] (LUCENE-4953) readerClosedListener is not invoked for ParallelCompositeReader's leaves

2013-04-24 Thread Uwe Schindler (JIRA)

[ 
https://issues.apache.org/jira/browse/LUCENE-4953?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13640979#comment-13640979
 ] 

Uwe Schindler edited comment on LUCENE-4953 at 4/24/13 9:29 PM:


bq. Ie, today LuceneTestCase.maybeWrapReader never incRefs the incoming reader, 
but with the fix, ParallelCompositeReader now does ... so this leads to 
failures.

Passing false instead of true to the wrapper is wrong. The solution is to leave 
the construction as is, just in doClose() call notifyReaderClosedListeners() on 
all subreaders (which is unfortunately private). But this would still not help 
for tests, because maybeWrapReader tries to hide itsself from FieldCache (this 
is why it has FCInvisibleMultiReader). Tests almost always close the original 
reader and never the wrapper, so with this patch the whole thing does not work.

The test failure is very seldom because it only happens:
- if you wrap (rarely) with a ParallelMultiReader (more rarely)
- use FieldCache

The number of tests with FieldCache is very small, so it took more than 1 year 
to see the first failure :-)

In fact the problem for the actual test failure is maybeWrapReader in 
combination with FieldCache. maybeWrapReader should not use 
ParallelCompositeReader, if it knows that FieldCache will be used. 
Unfortunately this is not known before.

The problem with the readerClosedListener not called by ParallelCompositeReader 
(with closeSubReaders=true) is real, and only affects PCR, because it creates 
atomic readers with own fieldcache key. Because of that it should manually 
unregister them (and not close them)

  was (Author: thetaphi):
bq. Ie, today LuceneTestCase.maybeWrapReader never incRefs the incoming 
reader, but with the fix, ParallelCompositeReader now does ... so this leads to 
failures.

Passing false instead of true to the wrapper is wrong. The solution is to leave 
the construction as is, just in doClose() call notifyReaderClosedListeners() on 
all subreaders (which is unfortunately private). But this would still not help 
for tests, because maybeWrapReader tries to hide itsself from FieldCache (this 
is why it has FCInvisibleMultiReader). Tests always class the original reader 
never the wrapper, so although with this patch the whole thing does not work.

The test failure is very seldom because it only happens:
- if you wrap (rarely) with a ParallelMultiReader (more rarely)
- use FieldCache

The number of tests with FieldCache is very small, so it took more than 1 year 
to see the first failure :-)

In fact the problem for the actual test failure is maybeWrapReader in 
combination with FieldCache. maybeWrapReader should not use 
ParallelCompositeReader, if it knows that FieldCache will be used. 
Unfortunately this is not known before.

The problem with the readerClosedListener not called by ParallelCompositeReader 
(with closeSubReaders=true) is real, and only affects PCR, because it creates 
atomic readers with own fieldcache key. Because of that it should manually 
unregister them (and not close them)
  
 readerClosedListener is not invoked for ParallelCompositeReader's leaves
 

 Key: LUCENE-4953
 URL: https://issues.apache.org/jira/browse/LUCENE-4953
 Project: Lucene - Core
  Issue Type: Bug
Reporter: Michael McCandless
 Fix For: 5.0, 4.4

 Attachments: LUCENE-4953.patch, LUCENE-4953.patch


 There was a test failure last night:
 {noformat}
 1 tests failed.
 REGRESSION:  
 org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest.testBasic
 Error Message:
 testBasic(org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest): 
 Insane FieldCache usage(s) found expected:0 but was:2
 Stack Trace:
 java.lang.AssertionError: 
 testBasic(org.apache.lucene.search.grouping.AllGroupHeadsCollectorTest): 
 Insane FieldCache usage(s) found expected:0 but was:2
 at 
 __randomizedtesting.SeedInfo.seed([1F9C2A2AD23A8E02:B466373F0DE6082C]:0)
 at org.junit.Assert.fail(Assert.java:93)
 at org.junit.Assert.failNotEquals(Assert.java:647)
 at org.junit.Assert.assertEquals(Assert.java:128)
 at org.junit.Assert.assertEquals(Assert.java:472)
 at 
 org.apache.lucene.util.LuceneTestCase.assertSaneFieldCaches(LuceneTestCase.java:592)
 at 
 org.apache.lucene.util.TestRuleFieldCacheSanity$1.evaluate(TestRuleFieldCacheSanity.java:55)
 at 
 org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:46)
 at 
 com.carrotsearch.randomizedtesting.rules.SystemPropertiesInvariantRule$1.evaluate(SystemPropertiesInvariantRule.java:55)
 at 
 org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:49)
 at