Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      I'd like to add a simple and useful MP implementation which does .... nothing ! . I've came across many places where either the following is documented or implemented: "if you want to prevent merges, set mergeFactor to a high enough value". I think a NoOpMergePolicy is just as good, and can REALLY allow you disable merges (except for maybe set mergeFactor to Int.MAX_VAL).

      As such, NoOpMergePolicy will be introduced as a singleton, and can be used for convenience purposes only. Also, for Parallel Index it's important, because I'd like the slices to never do any merges, unless ParallelWriter decides so. So they should be set w/ that MP.

      I have a patch ready. Waiting for LUCENE-2320 to go in, so that I don't need to change it afterwards.

      About the name - I like the name, but suggestions are welcome. I thought of a NullMergePolicy, but I don't like 'Null' used for a NoOp.

      1. LUCENE-2331.patch
        14 kB
        Shai Erera
      2. LUCENE-2331.patch
        13 kB
        Shai Erera
      3. LUCENE-2331.patch
        13 kB
        Shai Erera

        Activity

        Hide
        Earwin Burrfoot added a comment -

        NoMergesPolicy - that's exactly what it is, a policy of no merges

        Show
        Earwin Burrfoot added a comment - NoMergesPolicy - that's exactly what it is, a policy of no merges
        Hide
        Shai Erera added a comment -

        I like NoMergesPolicy ... perhaps, like NoLockFactory, we can call it NoMergePolicy? so MP is preserved in the name (not that it's critical)?

        Show
        Shai Erera added a comment - I like NoMergesPolicy ... perhaps, like NoLockFactory, we can call it NoMergePolicy? so MP is preserved in the name (not that it's critical)?
        Hide
        Michael McCandless added a comment -

        +1 for NoMergePolicy

        Show
        Michael McCandless added a comment - +1 for NoMergePolicy
        Hide
        Shai Erera added a comment -

        In the process, I'll also add a NoMergeScheduler which will have empty implementations of MS. That's kind of redundant if one uses NoMP, however for symmetry it's nice to have it, as well as for not running any unnecessary code, like CMS and its threads, just to discover MP returned nothing.

        Show
        Shai Erera added a comment - In the process, I'll also add a NoMergeScheduler which will have empty implementations of MS. That's kind of redundant if one uses NoMP, however for symmetry it's nice to have it, as well as for not running any unnecessary code, like CMS and its threads, just to discover MP returned nothing.
        Hide
        Shai Erera added a comment -

        Patch includes:

        • NoMergePolicy + TestNoMergePolicy
        • NoMergeScheduler + TestNoMergeScheduler
        • MergeScheduler - methods changed to public
        • CHANGES entry (New Features)
        Show
        Shai Erera added a comment - Patch includes: NoMergePolicy + TestNoMergePolicy NoMergeScheduler + TestNoMergeScheduler MergeScheduler - methods changed to public CHANGES entry (New Features)
        Hide
        Michael McCandless added a comment -

        Patch looks good Shai!

        But, can you normalize the white space (2 space indent)?

        Also... do you think we should allow instantiation of NoMergePolicy, allowing you to control if it uses CFS or not?

        Show
        Michael McCandless added a comment - Patch looks good Shai! But, can you normalize the white space (2 space indent)? Also... do you think we should allow instantiation of NoMergePolicy, allowing you to control if it uses CFS or not?
        Hide
        Shai Erera added a comment -

        Sorry - new eclipse and project settings . Should be ok now.

        Show
        Shai Erera added a comment - Sorry - new eclipse and project settings . Should be ok now.
        Hide
        Shai Erera added a comment -

        do you think we should allow instantiation of NoMergePolicy, allowing you to control if it uses CFS or not?

        You ask because of the useCompound* methods? I wanted NMP to be a singleton really, and I don't think those two really matter? Meaning, if you are using it, I guess you don't really care if it uses a cmpnd file or not?

        But if you think it's important, I can create 3 singletons: NO_COMPOUND_FILES_AND_STORE, COMPOUND_FILES, COMPOUND_FILES_AND_STORE (I really hate the long names though). We can settle w/ just two - (NO)COMPOUND_FILES ...

        Show
        Shai Erera added a comment - do you think we should allow instantiation of NoMergePolicy, allowing you to control if it uses CFS or not? You ask because of the useCompound* methods? I wanted NMP to be a singleton really, and I don't think those two really matter? Meaning, if you are using it, I guess you don't really care if it uses a cmpnd file or not? But if you think it's important, I can create 3 singletons: NO_COMPOUND_FILES_AND_STORE, COMPOUND_FILES, COMPOUND_FILES_AND_STORE (I really hate the long names though). We can settle w/ just two - (NO)COMPOUND_FILES ...
        Hide
        Michael McCandless added a comment -

        You ask because of the useCompound* methods?

        Yes, because these methods (unexpectedly – not good) also affect whether newly flushed segments are CFS or not.

        I think just the two is OK? One can always make their own class if they really need diff't settings for the stores vs non-stores.

        Show
        Michael McCandless added a comment - You ask because of the useCompound* methods? Yes, because these methods (unexpectedly – not good) also affect whether newly flushed segments are CFS or not. I think just the two is OK? One can always make their own class if they really need diff't settings for the stores vs non-stores.
        Hide
        Shai Erera added a comment -

        Patch includes NoMergePolicy.NO_COMPOUND_FILES and COMPOUND_FILES singletons.

        Show
        Shai Erera added a comment - Patch includes NoMergePolicy.NO_COMPOUND_FILES and COMPOUND_FILES singletons.
        Hide
        Michael McCandless added a comment -

        In the jdocs for NoMergeScheduler you say this:

        Note that you can achieve the same thing by using {@link NoMergePolicy}, however with {@link NoMergeScheduler}
        you also ensure that no unnecessary code of any {@link MergeScheduler} implementation is ever executed. 
        

        Should that

        {@link MergeScheduler}

        be

        {@link MergePolicy}

        instead? I can fix...

        Show
        Michael McCandless added a comment - In the jdocs for NoMergeScheduler you say this: Note that you can achieve the same thing by using {@link NoMergePolicy}, however with {@link NoMergeScheduler} you also ensure that no unnecessary code of any {@link MergeScheduler} implementation is ever executed. Should that {@link MergeScheduler} be {@link MergePolicy} instead? I can fix...
        Hide
        Shai Erera added a comment -

        I think it's correct. The idea is to say that even w/ NMP, if you use NMS you ensure that no MS code is ever run (e.g. if you use NMP only, then CMS code [default] will always run but won't do anything).

        Show
        Shai Erera added a comment - I think it's correct. The idea is to say that even w/ NMP, if you use NMS you ensure that no MS code is ever run (e.g. if you use NMP only, then CMS code [default] will always run but won't do anything).
        Hide
        Michael McCandless added a comment -

        OK, then I'll leave it.

        Patch looks good – these are simple additions – I'll commit shortly. Thanks Shai!

        Show
        Michael McCandless added a comment - OK, then I'll leave it. Patch looks good – these are simple additions – I'll commit shortly. Thanks Shai!
        Hide
        Michael McCandless added a comment -

        Committed on newtrunk.

        Show
        Michael McCandless added a comment - Committed on newtrunk.

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Shai Erera
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development