jpountz commented on code in PR #266:
URL: https://github.com/apache/lucene/pull/266#discussion_r1850802831


##########
lucene/core/src/java/org/apache/lucene/index/TieredMergePolicy.java:
##########
@@ -495,36 +495,36 @@ private MergeSpecification doFindMerges(
       for (int startIdx = 0; startIdx < sortedEligible.size(); startIdx++) {
 
         long totAfterMergeBytes = 0;
-
         final List<SegmentCommitInfo> candidate = new ArrayList<>();
         boolean hitTooLarge = false;
-        long bytesThisMerge = 0;
         for (int idx = startIdx;
             idx < sortedEligible.size()
-                && candidate.size() < mergeFactor
-                && bytesThisMerge < maxMergedSegmentBytes;
+                && candidate.size() < maxMergeAtOnce
+                // We allow merging more that mergeFactor segments together if 
the merged segment
+                // would be less than the floor segment size. This is 
important because segments
+                // below the floor segment size are more aggressively merged 
by this policy, so we
+                // need to grow them as quickly as possible.
+                && (candidate.size() < mergeFactor || totAfterMergeBytes < 
floorSegmentBytes)
+                && totAfterMergeBytes < maxMergedSegmentBytes;
             idx++) {
           final SegmentSizeAndDocs segSizeDocs = sortedEligible.get(idx);
           final long segBytes = segSizeDocs.sizeInBytes;
 
           if (totAfterMergeBytes + segBytes > maxMergedSegmentBytes) {
             hitTooLarge = true;
-            if (candidate.size() == 0) {
-              // We should never have something coming in that _cannot_ be 
merged, so handle
-              // singleton merges
-              candidate.add(segSizeDocs.segInfo);
-              bytesThisMerge += segBytes;
+            // We should never have something coming in that _cannot_ be 
merged, so handle
+            // singleton merges
+            if (candidate.size() > 0) {
+              // NOTE: we continue, so that we can try
+              // "packing" smaller segments into this merge
+              // to see if we can get closer to the max
+              // size; this in general is not perfect since
+              // this is really "bin packing" and we'd have
+              // to try different permutations.
+              continue;
             }
-            // NOTE: we continue, so that we can try
-            // "packing" smaller segments into this merge
-            // to see if we can get closer to the max
-            // size; this in general is not perfect since
-            // this is really "bin packing" and we'd have
-            // to try different permutations.
-            continue;
           }
           candidate.add(segSizeDocs.segInfo);
-          bytesThisMerge += segBytes;

Review Comment:
   You are right, these were duplicate variables!



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