Lucene - Core
  1. Lucene - Core
  2. LUCENE-3380

enable FileSwitchDirectory randomly in tests and fix compound-file/NoSuchDirectoryException bugs

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 4.9, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Looks like FileSwitchDirectory has the same bugs in it as LUCENE-3374.

      We should randomly enable this guy in tests and flush them all out the same way.

        Activity

        Hide
        Uwe Schindler added a comment -

        I assume the bugs in FileSwitchDirectory are the same NotExists Exceptions thrown

        We should maybe also add FileSwitchDirectory to the list of random directories. It could create two random directories (using LTC.newDirectory(false) 2 times with a suffix on the dir name like ".1" and ".2") and combine them with a FileSwitchDirectory. The Set<String> of extensions could be a random list of extensions from the IndexFileNames collection.

        Show
        Uwe Schindler added a comment - I assume the bugs in FileSwitchDirectory are the same NotExists Exceptions thrown We should maybe also add FileSwitchDirectory to the list of random directories. It could create two random directories (using LTC.newDirectory(false) 2 times with a suffix on the dir name like ".1" and ".2") and combine them with a FileSwitchDirectory. The Set<String> of extensions could be a random list of extensions from the IndexFileNames collection.
        Hide
        Robert Muir added a comment -

        no, there are problems involving compoundfilewriter as well!

        Show
        Robert Muir added a comment - no, there are problems involving compoundfilewriter as well!
        Hide
        Robert Muir added a comment -

        the compound file directory can be thought of easily:
        Imagine FileSwitchDirectory (F) which contains two directories (A and B)
        and in the configuration, "cfs" files go to A, everything else to B.

        so currently it calls F.createCompoundOutput("xxxx.cfs") which delegates to A.createCompoundOutput("xxxx.cfs") ->> CompoundFileWriter(A, "xxxx.cfs"), which then, new since LUCENE-3218, will create a.createOutput("xxxx.cfe")

        The problem is that this cfe file is created under the wrong directory, and you will get FNFE.

        We can use the solution I provided in LUCENE-3374, but seriously maybe we should rethink LUCENE-3218 before releasing, because this could break similar delegators and basically they will experience what is like index corruption.

        Show
        Robert Muir added a comment - the compound file directory can be thought of easily: Imagine FileSwitchDirectory (F) which contains two directories (A and B) and in the configuration, "cfs" files go to A, everything else to B. so currently it calls F.createCompoundOutput("xxxx.cfs") which delegates to A.createCompoundOutput("xxxx.cfs") ->> CompoundFileWriter(A, "xxxx.cfs"), which then, new since LUCENE-3218 , will create a.createOutput("xxxx.cfe") The problem is that this cfe file is created under the wrong directory, and you will get FNFE. We can use the solution I provided in LUCENE-3374 , but seriously maybe we should rethink LUCENE-3218 before releasing, because this could break similar delegators and basically they will experience what is like index corruption.
        Hide
        Uwe Schindler added a comment -

        In my opinion, the createCompoundOutput/Input should get both files then the directory can wrap correctly. Maybe the underlying CFWriter/Reader should only get 2 IndexInput/Output, no filenames, so the delegation can completely done in the directory.

        Show
        Uwe Schindler added a comment - In my opinion, the createCompoundOutput/Input should get both files then the directory can wrap correctly. Maybe the underlying CFWriter/Reader should only get 2 IndexInput/Output, no filenames, so the delegation can completely done in the directory.
        Hide
        Robert Muir added a comment -

        I think so too, this means that the createCompoundInput for example is going to have to read the version header and deal with the backwards case where there is no CFE file, but we can create a utility for this.

        Show
        Robert Muir added a comment - I think so too, this means that the createCompoundInput for example is going to have to read the version header and deal with the backwards case where there is no CFE file, but we can create a utility for this.
        Hide
        Robert Muir added a comment -

        and thinking about it more, that makes this approach hard too, because how would the delegation work?

        an alternative is for both these dirs (NRTCachingDIr/FileSwitchDIr) to implement special CompoundFileDirectories, or maybe we make a DelegatingCompoundFileDirectory even that they share, but this still doesn't solve the root problem: that its going to be even more confusing for the future and potentially a problem for any custom directories out there.

        Show
        Robert Muir added a comment - and thinking about it more, that makes this approach hard too, because how would the delegation work? an alternative is for both these dirs (NRTCachingDIr/FileSwitchDIr) to implement special CompoundFileDirectories, or maybe we make a DelegatingCompoundFileDirectory even that they share, but this still doesn't solve the root problem: that its going to be even more confusing for the future and potentially a problem for any custom directories out there.
        Hide
        Robert Muir added a comment -

        here's a patch with tests for the problems, and bugfixes.

        I didn't enable it randomly yet as thats more complicated and first I want to get these bugs fixed in trunk/branch.

        Show
        Robert Muir added a comment - here's a patch with tests for the problems, and bugfixes. I didn't enable it randomly yet as thats more complicated and first I want to get these bugs fixed in trunk/branch.
        Hide
        Steve Rowe added a comment -

        Bulk move 4.4 issues to 4.5 and 5.0

        Show
        Steve Rowe added a comment - Bulk move 4.4 issues to 4.5 and 5.0
        Hide
        Uwe Schindler added a comment -

        Move issue to Lucene 4.9.

        Show
        Uwe Schindler added a comment - Move issue to Lucene 4.9.

          People

          • Assignee:
            Robert Muir
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development