Lucene - Core
  1. Lucene - Core
  2. LUCENE-2693

Add delete term and query need to more precisely record the bytes used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9.4, 3.1, 4.0-ALPHA
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      DocumentsWriter's add delete query and add delete term add to the number of bytes used regardless of the query or term already existing in the respective map.

      1. LUCENE-2693.patch
        1 kB
        Shai Erera
      2. LUCENE-2693.patch
        1 kB
        Jason Rutherglen

        Activity

        Hide
        Jason Rutherglen added a comment -

        Patch. The query value was changed to BufferedDeletes.Num instead of Integer to save a little on object pointer allocation.

        As a side note, there are a number of unrelated generics warnings when compiling.

        Show
        Jason Rutherglen added a comment - Patch. The query value was changed to BufferedDeletes.Num instead of Integer to save a little on object pointer allocation. As a side note, there are a number of unrelated generics warnings when compiling.
        Hide
        Robert Muir added a comment -

        moving out

        Show
        Robert Muir added a comment - moving out
        Hide
        Shai Erera added a comment -

        Jason, are you going to fix it for 3.1, or shall I mark it 3.2?

        Show
        Shai Erera added a comment - Jason, are you going to fix it for 3.1, or shall I mark it 3.2?
        Hide
        Jason Rutherglen added a comment -

        Do you mean post a patch for 3.1? The change is fairly simple, can you simply add it and commit?

        Show
        Jason Rutherglen added a comment - Do you mean post a patch for 3.1? The change is fairly simple, can you simply add it and commit?
        Hide
        Shai Erera added a comment -

        Yeah I can add it. I didn't realize you're waiting for someone to take on that issue .

        Show
        Shai Erera added a comment - Yeah I can add it. I didn't realize you're waiting for someone to take on that issue .
        Hide
        Shai Erera added a comment -

        I reviewed the patch and it no longer applies to DocumentsWriter anymore, since deletes happen inside SegmentDeletes. So:

        1. delete-by-term: am I right to say it's handled properly in SegmentDeletes.addTerm()?
        2. delete-by-query: seems like SegmentDeletes.addQuery() always increments bytesUsed, whether the query was already in the map or not. I think that's wrong, so I'll add a patch that fixes it for you to review.
        Show
        Shai Erera added a comment - I reviewed the patch and it no longer applies to DocumentsWriter anymore, since deletes happen inside SegmentDeletes. So: delete-by-term: am I right to say it's handled properly in SegmentDeletes.addTerm()? delete-by-query: seems like SegmentDeletes.addQuery() always increments bytesUsed, whether the query was already in the map or not. I think that's wrong, so I'll add a patch that fixes it for you to review.
        Hide
        Shai Erera added a comment -

        Patch against 3x, though I believe it'll look the same against trunk. Fixed SegmentDeletes.addQuery and added a CHANGES entry. I'd appreciate if someone can take a look before I commit.

        Show
        Shai Erera added a comment - Patch against 3x, though I believe it'll look the same against trunk. Fixed SegmentDeletes.addQuery and added a CHANGES entry. I'd appreciate if someone can take a look before I commit.
        Hide
        Michael McCandless added a comment -

        New patch looks good Shai! We had already fixed the dup-Term case...

        Show
        Michael McCandless added a comment - New patch looks good Shai! We had already fixed the dup-Term case...
        Hide
        Shai Erera added a comment -

        Committed revision 1062824 (3x).
        Committed revision 1062832 (trunk).

        Thanks Jason !

        Show
        Shai Erera added a comment - Committed revision 1062824 (3x). Committed revision 1062832 (trunk). Thanks Jason !
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development