[GitHub] [lucene-solr] mikemccand commented on a change in pull request #2256: LUCENE-9507 Custom order for leaves in IndexReader and IndexWriter

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

mikemccand commented on a change in pull request #2256:
URL: https://github.com/apache/lucene-solr/pull/2256#discussion_r569643083



##########
File path: lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -143,12 +158,17 @@ static StandardDirectoryReader open(
 
       writer.incRefDeleter(segmentInfos);
 
+      if (writer.getConfig().getLeafSorter() != null) {
+        readers.sort(writer.getConfig().getLeafSorter());

Review comment:
       Is IWC.leafSorter allowed to be live-updated?  If so, maybe we should pull out local variable here to hold onto the `leafSorter` and use that variable here and below, to ensure it is the same?

##########
File path: lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -169,7 +189,10 @@ static StandardDirectoryReader open(
    * @lucene.internal
    */
   public static DirectoryReader open(
-      Directory directory, SegmentInfos infos, List<? extends LeafReader> oldReaders)
+      Directory directory,

Review comment:
       Are we handling the near-real-time reader (`IndexWriter.getReader(...)`) path here too?

##########
File path: lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -39,33 +40,44 @@
 
   final IndexWriter writer;
   final SegmentInfos segmentInfos;
+  private final Comparator<LeafReader> leafSorter;
   private final boolean applyAllDeletes;
   private final boolean writeAllDeletes;
 
-  /** called only from static open() methods */
+  /**
+   * package private constructor, called only from static open() methods. If parameter {@code
+   * leafSorter} is not {@code null}, the provided {@code readers} are expected to be already sorted

Review comment:
       Could we add an `assertLeavesSorted` method to confirm they really were properly sorted by the caller?  There are so many paths for creating `StandardDirectoryReader` through are code base that it would not surprise me if one of the misses to sort properly.




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