You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2023/03/02 15:49:00 UTC

[GitHub] [pinot] gortiz opened a new pull request, #10370: Change bitmap inverted index to be able to be embedded in a buffer.

gortiz opened a new pull request, #10370:
URL: https://github.com/apache/pinot/pull/10370

   Current inverted implementation assumes that the index will own the channel where it is being written. At the same time the reader assumes that the index starts in the position 0 of the given buffer.
   
   This PR changes that in order to be able to embed an inverted index in a more complex structure, something we would like to do in StarTree to implement proprietary features.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155494276


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java:
##########
@@ -83,13 +86,26 @@ public final class OffHeapBitmapInvertedIndexCreator implements DictionaryBasedI
   public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
       int numValues)
       throws IOException {
-    String columnName = fieldSpec.getName();
-    _invertedIndexFile = new File(indexDir, columnName + V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
-    _forwardIndexValueBufferFile = new File(indexDir, columnName + FORWARD_INDEX_VALUE_BUFFER_SUFFIX);
-    _forwardIndexLengthBufferFile = new File(indexDir, columnName + FORWARD_INDEX_LENGTH_BUFFER_SUFFIX);
-    _invertedIndexValueBufferFile = new File(indexDir, columnName + INVERTED_INDEX_VALUE_BUFFER_SUFFIX);
-    _invertedIndexLengthBufferFile = new File(indexDir, columnName + INVERTED_INDEX_LENGTH_BUFFER_SUFFIX);
-    _singleValue = fieldSpec.isSingleValueField();
+    this(indexDir, fieldSpec, cardinality, numDocs, numValues,
+        V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
+      int numValues, String extension)
+      throws IOException {
+    this(indexDir, fieldSpec.getName(), fieldSpec.isSingleValueField(), cardinality, numDocs, numValues, extension);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, String columnName, boolean singleValue, int cardinality,
+      int numDocs, int numValues, String extension)
+      throws IOException {
+    String ext = extension.equals(V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION) ? "" : "." + extension;
+    _invertedIndexFile = getDefaultFile(indexDir, columnName, extension);
+    _forwardIndexValueBufferFile = new File(indexDir, columnName + ext + FORWARD_INDEX_VALUE_BUFFER_SUFFIX);
+    _forwardIndexLengthBufferFile = new File(indexDir, columnName + ext + FORWARD_INDEX_LENGTH_BUFFER_SUFFIX);
+    _invertedIndexValueBufferFile = new File(indexDir, columnName + ext + INVERTED_INDEX_VALUE_BUFFER_SUFFIX);
+    _invertedIndexLengthBufferFile = new File(indexDir, columnName + ext + INVERTED_INDEX_LENGTH_BUFFER_SUFFIX);

Review Comment:
   Can these also use getDefaultFile?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] codecov-commenter commented on pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10370:
URL: https://github.com/apache/pinot/pull/10370#issuecomment-1452168794

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#10370](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cec106f) into [master](https://codecov.io/gh/apache/pinot/commit/9c7e771d22a0cbd3391593e9976669fff4d4b442?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9c7e771) will **increase** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10370      +/-   ##
   ============================================
   + Coverage     13.76%   13.79%   +0.02%     
     Complexity      231      231              
   ============================================
     Files          1979     1979              
     Lines        107729   107762      +33     
     Branches      16457    16460       +3     
   ============================================
   + Hits          14831    14866      +35     
   + Misses        91717    91710       -7     
   - Partials       1181     1186       +5     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests2 | `13.79% <0.00%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...nt/creator/impl/inv/BitmapInvertedIndexWriter.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvQml0bWFwSW52ZXJ0ZWRJbmRleFdyaXRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...or/impl/inv/OffHeapBitmapInvertedIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvT2ZmSGVhcEJpdG1hcEludmVydGVkSW5kZXhDcmVhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...gment/index/readers/BitmapInvertedIndexReader.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0bWFwSW52ZXJ0ZWRJbmRleFJlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.34% <0.00%> (-0.18%)` | :arrow_down: |
   | [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `39.83% <0.00%> (+0.82%)` | :arrow_up: |
   | [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `39.01% <0.00%> (+1.73%)` | :arrow_up: |
   | [...elix/core/periodictask/ControllerPeriodicTask.java](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3BlcmlvZGljdGFzay9Db250cm9sbGVyUGVyaW9kaWNUYXNrLmphdmE=) | `67.85% <0.00%> (+1.78%)` | :arrow_up: |
   | ... and [3 more](https://codecov.io/gh/apache/pinot/pull/10370?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155493898


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java:
##########
@@ -83,13 +86,26 @@ public final class OffHeapBitmapInvertedIndexCreator implements DictionaryBasedI
   public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
       int numValues)
       throws IOException {
-    String columnName = fieldSpec.getName();
-    _invertedIndexFile = new File(indexDir, columnName + V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
-    _forwardIndexValueBufferFile = new File(indexDir, columnName + FORWARD_INDEX_VALUE_BUFFER_SUFFIX);
-    _forwardIndexLengthBufferFile = new File(indexDir, columnName + FORWARD_INDEX_LENGTH_BUFFER_SUFFIX);
-    _invertedIndexValueBufferFile = new File(indexDir, columnName + INVERTED_INDEX_VALUE_BUFFER_SUFFIX);
-    _invertedIndexLengthBufferFile = new File(indexDir, columnName + INVERTED_INDEX_LENGTH_BUFFER_SUFFIX);
-    _singleValue = fieldSpec.isSingleValueField();
+    this(indexDir, fieldSpec, cardinality, numDocs, numValues,
+        V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
+      int numValues, String extension)
+      throws IOException {
+    this(indexDir, fieldSpec.getName(), fieldSpec.isSingleValueField(), cardinality, numDocs, numValues, extension);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, String columnName, boolean singleValue, int cardinality,
+      int numDocs, int numValues, String extension)
+      throws IOException {
+    String ext = extension.equals(V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION) ? "" : "." + extension;

Review Comment:
   Would this is simpler if ext was Optional<String>, and 
   `_invertedIndexFile 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155731525


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/BitmapInvertedIndexWriter.java:
##########
@@ -57,14 +57,33 @@ public final class BitmapInvertedIndexWriter implements Closeable {
   private final ByteBuffer _offsetBuffer;
   private ByteBuffer _bitmapBuffer;
   private long _bytesWritten;
+  private final boolean _ownsChannel;
 
   public BitmapInvertedIndexWriter(File outputFile, int numBitmaps)
       throws IOException {
+    this(new RandomAccessFile(outputFile, "rw").getChannel(), numBitmaps, true);
+  }
+
+  /**
+   * Creates a new writer that uses the given {@link FileChannel}.
+   * It will start to write on the current position of the channel assuming it is the last useful byte in the file.
+   * When this object is {@link #close() closed}, the channel is truncated to the last byte written by this writer.
+   * @param fileChannel the file channel to be used
+   * @param numBitmaps the number of bitmaps that are expected. The actual value cannot be higher than this value. Fewer
+   *                   bitmaps than the given value can be used, but in that case the representation will not be as
+   *                   expected.
+   * @param ownsChannel whether this writer owns the channel or not. If the channel is owned then it will be closed when
+   *                    this object is closed. Otherwise the owner will have to close it by itself. Even if this writer
+   *                    does not own the channel, it will be truncated when the writer is closed.
+   */
+  public BitmapInvertedIndexWriter(FileChannel fileChannel, int numBitmaps, boolean ownsChannel)
+      throws IOException {
+    _ownsChannel = ownsChannel;
     int sizeForOffsets = (numBitmaps + 1) * Integer.BYTES;
     long bitmapBufferEstimate = Math.min(PESSIMISTIC_BITMAP_SIZE_ESTIMATE * numBitmaps, MAX_INITIAL_BUFFER_SIZE);
-    _fileChannel = new RandomAccessFile(outputFile, "rw").getChannel();
-    _offsetBuffer = _fileChannel.map(FileChannel.MapMode.READ_WRITE, 0, sizeForOffsets);
-    _bytesWritten = sizeForOffsets;
+    _fileChannel = fileChannel;
+    _offsetBuffer = _fileChannel.map(FileChannel.MapMode.READ_WRITE, _fileChannel.position(), sizeForOffsets);
+    _bytesWritten = sizeForOffsets + _fileChannel.position();

Review Comment:
   changed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 merged pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 merged PR #10370:
URL: https://github.com/apache/pinot/pull/10370


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #10370:
URL: https://github.com/apache/pinot/pull/10370#issuecomment-1452090131

   @saurabhd336 @Jackie-Jiang please review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155493648


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java:
##########
@@ -83,13 +86,26 @@ public final class OffHeapBitmapInvertedIndexCreator implements DictionaryBasedI
   public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
       int numValues)
       throws IOException {
-    String columnName = fieldSpec.getName();
-    _invertedIndexFile = new File(indexDir, columnName + V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
-    _forwardIndexValueBufferFile = new File(indexDir, columnName + FORWARD_INDEX_VALUE_BUFFER_SUFFIX);
-    _forwardIndexLengthBufferFile = new File(indexDir, columnName + FORWARD_INDEX_LENGTH_BUFFER_SUFFIX);
-    _invertedIndexValueBufferFile = new File(indexDir, columnName + INVERTED_INDEX_VALUE_BUFFER_SUFFIX);
-    _invertedIndexLengthBufferFile = new File(indexDir, columnName + INVERTED_INDEX_LENGTH_BUFFER_SUFFIX);
-    _singleValue = fieldSpec.isSingleValueField();
+    this(indexDir, fieldSpec, cardinality, numDocs, numValues,
+        V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
+      int numValues, String extension)
+      throws IOException {
+    this(indexDir, fieldSpec.getName(), fieldSpec.isSingleValueField(), cardinality, numDocs, numValues, extension);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, String columnName, boolean singleValue, int cardinality,

Review Comment:
   Can we have a java doc for this constructor? Looks like custom file extensions are being allowed.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155527754


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java:
##########
@@ -83,13 +86,26 @@ public final class OffHeapBitmapInvertedIndexCreator implements DictionaryBasedI
   public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
       int numValues)
       throws IOException {
-    String columnName = fieldSpec.getName();
-    _invertedIndexFile = new File(indexDir, columnName + V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
-    _forwardIndexValueBufferFile = new File(indexDir, columnName + FORWARD_INDEX_VALUE_BUFFER_SUFFIX);
-    _forwardIndexLengthBufferFile = new File(indexDir, columnName + FORWARD_INDEX_LENGTH_BUFFER_SUFFIX);
-    _invertedIndexValueBufferFile = new File(indexDir, columnName + INVERTED_INDEX_VALUE_BUFFER_SUFFIX);
-    _invertedIndexLengthBufferFile = new File(indexDir, columnName + INVERTED_INDEX_LENGTH_BUFFER_SUFFIX);
-    _singleValue = fieldSpec.isSingleValueField();
+    this(indexDir, fieldSpec, cardinality, numDocs, numValues,
+        V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, FieldSpec fieldSpec, int cardinality, int numDocs,
+      int numValues, String extension)
+      throws IOException {
+    this(indexDir, fieldSpec.getName(), fieldSpec.isSingleValueField(), cardinality, numDocs, numValues, extension);
+  }
+
+  public OffHeapBitmapInvertedIndexCreator(File indexDir, String columnName, boolean singleValue, int cardinality,

Review Comment:
   I can add javadoc, but yes, this change let you create indexes in any file without extension constraints. That is part of the idea.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10370: Change bitmap inverted index to be able to be embedded in a buffer.

Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #10370:
URL: https://github.com/apache/pinot/pull/10370#discussion_r1155496024


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/BitmapInvertedIndexWriter.java:
##########
@@ -57,14 +57,33 @@ public final class BitmapInvertedIndexWriter implements Closeable {
   private final ByteBuffer _offsetBuffer;
   private ByteBuffer _bitmapBuffer;
   private long _bytesWritten;
+  private final boolean _ownsChannel;
 
   public BitmapInvertedIndexWriter(File outputFile, int numBitmaps)
       throws IOException {
+    this(new RandomAccessFile(outputFile, "rw").getChannel(), numBitmaps, true);
+  }
+
+  /**
+   * Creates a new writer that uses the given {@link FileChannel}.
+   * It will start to write on the current position of the channel assuming it is the last useful byte in the file.
+   * When this object is {@link #close() closed}, the channel is truncated to the last byte written by this writer.
+   * @param fileChannel the file channel to be used
+   * @param numBitmaps the number of bitmaps that are expected. The actual value cannot be higher than this value. Fewer
+   *                   bitmaps than the given value can be used, but in that case the representation will not be as
+   *                   expected.
+   * @param ownsChannel whether this writer owns the channel or not. If the channel is owned then it will be closed when
+   *                    this object is closed. Otherwise the owner will have to close it by itself. Even if this writer
+   *                    does not own the channel, it will be truncated when the writer is closed.
+   */
+  public BitmapInvertedIndexWriter(FileChannel fileChannel, int numBitmaps, boolean ownsChannel)
+      throws IOException {
+    _ownsChannel = ownsChannel;
     int sizeForOffsets = (numBitmaps + 1) * Integer.BYTES;
     long bitmapBufferEstimate = Math.min(PESSIMISTIC_BITMAP_SIZE_ESTIMATE * numBitmaps, MAX_INITIAL_BUFFER_SIZE);
-    _fileChannel = new RandomAccessFile(outputFile, "rw").getChannel();
-    _offsetBuffer = _fileChannel.map(FileChannel.MapMode.READ_WRITE, 0, sizeForOffsets);
-    _bytesWritten = sizeForOffsets;
+    _fileChannel = fileChannel;
+    _offsetBuffer = _fileChannel.map(FileChannel.MapMode.READ_WRITE, _fileChannel.position(), sizeForOffsets);
+    _bytesWritten = sizeForOffsets + _fileChannel.position();

Review Comment:
   Does this change the meaning of this variable? From _bytesWritten to _currentBufferPosition or something similar? Might warrant a change in variable name.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org