You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by si...@apache.org on 2020/04/22 19:26:58 UTC

[lucene-solr] branch master updated: LUCENE-9339: Only call MergeScheduler when we actually found new merges (#1445)

This is an automated email from the ASF dual-hosted git repository.

simonw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new 4a98918  LUCENE-9339: Only call MergeScheduler when we actually found new merges (#1445)
4a98918 is described below

commit 4a98918bfa2d57d35ac12c5d49e8f04fcd9791ae
Author: Simon Willnauer <si...@apache.org>
AuthorDate: Wed Apr 22 21:26:45 2020 +0200

    LUCENE-9339: Only call MergeScheduler when we actually found new merges (#1445)
    
    IW#maybeMerge calls the MergeScheduler even if it didn't find any merges we should instead only do this if there is in-fact anything there to merge and safe the call into a sync'd method.
---
 lucene/CHANGES.txt                                          |  3 +++
 .../org/apache/lucene/index/ConcurrentMergeScheduler.java   |  4 ++--
 .../core/src/java/org/apache/lucene/index/IndexWriter.java  | 13 +++++++------
 .../src/java/org/apache/lucene/index/MergeScheduler.java    |  6 ++----
 .../src/java/org/apache/lucene/index/NoMergeScheduler.java  |  2 +-
 .../java/org/apache/lucene/index/SerialMergeScheduler.java  |  2 +-
 .../test/org/apache/lucene/TestMergeSchedulerExternal.java  |  2 +-
 .../apache/lucene/index/TestConcurrentMergeScheduler.java   |  6 ++++--
 .../org/apache/lucene/index/TestIndexWriterMerging.java     |  2 +-
 .../test/org/apache/lucene/index/TestNoMergeScheduler.java  |  2 +-
 .../org/apache/lucene/index/BaseMergePolicyTestCase.java    |  4 ++--
 11 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index a9301eb..02b93f6 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -126,6 +126,9 @@ API Changes
   The DocumentsWriterPerThreadPool is a packaged protected final class which made it impossible
   to customize. (Simon Willnauer)
 
+* LUCENE-9339: MergeScheduler#merge doesn't accept a parameter if a new merge was found anymore.
+  (Simon Willnauer)
+
 New Features
 ---------------------
 (No changes)
diff --git a/lucene/core/src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java b/lucene/core/src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java
index 0324cd3..4eb8a65 100644
--- a/lucene/core/src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java
+++ b/lucene/core/src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java
@@ -495,7 +495,7 @@ public class ConcurrentMergeScheduler extends MergeScheduler {
   }
 
   @Override
-  public synchronized void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException {
+  public synchronized void merge(IndexWriter writer, MergeTrigger trigger) throws IOException {
 
     assert !Thread.holdsLock(writer);
 
@@ -641,7 +641,7 @@ public class ConcurrentMergeScheduler extends MergeScheduler {
     assert mergeThreads.contains(Thread.currentThread()) : "caller is not a merge thread";
     // Let CMS run new merges if necessary:
     try {
-      merge(writer, MergeTrigger.MERGE_FINISHED, true);
+      merge(writer, MergeTrigger.MERGE_FINISHED);
     } catch (AlreadyClosedException ace) {
       // OK
     } catch (IOException ioe) {
diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
index d7cedda..78bed20 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
@@ -2070,7 +2070,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
       }
     }
 
-    mergeScheduler.merge(this, MergeTrigger.EXPLICIT, newMergesFound);
+    mergeScheduler.merge(this, MergeTrigger.EXPLICIT);
 
     if (spec != null && doWait) {
       final int numMerges = spec.merges.size();
@@ -2152,8 +2152,9 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
 
   final void maybeMerge(MergePolicy mergePolicy, MergeTrigger trigger, int maxNumSegments) throws IOException {
     ensureOpen(false);
-    boolean newMergesFound = updatePendingMerges(mergePolicy, trigger, maxNumSegments);
-    mergeScheduler.merge(this, trigger, newMergesFound);
+    if (updatePendingMerges(mergePolicy, trigger, maxNumSegments)) {
+      mergeScheduler.merge(this, trigger);
+    }
   }
 
   private synchronized boolean updatePendingMerges(MergePolicy mergePolicy, MergeTrigger trigger, int maxNumSegments)
@@ -2534,7 +2535,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
     // Give merge scheduler last chance to run, in case
     // any pending merges are waiting. We can't hold IW's lock
     // when going into merge because it can lead to deadlock.
-    mergeScheduler.merge(this, MergeTrigger.CLOSING, false);
+    mergeScheduler.merge(this, MergeTrigger.CLOSING);
 
     synchronized (this) {
       ensureOpen(false);
@@ -3473,7 +3474,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
   }
 
   @SuppressWarnings("try")
-  private final void finishCommit() throws IOException {
+  private void finishCommit() throws IOException {
 
     boolean commitCompleted = false;
     String committedSegmentsFileName = null;
@@ -4381,7 +4382,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
     testPoint("mergeMiddleStart");
     merge.checkAborted();
 
-    Directory mergeDirectory = config.getMergeScheduler().wrapForMerge(merge, directory);
+    Directory mergeDirectory = mergeScheduler.wrapForMerge(merge, directory);
     List<SegmentCommitInfo> sourceSegments = merge.segments;
     
     IOContext context = new IOContext(merge.getStoreMergeInfo());
diff --git a/lucene/core/src/java/org/apache/lucene/index/MergeScheduler.java b/lucene/core/src/java/org/apache/lucene/index/MergeScheduler.java
index 66d0870..831b3e8 100644
--- a/lucene/core/src/java/org/apache/lucene/index/MergeScheduler.java
+++ b/lucene/core/src/java/org/apache/lucene/index/MergeScheduler.java
@@ -40,10 +40,8 @@ public abstract class MergeScheduler implements Closeable {
 
   /** Run the merges provided by {@link IndexWriter#getNextMerge()}.
    * @param writer the {@link IndexWriter} to obtain the merges from.
-   * @param trigger the {@link MergeTrigger} that caused this merge to happen
-   * @param newMergesFound <code>true</code> iff any new merges were found by the caller otherwise <code>false</code>
-   * */
-  public abstract void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException;
+   * @param trigger the {@link MergeTrigger} that caused this merge to happen */
+  public abstract void merge(IndexWriter writer, MergeTrigger trigger) throws IOException;
 
   /** 
    * Wraps the incoming {@link Directory} so that we can merge-throttle it
diff --git a/lucene/core/src/java/org/apache/lucene/index/NoMergeScheduler.java b/lucene/core/src/java/org/apache/lucene/index/NoMergeScheduler.java
index e4c0136..311f4fa 100644
--- a/lucene/core/src/java/org/apache/lucene/index/NoMergeScheduler.java
+++ b/lucene/core/src/java/org/apache/lucene/index/NoMergeScheduler.java
@@ -42,7 +42,7 @@ public final class NoMergeScheduler extends MergeScheduler {
   public void close() {}
 
   @Override
-  public void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) {}
+  public void merge(IndexWriter writer, MergeTrigger trigger) {}
   
   @Override
   public Directory wrapForMerge(OneMerge merge, Directory in) {
diff --git a/lucene/core/src/java/org/apache/lucene/index/SerialMergeScheduler.java b/lucene/core/src/java/org/apache/lucene/index/SerialMergeScheduler.java
index ce68247..74ec67a 100644
--- a/lucene/core/src/java/org/apache/lucene/index/SerialMergeScheduler.java
+++ b/lucene/core/src/java/org/apache/lucene/index/SerialMergeScheduler.java
@@ -31,7 +31,7 @@ public class SerialMergeScheduler extends MergeScheduler {
    * "synchronized" so that even if the application is using
    * multiple threads, only one merge may run at a time. */
   @Override
-  synchronized public void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException {
+  synchronized public void merge(IndexWriter writer, MergeTrigger trigger) throws IOException {
     while(true) {
       MergePolicy.OneMerge merge = writer.getNextMerge();
       if (merge == null) {
diff --git a/lucene/core/src/test/org/apache/lucene/TestMergeSchedulerExternal.java b/lucene/core/src/test/org/apache/lucene/TestMergeSchedulerExternal.java
index b689725..0534172 100644
--- a/lucene/core/src/test/org/apache/lucene/TestMergeSchedulerExternal.java
+++ b/lucene/core/src/test/org/apache/lucene/TestMergeSchedulerExternal.java
@@ -153,7 +153,7 @@ public class TestMergeSchedulerExternal extends LuceneTestCase {
   private static class ReportingMergeScheduler extends MergeScheduler {
 
     @Override
-    public void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException {
+    public void merge(IndexWriter writer, MergeTrigger trigger) throws IOException {
       OneMerge merge = null;
       while ((merge = writer.getNextMerge()) != null) {
         if (VERBOSE) {
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java b/lucene/core/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java
index f3a91ef..c93d638 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java
@@ -463,7 +463,8 @@ public class TestConcurrentMergeScheduler extends LuceneTestCase {
   public void testMaybeStallCalled() throws Exception {
     final AtomicBoolean wasCalled = new AtomicBoolean();
     Directory dir = newDirectory();
-    IndexWriterConfig iwc = newIndexWriterConfig(new MockAnalyzer(random()));
+    IndexWriterConfig iwc = newIndexWriterConfig(new MockAnalyzer(random()))
+        .setMergePolicy(new LogByteSizeMergePolicy());
     iwc.setMergeScheduler(new ConcurrentMergeScheduler() {
         @Override
         protected boolean maybeStall(IndexWriter writer) {
@@ -473,9 +474,10 @@ public class TestConcurrentMergeScheduler extends LuceneTestCase {
       });
     IndexWriter w = new IndexWriter(dir, iwc);
     w.addDocument(new Document());
+    w.flush();
+    w.addDocument(new Document());
     w.forceMerge(1);
     assertTrue(wasCalled.get());
-
     w.close();
     dir.close();
   }
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMerging.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMerging.java
index 9abd286..321bdf7 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMerging.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMerging.java
@@ -310,7 +310,7 @@ public class TestIndexWriterMerging extends LuceneTestCase {
   // merging a segment with >= 20 (maxMergeDocs) docs
   private static class MyMergeScheduler extends MergeScheduler {
     @Override
-    synchronized public void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException {
+    synchronized public void merge(IndexWriter writer, MergeTrigger trigger) throws IOException {
 
       while(true) {
         MergePolicy.OneMerge merge = writer.getNextMerge();
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestNoMergeScheduler.java b/lucene/core/src/test/org/apache/lucene/index/TestNoMergeScheduler.java
index 135d33e..7db1025 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestNoMergeScheduler.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestNoMergeScheduler.java
@@ -32,7 +32,7 @@ public class TestNoMergeScheduler extends LuceneTestCase {
   public void testNoMergeScheduler() throws Exception {
     MergeScheduler ms = NoMergeScheduler.INSTANCE;
     ms.close();
-    ms.merge(null, RandomPicks.randomFrom(random(), MergeTrigger.values()), random().nextBoolean());
+    ms.merge(null, RandomPicks.randomFrom(random(), MergeTrigger.values()));
   }
 
   @Test
diff --git a/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java
index 94a85df..bc07bf7 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java
@@ -70,7 +70,7 @@ public abstract class BaseMergePolicyTestCase extends LuceneTestCase {
       final AtomicBoolean mayMerge = new AtomicBoolean(true);
       final MergeScheduler mergeScheduler = new SerialMergeScheduler() {
           @Override
-          synchronized public void merge(IndexWriter writer, MergeTrigger trigger, boolean newMergesFound) throws IOException {
+          synchronized public void merge(IndexWriter writer, MergeTrigger trigger) throws IOException {
             if (mayMerge.get() == false) {
               MergePolicy.OneMerge merge = writer.getNextMerge();
               if (merge != null) {
@@ -79,7 +79,7 @@ public abstract class BaseMergePolicyTestCase extends LuceneTestCase {
               }
             }
 
-            super.merge(writer, trigger, newMergesFound);
+            super.merge(writer, trigger);
           }
         };