Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.10, 6.0
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      Since LUCENE-5711, MergePolicy is no longer wired to an IndexWriter instance. Therefore it can be moved to be a live setting on IndexWriter, which will allow apps to plug-in an MP on a live IW instance, without closing/reopening the writer. See for example LUCENE-5526 - instead of adding MP to forceMerge, apps could change the MP before calling forceMerge, with e.g. an UpgradeIndexMergePolicy.

      I think we can also make MergeScheduler a live setting, though I currently don't see the benefits of doing that, so I'd rather not touch it now.

      1. LUCENE-5883.patch
        19 kB
        Shai Erera
      2. LUCENE-5883.patch
        11 kB
        Shai Erera

        Activity

        Hide
        Shai Erera added a comment -

        Patch moves MergePolicy to LiveIndexWriterConfig. Note though that at the moment I removed the calls to MegePolicy.close() from within IndexWriter as I think it's wrong to do that. I also asked a question on the mailing list: http://markmail.org/message/psoohjaye3l3ejxl

        Show
        Shai Erera added a comment - Patch moves MergePolicy to LiveIndexWriterConfig. Note though that at the moment I removed the calls to MegePolicy.close() from within IndexWriter as I think it's wrong to do that. I also asked a question on the mailing list: http://markmail.org/message/psoohjaye3l3ejxl
        Hide
        Michael McCandless added a comment -

        +1

        Maybe note in the javadocs that the change only takes effect for subsequent merge selection, i.e. any merges in flight or any merges already registered by the previous MergePolicy "win".

        Show
        Michael McCandless added a comment - +1 Maybe note in the javadocs that the change only takes effect for subsequent merge selection, i.e. any merges in flight or any merges already registered by the previous MergePolicy "win".
        Hide
        Shai Erera added a comment -

        Thanks Mike, I improved the jdocs. This patch also removes Closeable from MergePolicy as discussed on the mailing list.

        Show
        Shai Erera added a comment - Thanks Mike, I improved the jdocs. This patch also removes Closeable from MergePolicy as discussed on the mailing list.
        Hide
        Shai Erera added a comment -

        BTW, the way I implemented it is that top-level API methods grab the current MergePolicy on the IWC and pass it down to methods like mergeMiddle(), commitInternal() etc., so that if the MP changes in the middle, nothing weird happens. E.g. if a merge was registered, but midway the MP changes and its CFS settings are different ... I think it's safer and it's not important to have THE most recent MP affect already registered merges.

        Show
        Shai Erera added a comment - BTW, the way I implemented it is that top-level API methods grab the current MergePolicy on the IWC and pass it down to methods like mergeMiddle(), commitInternal() etc., so that if the MP changes in the middle, nothing weird happens. E.g. if a merge was registered, but midway the MP changes and its CFS settings are different ... I think it's safer and it's not important to have THE most recent MP affect already registered merges.
        Hide
        Shai Erera added a comment -

        If it wasn't clear, I think this is ready to commit.

        Show
        Shai Erera added a comment - If it wasn't clear, I think this is ready to commit.
        Hide
        Michael McCandless added a comment -

        +1, thanks Shai!

        Show
        Michael McCandless added a comment - +1, thanks Shai!
        Hide
        Uwe Schindler added a comment - - edited

        +1, very good!

        I think LUCENE-5526 is not obsolete by that, but much easier to implement. I still would like to pass a MergePolicy to forceMerge, just for the forced merging.

        On the other hand, you can change merge policy, forceMerge() without waiting, and change back. This is possible, because the merge policy does not change for running merges...

        Show
        Uwe Schindler added a comment - - edited +1, very good! I think LUCENE-5526 is not obsolete by that, but much easier to implement. I still would like to pass a MergePolicy to forceMerge, just for the forced merging. On the other hand, you can change merge policy, forceMerge() without waiting, and change back. This is possible, because the merge policy does not change for running merges...
        Hide
        Shai Erera added a comment -

        Indeed. With this patch it would mean to pass the given MP or config.getMP(). We can discuss on LUCENE-5526 if it's needed at all though, I mean if it's OK to add another API to IW, when the app can change the MP, call forceMerge, change it back. But let's discuss there.

        Show
        Shai Erera added a comment - Indeed. With this patch it would mean to pass the given MP or config.getMP(). We can discuss on LUCENE-5526 if it's needed at all though, I mean if it's OK to add another API to IW, when the app can change the MP, call forceMerge, change it back. But let's discuss there.
        Hide
        ASF subversion and git services added a comment -

        Commit 1617910 from Shai Erera in branch 'dev/trunk'
        [ https://svn.apache.org/r1617910 ]

        LUCENE-5883: Move MergePolicy to LiveIndexWriterConfig

        Show
        ASF subversion and git services added a comment - Commit 1617910 from Shai Erera in branch 'dev/trunk' [ https://svn.apache.org/r1617910 ] LUCENE-5883 : Move MergePolicy to LiveIndexWriterConfig
        Hide
        ASF subversion and git services added a comment -

        Commit 1617911 from Shai Erera in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1617911 ]

        LUCENE-5883: Move MergePolicy to LiveIndexWriterConfig

        Show
        ASF subversion and git services added a comment - Commit 1617911 from Shai Erera in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1617911 ] LUCENE-5883 : Move MergePolicy to LiveIndexWriterConfig
        Hide
        Shai Erera added a comment -

        Committed to trunk and 4x.

        Show
        Shai Erera added a comment - Committed to trunk and 4x.

          People

          • Assignee:
            Shai Erera
            Reporter:
            Shai Erera
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development