Details

    • Type: Improvement
    • Status: Resolved
    • Priority: 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
        11 kB
        Shai Erera
      2. LUCENE-5883.patch
        19 kB
        Shai Erera

        Activity

        Hide
        shaie Shai Erera added a comment -

        Committed to trunk and 4x.

        Show
        shaie Shai Erera added a comment - Committed to trunk and 4x.
        Hide
        jira-bot 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
        jira-bot 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
        jira-bot 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
        jira-bot 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
        shaie 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
        shaie 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
        thetaphi 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
        thetaphi 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
        mikemccand Michael McCandless added a comment -

        +1, thanks Shai!

        Show
        mikemccand Michael McCandless added a comment - +1, thanks Shai!
        Hide
        shaie Shai Erera added a comment -

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

        Show
        shaie Shai Erera added a comment - If it wasn't clear, I think this is ready to commit.
        Hide
        shaie 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
        shaie 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
        shaie 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
        shaie 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
        mikemccand 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
        mikemccand 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
        shaie 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
        shaie 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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development