Lucene.Net
  1. Lucene.Net
  2. LUCENENET-484

Some possibly major tests intermittently fail

    Details

      Description

      These tests will fail intermittently in Debug or Release mode, in the core test suite:

      1. Lucene.Net.Index:
        • TestConcurrentMergeScheduler.TestFlushExceptions – FIXED
      2. Lucene.Net.Store:
        • TestLockFactory.TestStressLocks – FIXED
      3. Lucene.Net.Search:
        • TestSort.TestParallelMultiSort – FIXED
      4. Lucene.Net.Util:
        • TestFieldCacheSanityChecker.TestInsanity1 – FIXED
        • TestFieldCacheSanityChecker.TestInsanity2 – FIXED
      5. Lucene.Net.Support
        • TestWeakHashTableMultiThreadAccess.Test – FIXED

      TestWeakHashTableMultiThreadAccess should be fine to remove along with the WeakHashTable in the Support namespace, since it's been replaced with WeakDictionary.

      1. Lucenenet-484-WeakDictionaryTests.patch
        51 kB
        Luc Vanlerberghe
      2. Lucenenet-484-WeakDictionary.patch
        1 kB
        Luc Vanlerberghe
      3. Lucenenet-484-NativeFSLockFactory.patch
        0.9 kB
        Luc Vanlerberghe
      4. Lucenenet-484-FieldCacheImpl.patch
        4 kB
        Luc Vanlerberghe

        Activity

        Hide
        Christopher Currens added a comment -

        I should note, since they fail so intermittently, they can be reproduced more consistently if you wrap the test in a loop of 20+ iterations.

        Show
        Christopher Currens added a comment - I should note, since they fail so intermittently, they can be reproduced more consistently if you wrap the test in a loop of 20+ iterations.
        Hide
        Christopher Currens added a comment -

        So, I figured out the issue with the ConcurrentMergeScheduler, it was simply a port issue for the actual Test. I had to add a workaround in FailOnlyOnFlush.Eval(Directory), since java would compare stack frame with "main" (the thread running the test, it seems), and only execute the code if that was the case. That doesn't exactly work in .NET code, so instead, I inverted the check too look for the thread name not containing "Merge Thread". I can no longer reproduce it at all, where I could before every time.

        Show
        Christopher Currens added a comment - So, I figured out the issue with the ConcurrentMergeScheduler, it was simply a port issue for the actual Test. I had to add a workaround in FailOnlyOnFlush.Eval(Directory) , since java would compare stack frame with "main" (the thread running the test, it seems), and only execute the code if that was the case. That doesn't exactly work in .NET code, so instead, I inverted the check too look for the thread name not containing "Merge Thread". I can no longer reproduce it at all, where I could before every time.
        Hide
        Prescott Nasser added a comment -

        I cannot reproduce any of the failures, except for TestInsanity1 - doesn't matter how much I loop it or rerun it. Are there any others who could test this aside from Chris and Myself to possibly provide more insight into the failures

        Show
        Prescott Nasser added a comment - I cannot reproduce any of the failures, except for TestInsanity1 - doesn't matter how much I loop it or rerun it. Are there any others who could test this aside from Chris and Myself to possibly provide more insight into the failures
        Hide
        Christopher Currens added a comment -

        Some of the tests I can only produce if a) I run it in Release mode, AND b) I run the entire test suite, not just the single test.

        Show
        Christopher Currens added a comment - Some of the tests I can only produce if a) I run it in Release mode, AND b) I run the entire test suite, not just the single test.
        Hide
        Itamar Syn-Hershko added a comment -

        That's probably a matter of things not being cleaned up properly in some tests? (didn't actually look at the tests, just the immediate thing that comes to mind)

        Show
        Itamar Syn-Hershko added a comment - That's probably a matter of things not being cleaned up properly in some tests? (didn't actually look at the tests, just the immediate thing that comes to mind)
        Hide
        Prescott Nasser added a comment -

        Has anyone had any luck with any more of these tests? I can still only reproduce TestInsanity1 - Win7 x64, vs2010

        Show
        Prescott Nasser added a comment - Has anyone had any luck with any more of these tests? I can still only reproduce TestInsanity1 - Win7 x64, vs2010
        Hide
        Luc Vanlerberghe added a comment -

        The failures in the TestSanity test cases are due to a bug in Cleanup which is called whenever a GC is detected in CleanIfNeeded (which is itself called from several places)
        Cleanup actually drops all cache entries that have live keys instead of the other way around!
        I also corrected a race condition in WeakKey<T>.Equals (that will probably only happen under heavy load when you least

        I'll post patches with the corrections and updated test cases in a minute...

        Show
        Luc Vanlerberghe added a comment - The failures in the TestSanity test cases are due to a bug in Cleanup which is called whenever a GC is detected in CleanIfNeeded (which is itself called from several places) Cleanup actually drops all cache entries that have live keys instead of the other way around! I also corrected a race condition in WeakKey<T>.Equals (that will probably only happen under heavy load when you least I'll post patches with the corrections and updated test cases in a minute...
        Hide
        Luc Vanlerberghe added a comment -

        Corrects both Clean() and WeakKey<T>.Equals

        Show
        Luc Vanlerberghe added a comment - Corrects both Clean() and WeakKey<T>.Equals
        Hide
        Luc Vanlerberghe added a comment -

        This patch removes WeakHashtable and uses its tests for WeakDictionary instead (I actually renamed the test files and updated the tests so subversion would keep the history, but the .patch format apparently doesn't keep that info...)

        Show
        Luc Vanlerberghe added a comment - This patch removes WeakHashtable and uses its tests for WeakDictionary instead (I actually renamed the test files and updated the tests so subversion would keep the history, but the .patch format apparently doesn't keep that info...)
        Hide
        Christopher Currens added a comment -

        Thanks Luc. This is great stuff. I'll run the patch on my local box and double check everything. Your help with this is appreciated by all of us!

        Show
        Christopher Currens added a comment - Thanks Luc. This is great stuff. I'll run the patch on my local box and double check everything. Your help with this is appreciated by all of us!
        Hide
        Christopher Currens added a comment -

        Applied the patches. Getting closer to resolving this issue.

        Show
        Christopher Currens added a comment - Applied the patches. Getting closer to resolving this issue.
        Hide
        Luc Vanlerberghe added a comment -

        TestStressLocks and TestStressLocksNativeFSLockFactory fail intermittently due to an UnauthorizedAccessException in NativeFSLockFactory.Obtain
        In the code it is actually described that on Windows you can get intermittent "Access Denied" exceptions and the java version handles them properly.

        BUT: in java, the "Access Denied" raises an IOException, whereas in .Net this raises an UnauthorizedAccessException

        So the solution is to add an identical catch block for the UnauthorizedAccessException.
        This will cause the lock obtain to simply fail and the calling methods to retry.

        Show
        Luc Vanlerberghe added a comment - TestStressLocks and TestStressLocksNativeFSLockFactory fail intermittently due to an UnauthorizedAccessException in NativeFSLockFactory.Obtain In the code it is actually described that on Windows you can get intermittent "Access Denied" exceptions and the java version handles them properly. BUT : in java, the "Access Denied" raises an IOException, whereas in .Net this raises an UnauthorizedAccessException So the solution is to add an identical catch block for the UnauthorizedAccessException. This will cause the lock obtain to simply fail and the calling methods to retry.
        Hide
        Luc Vanlerberghe added a comment -

        Fixes the intermittent failures in
        TestStressLocks and TestStressLocksNativeFSLockFactory

        Show
        Luc Vanlerberghe added a comment - Fixes the intermittent failures in TestStressLocks and TestStressLocksNativeFSLockFactory
        Hide
        Luc Vanlerberghe added a comment -

        TestSort.TestParallelMultiSort crashes sometimes because of a race condition in FieldCacheImpl.GetCacheEntries.

        It's only called in DEBUG during tests while calling FieldCacheSanityChecker.CheckSanity.
        In TestSort.TestParallelMultiSort this can happen on multiple threads simultaneously.

        I checked the java version and it has the same problem in version 3.0.3 but was corrected later in rev. 912330:
        "uschindler 21/02/2010 12:16:42 LUCENE-2273: Fixed bug in FieldCacheImpl.getCacheEntries() that used WeakHashMap incorrectly and lead to ConcurrentModificationException"

        The patch also correct FieldCachImpl.PurgeAllCaches() and .Purge(IndexReader r)
        The first is also only called in test, but the second one is called in the DoClose() methods on several Reader implementations and could cause trouble if left unpatched (though near to impossible to write a test for, IMHO)

        I'll post the translated patch in a minute

        Show
        Luc Vanlerberghe added a comment - TestSort.TestParallelMultiSort crashes sometimes because of a race condition in FieldCacheImpl.GetCacheEntries. It's only called in DEBUG during tests while calling FieldCacheSanityChecker.CheckSanity. In TestSort.TestParallelMultiSort this can happen on multiple threads simultaneously. I checked the java version and it has the same problem in version 3.0.3 but was corrected later in rev. 912330: "uschindler 21/02/2010 12:16:42 LUCENE-2273 : Fixed bug in FieldCacheImpl.getCacheEntries() that used WeakHashMap incorrectly and lead to ConcurrentModificationException" The patch also correct FieldCachImpl.PurgeAllCaches() and .Purge(IndexReader r) The first is also only called in test, but the second one is called in the DoClose() methods on several Reader implementations and could cause trouble if left unpatched (though near to impossible to write a test for, IMHO) I'll post the translated patch in a minute
        Hide
        Luc Vanlerberghe added a comment -

        Translated patch in rev. 912330 of java version:
        uschindler 21/02/2010 12:16:42 LUCENE-2273: Fixed bug in FieldCacheImpl.getCacheEntries() that used WeakHashMap incorrectly and lead to ConcurrentModificationException

        Fixes interittent failures in TestSort.TestParallelMultiSort

        Show
        Luc Vanlerberghe added a comment - Translated patch in rev. 912330 of java version: uschindler 21/02/2010 12:16:42 LUCENE-2273 : Fixed bug in FieldCacheImpl.getCacheEntries() that used WeakHashMap incorrectly and lead to ConcurrentModificationException Fixes interittent failures in TestSort.TestParallelMultiSort
        Hide
        Christopher Currens added a comment -

        Thanks for your help with all of these Luc. Thanks to your hard work, this issue is finally closed, and for the first time in a long time, the whole test suite should consistently pass!

        Show
        Christopher Currens added a comment - Thanks for your help with all of these Luc. Thanks to your hard work, this issue is finally closed, and for the first time in a long time, the whole test suite should consistently pass!

          People

          • Assignee:
            Unassigned
            Reporter:
            Christopher Currens
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development