Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-3061

Open IndexWriter API to allow custom MergeScheduler implementation

    Details

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

      Description

      IndexWriter's getNextMerge() and merge(OneMerge) are package-private, which makes it impossible for someone to implement his own MergeScheduler. We should open up these API, as well as any other that can be useful for custom MS implementations.

      1. LUCENE-3061.patch
        4 kB
        Shai Erera
      2. LUCENE-3061.patch
        5 kB
        Shai Erera

        Activity

        Hide
        shaie Shai Erera added a comment -

        Open up necessary API + add TestCustomMergeScheduler under src/test/o.a.l/index/publicapi.

        The changes are very trivial. If you would like to suggest alternative package I should put the test in, I will gladly do it.

        Show
        shaie Shai Erera added a comment - Open up necessary API + add TestCustomMergeScheduler under src/test/o.a.l/index/publicapi. The changes are very trivial. If you would like to suggest alternative package I should put the test in, I will gladly do it.
        Hide
        thetaphi Uwe Schindler added a comment -

        All of those the public API tests are directly under o.a.lucene at the moment.

        Show
        thetaphi Uwe Schindler added a comment - All of those the public API tests are directly under o.a.lucene at the moment.
        Hide
        shaie Shai Erera added a comment -

        Thanks Uwe ! Following your comment, I noticed there is a TestMergeSchedulerExternal under o.a.l, which covers extending ConcurrentMergeScheduler.

        So I moved my MS impl + test case there.

        I think this is ready to commit

        Show
        shaie Shai Erera added a comment - Thanks Uwe ! Following your comment, I noticed there is a TestMergeSchedulerExternal under o.a.l, which covers extending ConcurrentMergeScheduler. So I moved my MS impl + test case there. I think this is ready to commit
        Hide
        earwin Earwin Burrfoot added a comment -

        Mark these as @experimental?

        Show
        earwin Earwin Burrfoot added a comment - Mark these as @experimental?
        Hide
        shaie Shai Erera added a comment -

        I don't think they are experimental though – they exist for ages. We only made them public.

        I get your point - you don't think we should commit to this API signature, but IMO we should – if MS is a valid extension point by applications, we must support this API, otherwise MS cannot be extended at all. Also, getNextMerge() jdoc specifies "Expert: the MergeScheeduler calls this method ..." - this kind of makes this API public long time ago, only it wasn't.

        Show
        shaie Shai Erera added a comment - I don't think they are experimental though – they exist for ages. We only made them public. I get your point - you don't think we should commit to this API signature, but IMO we should – if MS is a valid extension point by applications, we must support this API, otherwise MS cannot be extended at all. Also, getNextMerge() jdoc specifies "Expert: the MergeScheeduler calls this method ..." - this kind of makes this API public long time ago, only it wasn't.
        Hide
        mikemccand Michael McCandless added a comment -

        I think they should be @experimental? (Eg, MS itself is).

        Show
        mikemccand Michael McCandless added a comment - I think they should be @experimental? (Eg, MS itself is).
        Hide
        shaie Shai Erera added a comment -

        I didn't notice MS is experimental. It's weird (to me) that it is. Perhaps we need another tag @lucene.expert, with same semantics as experimental, but better name. Experimental feels like I'm touching stuff that is not fully ready yet.

        But nm, I'll tag these method as experimental. We can discuss the @expert tag on the list, outside this issue.

        I'll commit shortly.

        Show
        shaie Shai Erera added a comment - I didn't notice MS is experimental. It's weird (to me) that it is. Perhaps we need another tag @lucene.expert, with same semantics as experimental, but better name. Experimental feels like I'm touching stuff that is not fully ready yet. But nm, I'll tag these method as experimental. We can discuss the @expert tag on the list, outside this issue. I'll commit shortly.
        Hide
        shaie Shai Erera added a comment -

        Committed revision 1098543 (3x).
        Committed revision 1098576 (trunk).

        Show
        shaie Shai Erera added a comment - Committed revision 1098543 (3x). Committed revision 1098576 (trunk).
        Hide
        rcmuir Robert Muir added a comment -

        Bulk closing for 3.2

        Show
        rcmuir Robert Muir added a comment - Bulk closing for 3.2

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development