[jira] [Comment Edited] (LUCENE-4953) readerClosedListener is not invoked for ParallelCompositeReader's leaves
[ 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
[ 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