Lucene - Core
  1. Lucene - Core
  2. LUCENE-3296

Enable passing a config into PKIndexSplitter

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 3.3, 4.0-ALPHA
    • Fix Version/s: 3.4, 4.0-ALPHA
    • Component/s: core/other
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      I need to be able to pass the IndexWriterConfig into the IW used by PKIndexSplitter.

      1. LUCENE-3296.patch
        2 kB
        Jason Rutherglen
      2. LUCENE-3296.patch
        2 kB
        Jason Rutherglen
      3. LUCENE-3296.patch
        5 kB
        Simon Willnauer
      4. LUCENE-3296.patch
        7 kB
        Simon Willnauer

        Issue Links

          Activity

          Hide
          Jason Rutherglen added a comment -

          Patch, all tests pass.

          Show
          Jason Rutherglen added a comment - Patch, all tests pass.
          Hide
          Simon Willnauer added a comment -

          looks good.. do you have to use Version.LUCENE_CURRENT in the ctors or can you pass in a version. using Version.LUCENE_CURRENT is discouraged for several reasons.

          Show
          Simon Willnauer added a comment - looks good.. do you have to use Version.LUCENE_CURRENT in the ctors or can you pass in a version. using Version.LUCENE_CURRENT is discouraged for several reasons.
          Hide
          Jason Rutherglen added a comment -

          That was in there previously. Lets change it.

          Show
          Jason Rutherglen added a comment - That was in there previously. Lets change it.
          Hide
          Jason Rutherglen added a comment -

          This patch uses LUCENE_40. All tests pass.

          Show
          Jason Rutherglen added a comment - This patch uses LUCENE_40. All tests pass.
          Hide
          Uwe Schindler added a comment -

          Simon: The Version.LUCENE_CURRENT is not important here, for easier porting, the version should be LUCENE_CURRENT (and it was before Jason's patch). Else we will have to always upgrade it with every new release. The same applies to the IndexUpdater class in core, it also uses LUCENE_CURRENT when you not pass in anything (as the version is completely useless for simple merge operations - like here).

          I would take this issue, but with LUCENE_CURRENT.

          Show
          Uwe Schindler added a comment - Simon: The Version.LUCENE_CURRENT is not important here, for easier porting, the version should be LUCENE_CURRENT (and it was before Jason's patch). Else we will have to always upgrade it with every new release. The same applies to the IndexUpdater class in core, it also uses LUCENE_CURRENT when you not pass in anything (as the version is completely useless for simple merge operations - like here). I would take this issue, but with LUCENE_CURRENT.
          Hide
          Uwe Schindler added a comment -

          btw.: The LUCENE_CURRENT was inserted by myself in the orginal code

          Show
          Uwe Schindler added a comment - btw.: The LUCENE_CURRENT was inserted by myself in the orginal code
          Hide
          Jason Rutherglen added a comment -

          Uwe, the first patch [1] is implemented with CURRENT.

          1. https://issues.apache.org/jira/secure/attachment/12485805/LUCENE-3296.patch

          Show
          Jason Rutherglen added a comment - Uwe, the first patch [1] is implemented with CURRENT. 1. https://issues.apache.org/jira/secure/attachment/12485805/LUCENE-3296.patch
          Hide
          Simon Willnauer added a comment - - edited

          here is a new patch. I added a second IWC since we can not reuse IWC instances across IW due to SetOnce restrictions. I also moved out the VERSION_CURRENT and made it a ctor argument. We should not randomly use the VERSION_CURRENT but rather be consistent when we use version.

          Simon: The Version.LUCENE_CURRENT is not important here, for easier porting, the version should be LUCENE_CURRENT (and it was before Jason's patch). Else we will have to always upgrade it with every new release. The same applies to the IndexUpdater class in core, it also uses LUCENE_CURRENT when you not pass in anything (as the version is completely useless for simple merge operations - like here).

          not entirely true, we use the index splitter in 3.x and if you upgrade from 3.1 to 3.2 you get a new mergepolicy by default which doesn't merge in order. I think its a problem that this version is not in 3.x yet so let fix it properly and backport.

          Simon

          Show
          Simon Willnauer added a comment - - edited here is a new patch. I added a second IWC since we can not reuse IWC instances across IW due to SetOnce restrictions. I also moved out the VERSION_CURRENT and made it a ctor argument. We should not randomly use the VERSION_CURRENT but rather be consistent when we use version. Simon: The Version.LUCENE_CURRENT is not important here, for easier porting, the version should be LUCENE_CURRENT (and it was before Jason's patch). Else we will have to always upgrade it with every new release. The same applies to the IndexUpdater class in core, it also uses LUCENE_CURRENT when you not pass in anything (as the version is completely useless for simple merge operations - like here). not entirely true, we use the index splitter in 3.x and if you upgrade from 3.1 to 3.2 you get a new mergepolicy by default which doesn't merge in order. I think its a problem that this version is not in 3.x yet so let fix it properly and backport. Simon
          Hide
          Uwe Schindler added a comment -

          not entirely true, we use the index splitter in 3.x and if you upgrade from 3.1 to 3.2 you get a new mergepolicy by default which doesn't merge in order. I think its a problem that this version is not in 3.x yet so let fix it properly and backport.

          PKIndexSplitter is new in 3.3, so you would never used it with older versions...

          Show
          Uwe Schindler added a comment - not entirely true, we use the index splitter in 3.x and if you upgrade from 3.1 to 3.2 you get a new mergepolicy by default which doesn't merge in order. I think its a problem that this version is not in 3.x yet so let fix it properly and backport. PKIndexSplitter is new in 3.3, so you would never used it with older versions...
          Hide
          Uwe Schindler added a comment -

          We should not randomly use the VERSION_CURRENT but rather be consistent when we use version.

          I agree, but when you backport to 3.4, you have to keep backwards compatibility, so only deprecate the ctor there.

          IndexUpgrader only uses LUCENE_CURRENT when you invoke from command line, in all other cases its required arg, so we are consistent here.

          We should also look at the other IndexSplitters in this package!

          Show
          Uwe Schindler added a comment - We should not randomly use the VERSION_CURRENT but rather be consistent when we use version. I agree, but when you backport to 3.4, you have to keep backwards compatibility, so only deprecate the ctor there. IndexUpgrader only uses LUCENE_CURRENT when you invoke from command line, in all other cases its required arg, so we are consistent here. We should also look at the other IndexSplitters in this package!
          Hide
          Simon Willnauer added a comment -

          this patch includes added version to MultipassIndexSplitter ctor.

          I am going to commit this and backport to 3.x

          Show
          Simon Willnauer added a comment - this patch includes added version to MultipassIndexSplitter ctor. I am going to commit this and backport to 3.x
          Hide
          Simon Willnauer added a comment -

          Committed to trunk and backported to 3.x. I marked parts in 3.x as deprecated.

          Show
          Simon Willnauer added a comment - Committed to trunk and backported to 3.x. I marked parts in 3.x as deprecated.

            People

            • Assignee:
              Simon Willnauer
              Reporter:
              Jason Rutherglen
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development