Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.9
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Wrap SegmentInfos in a public class so that subclasses of MergePolicy do not need to be in the org.apache.lucene.index package.

      1. LUCENE-1742.patch
        5 kB
        Jason Rutherglen
      2. LUCENE-1742.patch
        10 kB
        Jason Rutherglen
      3. LUCENE-1742.patch
        12 kB
        Jason Rutherglen
      4. LUCENE-1742.patch
        14 kB
        Jason Rutherglen
      5. LUCENE-1742.patch
        15 kB
        Michael McCandless

        Activity

        Hide
        Jason Rutherglen added a comment -

        In order for this class to be compatible with out current
        default LogMergePolicy, we'll need to expose readers from the IW
        reader pool. This is because presumably classes may need to
        access readers such as in
        LogMergePolicy.findMergesToExpungeDeletes.

        Show
        Jason Rutherglen added a comment - In order for this class to be compatible with out current default LogMergePolicy, we'll need to expose readers from the IW reader pool. This is because presumably classes may need to access readers such as in LogMergePolicy.findMergesToExpungeDeletes.
        Hide
        Jason Rutherglen added a comment -

        After looking at it, I wasn't sure why we couldn't simply make
        the read only methods in SegmentInfo and SegmentInfos (and the
        classes) public.

        Maybe this can make it into 2.9?

        Show
        Jason Rutherglen added a comment - After looking at it, I wasn't sure why we couldn't simply make the read only methods in SegmentInfo and SegmentInfos (and the classes) public. Maybe this can make it into 2.9?
        Hide
        Michael McCandless added a comment -

        I agree it'd be good to do this, and this baby step (making the read-only methods public) seems like a good start.

        Would we want better names here (Segments/Segment), as Earwin suggested a while back?

        Can you also mark these classes as Expert and add the "subject to change w/o warning between releases" caveat?

        Show
        Michael McCandless added a comment - I agree it'd be good to do this, and this baby step (making the read-only methods public) seems like a good start. Would we want better names here (Segments/Segment), as Earwin suggested a while back? Can you also mark these classes as Expert and add the "subject to change w/o warning between releases" caveat?
        Hide
        Jason Rutherglen added a comment -
        • Added some more javadocs (as suggested). More could be added
          to the SegmentInfo and SegmentInfos methods.
        • Made SegmentReader public. I think we need to do this as it's
          becoming more necessary after LUCENE-1483 where the user may
          access individual readers?

        Would we want better names here (Segments/Segment), as
        Earwin suggested a while back?

        Wouldn't this require a bunch of renaming/refactoring? Earwin
        what was your suggestion? (couldn't find it)

        Show
        Jason Rutherglen added a comment - Added some more javadocs (as suggested). More could be added to the SegmentInfo and SegmentInfos methods. Made SegmentReader public. I think we need to do this as it's becoming more necessary after LUCENE-1483 where the user may access individual readers? Would we want better names here (Segments/Segment), as Earwin suggested a while back? Wouldn't this require a bunch of renaming/refactoring? Earwin what was your suggestion? (couldn't find it)
        Hide
        Michael McCandless added a comment -

        I don't think we should make IndexWriter's ReaderPool public just yet? Maybe instead we can add API to query for whether a segment has pending unflushed deletes? (And fix core merge policies to use that API when deciding how to expungeDeletes).

        Show
        Michael McCandless added a comment - I don't think we should make IndexWriter's ReaderPool public just yet? Maybe instead we can add API to query for whether a segment has pending unflushed deletes? (And fix core merge policies to use that API when deciding how to expungeDeletes).
        Hide
        Jason Rutherglen added a comment -
        • Reader pool isn't public anymore
        • Left methods of reader as public (could roll back?)
        • I'd rather that readerpool be public, however since it's new I
          guess we don't want people relying on it?
        • All tests pass
        • It would be great to get this into 2.9
        Show
        Jason Rutherglen added a comment - Reader pool isn't public anymore Left methods of reader as public (could roll back?) I'd rather that readerpool be public, however since it's new I guess we don't want people relying on it? All tests pass It would be great to get this into 2.9
        Hide
        Michael McCandless added a comment -

        Latest patch looks good Jason! I'll commit soon.

        since it's new I guess we don't want people relying on it?

        Right, and, we haven't thought about / tested for users randomly checking out readers from a writer at different times, it'd make me nervous to expose that now.

        Show
        Michael McCandless added a comment - Latest patch looks good Jason! I'll commit soon. since it's new I guess we don't want people relying on it? Right, and, we haven't thought about / tested for users randomly checking out readers from a writer at different times, it'd make me nervous to expose that now.
        Hide
        Jason Rutherglen added a comment -

        LogMergePolicy needs to access the pooled reader's num deletes. I'll add it, remove IW.hasDeletes, and offer IW.numDeletedDocs(SegmentInfo)

        Show
        Jason Rutherglen added a comment - LogMergePolicy needs to access the pooled reader's num deletes. I'll add it, remove IW.hasDeletes, and offer IW.numDeletedDocs(SegmentInfo)
        Hide
        Jason Rutherglen added a comment -
        • Added IW.numDeletedDocs method which LogMergePolicy uses to obtain the delCount of an info
        • Updated to trunk
        Show
        Jason Rutherglen added a comment - Added IW.numDeletedDocs method which LogMergePolicy uses to obtain the delCount of an info Updated to trunk
        Hide
        Michael McCandless added a comment -

        Attached patch with tiny changes: made a few more read-only methods public, fixed javadoc warning, one formatting fix, added CHANGES.

        I think it's ready to commit. I'll commit soon...

        Show
        Michael McCandless added a comment - Attached patch with tiny changes: made a few more read-only methods public, fixed javadoc warning, one formatting fix, added CHANGES. I think it's ready to commit. I'll commit soon...
        Hide
        Michael McCandless added a comment -

        Thanks Jason!

        Show
        Michael McCandless added a comment - Thanks Jason!

          People

          • Assignee:
            Unassigned
            Reporter:
            Jason Rutherglen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 48h
              48h
              Remaining:
              Remaining Estimate - 48h
              48h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development