Skip to content

allow up to 4GB per bitmap index #8796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
siddharthteotia merged 1 commit into from
Jun 1, 2022
Merged

allow up to 4GB per bitmap index #8796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions ...va/org/apache/pinot/segment/local/segment/creator/impl/inv/BitmapInvertedIndexWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.pinot.segment.local.segment.creator.impl.inv;

import com.google.common.base.Preconditions;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -55,7 +56,7 @@ public final class BitmapInvertedIndexWriter implements Closeable {
private final FileChannel _fileChannel;
private final ByteBuffer _offsetBuffer;
private ByteBuffer _bitmapBuffer;
private int _bytesWritten;
private long _bytesWritten;

public BitmapInvertedIndexWriter(File outputFile, int numBitmaps)
throws IOException {
Expand All @@ -71,7 +72,7 @@ public void add(RoaringBitmap bitmap)
throws IOException {
int length = bitmap.serializedSizeInBytes();
resizeIfNecessary(length);
_offsetBuffer.putInt(_bytesWritten);
_offsetBuffer.putInt(asUnsignedInt(_bytesWritten));
bitmap.serialize(_bitmapBuffer);
_bytesWritten += length;
}
Expand All @@ -84,7 +85,7 @@ public void add(byte[] bitmapBytes)
public void add(byte[] bitmapBytes, int length)
throws IOException {
resizeIfNecessary(length);
_offsetBuffer.putInt(_bytesWritten);
_offsetBuffer.putInt(asUnsignedInt(_bytesWritten));
_bitmapBuffer.put(bitmapBytes, 0, length);
_bytesWritten += length;
}
Expand Down Expand Up @@ -113,8 +114,8 @@ private void cleanBitmapBuffer()
@Override
public void close()
throws IOException {
int fileLength = _bytesWritten;
_offsetBuffer.putInt(fileLength);
long fileLength = _bytesWritten;
_offsetBuffer.putInt(asUnsignedInt(fileLength));
_fileChannel.truncate(fileLength);
_fileChannel.close();
if (CleanerUtil.UNMAP_SUPPORTED) {
Expand All @@ -123,4 +124,9 @@ public void close()
cleanBitmapBuffer();
}
}

private int asUnsignedInt(long value) {
Preconditions.checkArgument(value >>> 32 == 0, "overflowed 4GB");
return (int) (value & 0xFFFFFFFFL);
}
}
15 changes: 9 additions & 6 deletions .../java/org/apache/pinot/segment/local/segment/index/readers/BitmapInvertedIndexReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,25 @@ public class BitmapInvertedIndexReader implements InvertedIndexReader<ImmutableR
// Use the offset of the first bitmap to support 2 different format of the inverted index:
// 1. Offset buffer stores the offsets within the whole data buffer (including offset buffer)
// 2. Offset buffer stores the offsets within the bitmap buffer
private final int _firstOffset;
private final long _firstOffset;

public BitmapInvertedIndexReader(PinotDataBuffer dataBuffer, int numBitmaps) {
long offsetBufferEndOffset = (long) (numBitmaps + 1) * Integer.BYTES;
_offsetBuffer = dataBuffer.view(0, offsetBufferEndOffset, ByteOrder.BIG_ENDIAN);
_bitmapBuffer = dataBuffer.view(offsetBufferEndOffset, dataBuffer.size());

_firstOffset = _offsetBuffer.getInt(0);
_firstOffset = getOffset(0);
}

@SuppressWarnings("unchecked")
@Override
public ImmutableRoaringBitmap getDocIds(int dictId) {
int offset = _offsetBuffer.getInt(dictId * Integer.BYTES);
int length = _offsetBuffer.getInt((dictId + 1) * Integer.BYTES) - offset;
return new ImmutableRoaringBitmap(_bitmapBuffer.toDirectByteBuffer(offset - _firstOffset, length));
long offset = getOffset(dictId);
long length = getOffset(dictId + 1) - offset;
return new ImmutableRoaringBitmap(_bitmapBuffer.toDirectByteBuffer(offset - _firstOffset, (int) length));
Copy link
Member Author

@richardstartin richardstartin May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the maximum size of a RoaringBitmap is 2^16 * (8KB + 2B) ~ 512MB so the cast to int never overflows.

Copy link
Member

@jackjlli jackjlli May 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to add this comments of analysis to the code above as well?

}

private long getOffset(int dictId) {
return _offsetBuffer.getInt(dictId * Integer.BYTES) & 0xFFFFFFFFL;
}

@Override
Expand Down