Lucene - Core
  1. Lucene - Core
  2. LUCENE-3583

benchmark tests always fail on windows because directory cannot be removed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.5, 4.0-ALPHA
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Environment:

      Only fails for Lucene trunk

    • Lucene Fields:
      New, Patch Available

      Description

      This seems to be a bug recently introduced. I have no idea what's wrong. Attached is a log file, reproduces everytime.

      1. LUCENE-3583.patch
        0.8 kB
        Shai Erera
      2. LUCENE-3583.patch
        3 kB
        Robert Muir
      3. io-event-log.txt
        141 kB
        Uwe Schindler
      4. benchmark-test-output.txt
        16 kB
        Uwe Schindler

        Activity

        Hide
        Uwe Schindler added a comment -

        Bulk close after release of 3.5

        Show
        Uwe Schindler added a comment - Bulk close after release of 3.5
        Hide
        Robert Muir added a comment -

        Shai: thanks for getting to the bottom of this!

        Show
        Robert Muir added a comment - Shai: thanks for getting to the bottom of this!
        Hide
        Shai Erera added a comment -

        Committed test fixes to 3x and trunk.

        Show
        Shai Erera added a comment - Committed test fixes to 3x and trunk.
        Hide
        Shai Erera added a comment -

        Ok found it: testInvalidFormat expected exceptions to be thrown while the algorithm runs, and so tasks.close() wasn't called. I'll change the code to close in a try-finally, for all Closeable resources.

        Show
        Shai Erera added a comment - Ok found it: testInvalidFormat expected exceptions to be thrown while the algorithm runs, and so tasks.close() wasn't called. I'll change the code to close in a try-finally, for all Closeable resources.
        Hide
        Shai Erera added a comment -

        Committed Robert's changes to trunk rev 1204851. However after porting to 3x and running the tests, LineDocSourceTest consistently fails with this:

        ant test -Dtestcase=LineDocSourceTest -Dtests.seed=-2895bf521e2206c:0:4f1f278a279ce441

        I'm investigating (don't know if it fails on trunk too yet).

        Show
        Shai Erera added a comment - Committed Robert's changes to trunk rev 1204851. However after porting to 3x and running the tests, LineDocSourceTest consistently fails with this: ant test -Dtestcase=LineDocSourceTest -Dtests.seed=-2895bf521e2206c:0:4f1f278a279ce441 I'm investigating (don't know if it fails on trunk too yet).
        Hide
        Shai Erera added a comment -

        You're right, I didn't notice it. I'll work on it now.

        Show
        Shai Erera added a comment - You're right, I didn't notice it. I'll work on it now.
        Hide
        Uwe Schindler added a comment -

        I still think we should also commit Robert's patch.

        Show
        Uwe Schindler added a comment - I still think we should also commit Robert's patch.
        Hide
        Shai Erera added a comment -

        Committed rev 1204826 (3x).
        Ported changes to trunk in rev 1204828.

        Show
        Shai Erera added a comment - Committed rev 1204826 (3x). Ported changes to trunk in rev 1204828.
        Hide
        Shai Erera added a comment -

        Patch fixes the problem in LineDocSourceTest - add tasks.close() (otherwise LDS keeps a reader open on the file).

        I intend to commit shortly, after verifying all tests pass and no other such changes are required.

        Show
        Shai Erera added a comment - Patch fixes the problem in LineDocSourceTest - add tasks.close() (otherwise LDS keeps a reader open on the file). I intend to commit shortly, after verifying all tests pass and no other such changes are required.
        Hide
        Robert Muir added a comment -

        Setting affects 3.x, becuase i'm gonna backport my previous fix that unravelled this problem.

        Show
        Robert Muir added a comment - Setting affects 3.x, becuase i'm gonna backport my previous fix that unravelled this problem.
        Hide
        Robert Muir added a comment -

        By the way this is the reason these tests fail now:
        http://svn.apache.org/viewvc/lucene/dev/branches/lucene2621/modules/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java?view=diff&r1=1201966&r2=1201967&pathrev=1201967

        Before they just created files underneath TEMP_DIR directly and were not closing things and were silently wrong... so the
        problems have been here a while.

        Show
        Robert Muir added a comment - By the way this is the reason these tests fail now: http://svn.apache.org/viewvc/lucene/dev/branches/lucene2621/modules/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java?view=diff&r1=1201966&r2=1201967&pathrev=1201967 Before they just created files underneath TEMP_DIR directly and were not closing things and were silently wrong... so the problems have been here a while.
        Hide
        Robert Muir added a comment -

        with this patch the benchmark tests pass most of the time on my windows computer.

        But sometimes they still fail (with the same filenames involved)... I have no idea why.

        Show
        Robert Muir added a comment - with this patch the benchmark tests pass most of the time on my windows computer. But sometimes they still fail (with the same filenames involved)... I have no idea why.
        Hide
        Robert Muir added a comment -

        When LuceneTestCase cannot delete a file that it wants to, I'm gonna see if we can improve the output for starters.

        This would make it easier to debug issues like this.

        Show
        Robert Muir added a comment - When LuceneTestCase cannot delete a file that it wants to, I'm gonna see if we can improve the output for starters. This would make it easier to debug issues like this.
        Hide
        Uwe Schindler added a comment -

        Here the complete I/O event log created by sysinternals process monitor when running test TestLineDocSource

        Show
        Uwe Schindler added a comment - Here the complete I/O event log created by sysinternals process monitor when running test TestLineDocSource
        Hide
        Robert Muir added a comment -

        It looks to me like the problem is inside LineDocSourceTest:doIndexAndSearchTest

        i ran the test with -Dtestmethod=testWithDocsName, and it passes if i comment out the call to this method.

        Show
        Robert Muir added a comment - It looks to me like the problem is inside LineDocSourceTest:doIndexAndSearchTest i ran the test with -Dtestmethod=testWithDocsName, and it passes if i comment out the call to this method.
        Hide
        Robert Muir added a comment -

        OK it fails even if you have no space in the directory.

        So i'm guessing this test leaves a file open (outside of mockdirectorywrapper)

        We need FileSystem.setDefault(new MockWindowsFileSystem())...

        Show
        Robert Muir added a comment - OK it fails even if you have no space in the directory. So i'm guessing this test leaves a file open (outside of mockdirectorywrapper) We need FileSystem.setDefault(new MockWindowsFileSystem())...
        Hide
        Robert Muir added a comment -

        hudson needs a space in its folder too

        The problem is BenchmarkTestCase, i'm sure i introduced the bug when i fixed another bug (it used TEMP_DIR directly).

        Show
        Robert Muir added a comment - hudson needs a space in its folder too The problem is BenchmarkTestCase, i'm sure i introduced the bug when i fixed another bug (it used TEMP_DIR directly).
        Hide
        Uwe Schindler added a comment -

        Only fails for Lucene trunk, seems to be related to the additional cleanupo by the removal of new IndexSearcher(directory).

        Show
        Uwe Schindler added a comment - Only fails for Lucene trunk, seems to be related to the additional cleanupo by the removal of new IndexSearcher(directory).

          People

          • Assignee:
            Shai Erera
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development