[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader

[GitBox <gi...@apache.org>]

dnhatn commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472332671



##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3179,9 +3317,9 @@ private long prepareCommitInternal() throws IOException {
       SegmentInfos toCommit = null;
       boolean anyChanges = false;
       long seqNo;
-      MergePolicy.MergeSpecification onCommitMerges = null;
-      AtomicBoolean includeInCommit = new AtomicBoolean(true);
-      final long maxCommitMergeWaitMillis = config.getMaxCommitMergeWaitMillis();
+      MergePolicy.MergeSpecification pointInTimeMerges = null;
+      AtomicBoolean hasTimedOut = new AtomicBoolean(false);

Review comment:
       Should we rename this to stopCollectingMergedReaders? I find that name better.

##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
     // obtained during this flush are pooled, the first time
     // this method is called:
     readerPool.enableReaderPooling();
-    DirectoryReader r = null;
+    StandardDirectoryReader r = null;
     doBeforeFlush();
-    boolean anyChanges = false;
+    boolean anyChanges;
     /*
      * for releasing a NRT reader we must ensure that 
      * DW doesn't add any segments or deletes until we are
      * done with creating the NRT DirectoryReader. 
      * We release the two stage full flush after we are done opening the
      * directory reader!
      */
+    MergePolicy.MergeSpecification onGetReaderMerges = null;
+    AtomicBoolean stopCollectingMergedReaders = new AtomicBoolean(false);
+    Map<String, SegmentReader> mergedReaders = new HashMap<>();
+    Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+    // this function is used to control which SR are opened in order to keep track of them
+    // and to reuse them in the case we wait for merges in this getReader call.
+    IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+      final ReadersAndUpdates rld = getPooledInstance(sci, true);
+      try {
+        assert Thread.holdsLock(IndexWriter.this);
+        SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+        openedReadOnlyClones.put(sci.info.name, segmentReader);

Review comment:
       Should we keep track the clones iff `maxFullFlushMergeWaitMillis` is positive? I know this is quite trivial.

##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
     // obtained during this flush are pooled, the first time
     // this method is called:
     readerPool.enableReaderPooling();
-    DirectoryReader r = null;
+    StandardDirectoryReader r = null;
     doBeforeFlush();
-    boolean anyChanges = false;
+    boolean anyChanges;
     /*
      * for releasing a NRT reader we must ensure that 
      * DW doesn't add any segments or deletes until we are
      * done with creating the NRT DirectoryReader. 
      * We release the two stage full flush after we are done opening the
      * directory reader!
      */
+    MergePolicy.MergeSpecification onGetReaderMerges = null;
+    AtomicBoolean stopCollectingMergedReaders = new AtomicBoolean(false);
+    Map<String, SegmentReader> mergedReaders = new HashMap<>();
+    Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+    // this function is used to control which SR are opened in order to keep track of them
+    // and to reuse them in the case we wait for merges in this getReader call.
+    IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+      final ReadersAndUpdates rld = getPooledInstance(sci, true);
+      try {
+        assert Thread.holdsLock(IndexWriter.this);
+        SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+        openedReadOnlyClones.put(sci.info.name, segmentReader);

Review comment:
       Should we keep track the clones iff `maxFullFlushMergeWaitMillis` is positive? I know this is not expensive.




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

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



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