uschindler commented on code in PR #13535:
URL: https://github.com/apache/lucene/pull/13535#discussion_r1665828693


##########
lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java:
##########
@@ -578,7 +578,8 @@ public synchronized boolean writeFieldUpdates(
       // IndexWriter.commitMergedDeletes).
       final SegmentReader reader;
       if (this.reader == null) {
-        reader = new SegmentReader(info, indexCreatedVersionMajor, 
IOContext.READONCE);
+        IOContext context = info.info.getUseCompoundFile() ? IOContext.DEFAULT 
: IOContext.READONCE;

Review Comment:
   This looks correct with READONCE, as we close the segment reader down there, 
so it is only open for short time and we never look into it by different 
threads.
   
   So I think we can revert that change and use READONCE. It is still a bit 
expensive to mmap a huge CFS file  just to read a few bytes. 



##########
lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java:
##########
@@ -232,7 +232,7 @@ private FieldInfos readFieldInfos() throws IOException {
                 segInfo
                     .getCodec()
                     .compoundFormat()
-                    .getCompoundReader(segInfo.dir, segInfo, 
IOContext.READONCE);
+                    .getCompoundReader(segInfo.dir, segInfo, 
IOContext.DEFAULT);

Review Comment:
   READONCE is also fine here, because we only open the CFS file for very short 
time and close it down there. So as said before, this is a bit of overhead to 
mmap a large CFS file just to read fieldinfos.
   
   We should think about this a bit in a separate issue. It is good you catched 
it.



##########
lucene/core/src/java21/org/apache/lucene/store/MemorySegmentIndexInput.java:
##########
@@ -53,6 +53,7 @@ abstract class MemorySegmentIndexInput extends IndexInput
   final long length;
   final long chunkSizeMask;
   final int chunkSizePower;
+  final boolean confined;

Review Comment:
   We could also pass down the IOContext for more flexibility in the future.
   
   For now a single boolean is fine!



-- 
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: issues-unsubscr...@lucene.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to