[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10842: Support Off Heap for Native Text Indices

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

Jackie-Jiang commented on code in PR #10842:
URL: https://github.com/apache/pinot/pull/10842#discussion_r1218435393


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/NativeTextIndexReader.java:
##########
@@ -52,7 +52,10 @@ public class NativeTextIndexReader implements TextIndexReader {
   public NativeTextIndexReader(String column, File indexDir) {
     _column = column;
     try {
-      _buffer = PinotDataBuffer.loadBigEndianFile(getTextIndexFile(indexDir));
+      String desc = "Native text index buffer: " + column;

Review Comment:
   Both ways load the index off-heap. The difference is that in the original code it is loaded into direct memory, in the new code it is memory mapped. Ideally we should choose how to load the file based on the `ReadMode` (`heap` actually means direct memory), but since that cannot be easily passed to here, we may add a TODO here to address it later.
   
   Looking at the code, I found the `_buffer` is not closed (not introduced in this PR), which can cause memory leak.



-- 
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