Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 7.0, 6.2
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Today, lucene is mostly write-once, but not always, and these are just very exceptional cases.

      This is an invitation for exceptional bugs: (and we have occasional test failures when doing "no-wait close" because of this).

      I would prefer it if we didn't try to delete files before we open them for write, and if we opened them with the CREATE_NEW option by default to throw an exception, if the file already exists.

      The trickier parts of the change are going to be IndexFileDeleter and exceptions on merge / CFS construction logic.

      Overall for IndexFileDeleter I think the least invasive option might be to only delete files older than the current commit point? This will ensure that inflateGens() always avoids trying to overwrite any files that were from an aborted segment.

      For CFS construction/exceptions on merge, we really need to remove the custom "sniping" of index files there and let only IndexFileDeleter delete files. My previous failed approach involved always consistently using TrackingDirectoryWrapper, but it failed, and only in backwards compatibility tests, because of LUCENE-6146 (but i could never figure that out). I am hoping this time I will be successful

      Longer term we should think about more simplifications, progress has been made on LUCENE-5987, but I think overall we still try to be a superhero for exceptions on merge?

      1. LUCENE-6171.patch
        29 kB
        Michael McCandless

        Activity

        Hide
        mikemccand Michael McCandless added a comment -

        Here's a naive initial patch ... all I did was add the CREATE_NEW
        flag in FSDirectory.createOutput, and removed
        MockDirectoryWrapper.setPreventDoubleWrite and all tests that
        invoked that.

        Lucene tests passed (once!); not sure about solr tests. I haven't looked into the other fixes Robert Muir suggested yet ...

        Show
        mikemccand Michael McCandless added a comment - Here's a naive initial patch ... all I did was add the CREATE_NEW flag in FSDirectory.createOutput , and removed MockDirectoryWrapper.setPreventDoubleWrite and all tests that invoked that. Lucene tests passed (once!); not sure about solr tests. I haven't looked into the other fixes Robert Muir suggested yet ...
        Hide
        dsmiley David Smiley added a comment -

        Curious – what does updatable docValues do? Does it not update a file or does each update create a new one, perhaps in parallel-index like manner?

        Show
        dsmiley David Smiley added a comment - Curious – what does updatable docValues do? Does it not update a file or does each update create a new one, perhaps in parallel-index like manner?
        Hide
        mikemccand Michael McCandless added a comment -

        does each update create a new one, perhaps in parallel-index like manner?

        That's what we do ... we always fully write the new doc values to another file, and stop referencing the old one.

        Show
        mikemccand Michael McCandless added a comment - does each update create a new one, perhaps in parallel-index like manner? That's what we do ... we always fully write the new doc values to another file, and stop referencing the old one.
        Hide
        rcmuir Robert Muir added a comment -

        +1. This part of the change speaks volumes:

             public FSIndexOutput(String name) throws IOException {
        -      this(name, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE);
        +      this(name, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
             }
        

        Some of the issues i mentioned may have been resolved already? This issue is quite old. I think many of these problems have been addressed since then.

        Show
        rcmuir Robert Muir added a comment - +1. This part of the change speaks volumes: public FSIndexOutput(String name) throws IOException { - this(name, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE); + this(name, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW); } Some of the issues i mentioned may have been resolved already? This issue is quite old. I think many of these problems have been addressed since then.
        Hide
        mikemccand Michael McCandless added a comment -

        Thanks Robert Muir ... I think we have made progress since then. I'll clean up the patch and make sure tests pass and push so we can get Jenkins chewing on it.

        Show
        mikemccand Michael McCandless added a comment - Thanks Robert Muir ... I think we have made progress since then. I'll clean up the patch and make sure tests pass and push so we can get Jenkins chewing on it.
        Hide
        mikemccand Michael McCandless added a comment -

        Bulk close resolved issues after 6.2.0 release.

        Show
        mikemccand Michael McCandless added a comment - Bulk close resolved issues after 6.2.0 release.

          People

          • Assignee:
            mikemccand Michael McCandless
            Reporter:
            rcmuir Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development