RE: svn commit: r1363272 - /lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Hi, If you use a plain SlowCompositeReaderWrapper without an FCInvisible wrapper on the MultiReader behind, the FC insanity checker will warn the user! So FieldCache.DEFAULT.getInts(SlowCompositeReaderWrapper.wrap(directoryReader), field) will create insanity and the detector will find that. The reason for this is, because SlowCompositeReader returns the cache key of the directoryReader it wraps, so the insanity checker sees the child readers and shout "ALARM!". We just "hide" the composite reader key, so the random wrapping causes no insanity, because the atomic reader wrapper looks really atomic to the insanity checker. Uwe - Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de eMail: u...@thetaphi.de > -Original Message- > From: martijn.is.h...@gmail.com [mailto:martijn.is.h...@gmail.com] On > Behalf Of Martijn v Groningen > Sent: Monday, July 23, 2012 10:44 AM > To: dev@lucene.apache.org > Subject: Re: svn commit: r1363272 - /lucene/dev/trunk/lucene/test- > framework/src/java/org/apache/lucene/util/LuceneTestCase.java > > > That is wanted! We want to test SlowMultiReaderWrapper and verify that it > really behaves like a full conformant AtomicReader. Because of that we > sometimes war with it. The fix Robert did is fine and is identical to the one > we > did in the past. As this is no real Lucene usage pattern, not detecting the > obvious cache violation is wanted here. > Ok. I was just wondering about the fact that a toplevel readercontext is > given as > argument to Collector#setNextReader(). It confused me, but as you said it is > no > real Lucene usage pattern. > > > > > For incorrect Lucene usage (like getting a field cache entry on a slow > wrapper), we can still warn the user. Buf for tests this insanity is wanted > by the > wrapping. > I think the SlowCompositeReaderWrapper jdocs warns the user already enough, > so that is fine for now. > > Martijn > > - > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional > commands, e-mail: dev-h...@lucene.apache.org - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: svn commit: r1363272 - /lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
> That is wanted! We want to test SlowMultiReaderWrapper and verify that it > really behaves like a full conformant AtomicReader. Because of that we > sometimes war with it. The fix Robert did is fine and is identical to the one > we did in the past. As this is no real Lucene usage pattern, not detecting > the obvious cache violation is wanted here. Ok. I was just wondering about the fact that a toplevel readercontext is given as argument to Collector#setNextReader(). It confused me, but as you said it is no real Lucene usage pattern. > > For incorrect Lucene usage (like getting a field cache entry on a slow > wrapper), we can still warn the user. Buf for tests this insanity is wanted > by the wrapping. I think the SlowCompositeReaderWrapper jdocs warns the user already enough, so that is fine for now. Martijn - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
RE: svn commit: r1363272 - /lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Hi, > Thanks for also taking a look at the test failures! > > On 20 July 2012 19:14, Robert Muir wrote: > > Hi Martinj: thanks for looking into this! > > > > I think have a better fix for these: > > > > the problem is actually in the AssertingAtomicReaders that > > AssertingDirectoryReader wraps its subreaders with. > > So I added the invisible-cache-key hack there, and removed it > > completely from LuceneTestCase. > Yes this was the problem. I think this fix wouldn't make hudson fail again. > Since > the FC insanity can't be detected. > > During some test runs the AtomicReaderContext (in Collector#setNextReader > method) contains a SlowCompositeReaderWrapper and this reader wraps > another reader something like this: > SlowCompositeReaderWrapper(ParallelCompositeReader(FCInvisibleMultiRead > er(StandardDirectoryReader(segments_4:9 > _0(5.0):C463 _1(5.0):C930 _2(5.0):C378 _3(5.0):C184 > > This used to cause the FC insanity test failures, but is this a scenario that > actually needs to be tested? Having a top level readercontext in the > setNextReader method seems like wrong usage to me. That is wanted! We want to test SlowMultiReaderWrapper and verify that it really behaves like a full conformant AtomicReader. Because of that we sometimes war with it. The fix Robert did is fine and is identical to the one we did in the past. As this is no real Lucene usage pattern, not detecting the obvious cache violation is wanted here. For incorrect Lucene usage (like getting a field cache entry on a slow wrapper), we can still warn the user. Buf for tests this insanity is wanted by the wrapping. Uwe - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: svn commit: r1363272 - /lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Thanks for also taking a look at the test failures! On 20 July 2012 19:14, Robert Muir wrote: > Hi Martinj: thanks for looking into this! > > I think have a better fix for these: > > the problem is actually in the AssertingAtomicReaders that > AssertingDirectoryReader wraps its subreaders with. > So I added the invisible-cache-key hack there, and removed it > completely from LuceneTestCase. Yes this was the problem. I think this fix wouldn't make hudson fail again. Since the FC insanity can't be detected. During some test runs the AtomicReaderContext (in Collector#setNextReader method) contains a SlowCompositeReaderWrapper and this reader wraps another reader something like this: SlowCompositeReaderWrapper(ParallelCompositeReader(FCInvisibleMultiReader(StandardDirectoryReader(segments_4:9 _0(5.0):C463 _1(5.0):C930 _2(5.0):C378 _3(5.0):C184 This used to cause the FC insanity test failures, but is this a scenario that actually needs to be tested? Having a top level readercontext in the setNextReader method seems like wrong usage to me. Martijn - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: svn commit: r1363272 - /lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Hi Martinj: thanks for looking into this!
I think have a better fix for these:
the problem is actually in the AssertingAtomicReaders that
AssertingDirectoryReader wraps its subreaders with.
So I added the invisible-cache-key hack there, and removed it
completely from LuceneTestCase.
I tested this with the hudson seeds that failed (at their appropriate
revisions) and it seems to work fine.
I also ran tests for queries/grouping/join with -Dnightly=true,
-Dtests.multiplier=5, etc etc a few times and it all works.
I'd really like to have AssertingDirectoryReader being used again.
If there are problems we can just back out the change.
On Thu, Jul 19, 2012 at 5:48 AM, wrote:
> Author: mvg
> Date: Thu Jul 19 09:48:04 2012
> New Revision: 1363272
>
> URL: http://svn.apache.org/viewvc?rev=1363272&view=rev
> Log:
> Fix of rare FC insanity during tests that have occurred in grouping & joining
> tests.
>
> Modified:
>
> lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
>
> Modified:
> lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
> URL:
> http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java?rev=1363272&r1=1363271&r2=1363272&view=diff
> ==
> ---
> lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
> (original)
> +++
> lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
> Thu Jul 19 09:48:04 2012
> @@ -1048,7 +1048,7 @@ public abstract class LuceneTestCase ext
> if (r instanceof AtomicReader) {
>r = new FCInvisibleMultiReader(new
> AssertingAtomicReader((AtomicReader)r));
> } else if (r instanceof DirectoryReader) {
> - r = new FCInvisibleMultiReader(new
> AssertingDirectoryReader((DirectoryReader)r));
> + r = new FCInvisibleMultiReader((DirectoryReader)r);
> }
> break;
>default:
>
>
--
lucidimagination.com
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
