Lucene - Core
  1. Lucene - Core
  2. LUCENE-2633

PackedInts does not support structures above 256MB

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Environment:

      All

    • Lucene Fields:
      New, Patch Available

      Description

      The PackedInts Packed32 and Packed64 fails when the internal structure exceeds 256MB. This is due to a missing cast that results in the bit position calculation being limited by Integer.MAX_VALUE (256MB * 8 = 2GB).

      1. LUCENE-2633.patch
        4 kB
        Toke Eskildsen

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          Bulk close after release of 3.5

          Show
          Uwe Schindler added a comment - Bulk close after release of 3.5
          Hide
          Michael McCandless added a comment -

          Thanks Toke!

          Show
          Michael McCandless added a comment - Thanks Toke!
          Hide
          Michael McCandless added a comment -

          Runs fine for me too – the test 'only' needs 256 MB so I think it's fine.

          Show
          Michael McCandless added a comment - Runs fine for me too – the test 'only' needs 256 MB so I think it's fine.
          Hide
          Robert Muir added a comment -

          I have no problems running 'ant test' with this patch/test applied.

          as mentioned on LUCENE-1990, ant allows 512MB in our build already.

          i mentioned there that it would be nice to reduce this to say 256MB,
          but its not causing me any problems personally and computers get more
          RAM every day.

          I think we should commit with the test enabled.

          Show
          Robert Muir added a comment - I have no problems running 'ant test' with this patch/test applied. as mentioned on LUCENE-1990 , ant allows 512MB in our build already. i mentioned there that it would be nice to reduce this to say 256MB, but its not causing me any problems personally and computers get more RAM every day. I think we should commit with the test enabled.
          Hide
          Simon Willnauer added a comment -

          +1, if the test is problematic, we can always either comment out the new test, or @Ignore it, and add a note.

          +1 - I thought about something like the Assume#assumeTrue(MAX_HEAP_MEM >= 256); (http://kentbeck.github.com/junit/javadoc/latest/org/junit/Assume.html#assumeTrue(boolean)) which could be set to a certain default value and would exclude the test if not enough memory is available. That would prevent uncommenting the test - I guess @Ignore would work too but maybe we want disable / enable at a central place for tests like that.

          simon

          Show
          Simon Willnauer added a comment - +1, if the test is problematic, we can always either comment out the new test, or @Ignore it, and add a note. +1 - I thought about something like the Assume#assumeTrue(MAX_HEAP_MEM >= 256); ( http://kentbeck.github.com/junit/javadoc/latest/org/junit/Assume.html#assumeTrue(boolean )) which could be set to a certain default value and would exclude the test if not enough memory is available. That would prevent uncommenting the test - I guess @Ignore would work too but maybe we want disable / enable at a central place for tests like that. simon
          Hide
          Robert Muir added a comment -

          +1, if the test is problematic, we can always either comment out the new test, or @Ignore it, and add a note.

          Show
          Robert Muir added a comment - +1, if the test is problematic, we can always either comment out the new test, or @Ignore it, and add a note.
          Hide
          Mark Miller added a comment -

          Any objections to at least committing the fix (the casts are clearly needed) while we wait and sort out the test? Appears to be a clear and simple bug, but the test issue is not so clear and simple and looks like it could drag out the fix being applied here - but that doesn't seem necessary?

          Show
          Mark Miller added a comment - Any objections to at least committing the fix (the casts are clearly needed) while we wait and sort out the test? Appears to be a clear and simple bug, but the test issue is not so clear and simple and looks like it could drag out the fix being applied here - but that doesn't seem necessary?
          Hide
          Toke Eskildsen added a comment -

          See LUCENE-1990 for discussion of this fix.

          Show
          Toke Eskildsen added a comment - See LUCENE-1990 for discussion of this fix.
          Hide
          Toke Eskildsen added a comment -

          Bug fixed for Packed32 and Packe64. Unit test added that triggers the bug in the non-patched versions.

          Show
          Toke Eskildsen added a comment - Bug fixed for Packed32 and Packe64. Unit test added that triggers the bug in the non-patched versions.

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Toke Eskildsen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development