[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1222: LUCENE-9414 EagerCheapMergePolicy

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

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



##########
File path: lucene/core/src/java/org/apache/lucene/index/EagerCheapMergePolicy.java
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
+
+import org.apache.lucene.util.InfoStream;
+
+/**
+ * Eagerly proposes cheap merges on a commit. The goal is to help keep the segment count low where
+ * we can do so cheaply, particularly on a commit so that the IndexSearcher has fewer segments to
+ * search over.
+ */
+public class EagerCheapMergePolicy extends TieredMergePolicy {
+  // Extends TieredMergePolicy for convenience but we could adjust to delegate or modify TieredMergePolicy
+
+  private static final String INFO_STREAM_CATEGORY = "CHEAP_MP";
+
+  private long cheapMergeThresholdBytes = 2 * 1024 * 1024; // 2MB
+  private int cheapMinMergeAtOnce = 3;
+  private double cheapMaxSizeRatio = 1.0;
+  private int cheapLimitConcurrentMergeSegments = 0; // thus only when no merges in-progress
+
+  /** Sole constructor. */
+  public EagerCheapMergePolicy() {
+  }
+
+  /** Returns {@link #setCheapMergeThresholdMB(double)}. */
+  public long getCheapMergeThresholdBytes() {
+    return cheapMergeThresholdBytes;
+  }
+
+  /** Limit cheap merges to those less than this size in megabytes. */
+  public EagerCheapMergePolicy setCheapMergeThresholdMB(double v) {
+    v *= 1024 * 1024;
+    cheapMergeThresholdBytes = (v > Long.MAX_VALUE) ? Long.MAX_VALUE : (long) v;
+    return this;
+  }
+
+  /** Returns {@link #setCheapMinMergeAtOnce(int)}. */
+  public int getCheapMinMergeAtOnce() {
+    return cheapMinMergeAtOnce;
+  }
+
+  /** Must have at least this many cheap segments to suggest merging them. */
+  public EagerCheapMergePolicy setCheapMinMergeAtOnce(int cheapMinMergeAtOnce) {
+    this.cheapMinMergeAtOnce = cheapMinMergeAtOnce;
+    return this;
+  }
+
+  /** Returns {@link #setCheapMaxSizeRatio(double)}. */
+  public double getCheapMaxSizeRatio() {
+    return cheapMaxSizeRatio;
+  }
+
+  /**
+   * Limit the maximum ratio of size between a candidate segment (that is deemed cheap) and the sum
+   * of all smaller segments. Example: If it's 2.0, a 20KB segment will merge with a 10KB
+   * segment (or say two 5KB segments) but will not with a 9KB segment.
+   * The limit should be tuned if {@link #setCheapMinMergeAtOnce(int)} is changed. It should be more
+   * than 1/(min-1).
+   */
+  public EagerCheapMergePolicy setCheapMaxSizeRatio(double cheapMaxSizeRatio) {
+    if (cheapMaxSizeRatio <= 0.0) {
+      throw new IllegalArgumentException("must be > 0");
+    }
+    this.cheapMaxSizeRatio = cheapMaxSizeRatio;
+    return this;
+  }
+
+  /** Returns {@link #setCheapLimitConcurrentMergeSegments(int)}. */
+  public int getCheapLimitConcurrentMergeSegments() {
+    return cheapLimitConcurrentMergeSegments;
+  }
+
+  /**
+   * Only find a cheap merge when the number of existing merging segments is less than or equal to
+   * this number.  In all likelihood, those segments are being merged concurrently. By default this
+   * setting is 0, thus won't find cheap merges during concurrent merging. If a higher number is
+   * used (e.g. 10) then there is a likely chance {@link org.apache.lucene.index.ConcurrentMergeScheduler}
+   * will intentionally stall the current thread if it's thresholds have been reached.  This may be
+   * okay if there are multiple merge threads (thus less likely to reach such thresholds) or if it's
+   * deemed more important to do cheap merges at the expense of slowing index throughput further.
+   */
+  public EagerCheapMergePolicy setCheapLimitConcurrentMergeSegments(int cheapLimitConcurrentMergeSegments) {

Review comment:
       Well, dangerous because it leads to merge starvation.
   
   Those tiny/fast merges really need to run at all times.  If you stop them because a huge merge is running it can lead to an explosion of small segments in the index, GC problems, running out of file descriptors, etc.




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