SOLR-1458: save optimized index points, fix deletion policy wrt number of optimized points to save, change policy to have a separate count for optimized

git-svn-id: https://svn.apache.org/repos/asf/lucene/solr/trunk@819336 13f79535-47bb-0310-9956-ffa450edef68

example/solr/conf/solrconfig.xml

@@ -146,10 +146,10 @@
         of the criteria.
     -->
     <deletionPolicy class="solr.SolrDeletionPolicy">
-      <!-- Keep only optimized commit points -->
+      <!-- The number of commit points to be kept -->
-      <str name="keepOptimizedOnly">false</str>
-      <!-- The maximum number of commit points to be kept -->
       <str name="maxCommitsToKeep">1</str>
+      <!-- The number of optimized commit points to be kept -->
+      <str name="maxOptimizedCommitsToKeep">0</str>
       <!--
           Delete all commit points once they have reached the given age.
           Supports DateMathParser syntax e.g.

src/java/org/apache/solr/core/SolrDeletionPolicy.java

@@ -44,20 +44,32 @@ import java.util.Locale;
 public class SolrDeletionPolicy implements IndexDeletionPolicy, NamedListInitializedPlugin {
   public static Logger log = LoggerFactory.getLogger(SolrCore.class);
 
-  private boolean keepOptimizedOnly = false;
   private String maxCommitAge = null;
   private int maxCommitsToKeep = 1;
+  private int maxOptimizedCommitsToKeep = 0;
 
   public void init(NamedList args) {
     String keepOptimizedOnlyString = (String) args.get("keepOptimizedOnly");
     String maxCommitsToKeepString = (String) args.get("maxCommitsToKeep");
+    String maxOptimizedCommitsToKeepString = (String) args.get("maxOptimizedCommitsToKeep");
     String maxCommitAgeString = (String) args.get("maxCommitAge");
-    if (keepOptimizedOnlyString != null && keepOptimizedOnlyString.trim().length() > 0)
+
-      keepOptimizedOnly = Boolean.parseBoolean(keepOptimizedOnlyString);
     if (maxCommitsToKeepString != null && maxCommitsToKeepString.trim().length() > 0)
       maxCommitsToKeep = Integer.parseInt(maxCommitsToKeepString);
     if (maxCommitAgeString != null && maxCommitAgeString.trim().length() > 0)
       maxCommitAge = "-" + maxCommitAgeString;
+    if (maxOptimizedCommitsToKeepString != null && maxOptimizedCommitsToKeepString.trim().length() > 0) {
+      maxOptimizedCommitsToKeep = Integer.parseInt(maxOptimizedCommitsToKeepString);
+    }
+    
+    // legacy support
+    if (keepOptimizedOnlyString != null && keepOptimizedOnlyString.trim().length() > 0) {
+      boolean keepOptimizedOnly = Boolean.parseBoolean(keepOptimizedOnlyString);
+      if (keepOptimizedOnly) {
+        maxOptimizedCommitsToKeep = Math.max(maxOptimizedCommitsToKeep, maxCommitsToKeep);
+        maxCommitsToKeep=0;
+      }
+    }
   }
 
   static String str(IndexCommit commit) {
@@ -117,43 +129,49 @@ public class SolrDeletionPolicy implements IndexDeletionPolicy, NamedListInitial
     // in this specific call (may be across diff IndexWriter instances).
     // this will happen rarely, so just synchronize everything
     // for safety and to avoid race conditions
-    DateMathParser dmp = new DateMathParser(DateField.UTC, Locale.US);
 
     synchronized (this) {
-      IndexCommit last = commits.get(commits.size() - 1);
+      long maxCommitAgeTimeStamp = -1L;
-      log.info("last commit = " + last.getVersion());
+      IndexCommit newest = commits.get(commits.size() - 1);
+      log.info("newest commit = " + newest.getVersion());
 
-      int numCommitsToDelete = commits.size() - maxCommitsToKeep;
+      int optimizedKept = newest.isOptimized() ? 1 : 0;
-      int i = 0;
+      int totalKept = 1;
-      for (IndexCommit commit : commits) {
-        // don't delete the last commit point
-        if (commit == last) {
-          continue;
-        }
 
-        if (i < numCommitsToDelete) {
+      // work our way from newest to oldest, skipping the first since we always want to keep it.
-          commit.delete();
+      for (int i=commits.size()-2; i>=0; i--) {
-          i++;
+        IndexCommit commit = commits.get(i);
-          continue;
-        }
 
+        // delete anything too old, regardless of other policies
         try {
-          if (maxCommitAge != null)
+          if (maxCommitAge != null) {
-            if (commit.getTimestamp() < dmp.parseMath(maxCommitAge).getTime()) {
+            if (maxCommitAgeTimeStamp==-1) {
+              DateMathParser dmp = new DateMathParser(DateField.UTC, Locale.US);
+              maxCommitAgeTimeStamp = dmp.parseMath(maxCommitAge).getTime();
+            }
+            if (commit.getTimestamp() < maxCommitAgeTimeStamp) {
               commit.delete();
               continue;
             }
+          }
         } catch (Exception e) {
           log.warn("Exception while checking commit point's age for deletion", e);
         }
 
-        if (keepOptimizedOnly) {
+        if (optimizedKept < maxOptimizedCommitsToKeep && commit.isOptimized()) {
-          if (!commit.isOptimized()) {
+          totalKept++;
-            commit.delete();
+          optimizedKept++;
-            log.info("Marking unoptimized index " + getId(commit) + " for deletion.");
+          continue;
-          }
         }
+
+        if (totalKept < maxCommitsToKeep) {
+          totalKept++;
+          continue;
+        }
+                                                  
+        commit.delete();
       }
+
     } // end synchronized
   }
 
@@ -178,10 +196,6 @@ public class SolrDeletionPolicy implements IndexDeletionPolicy, NamedListInitial
     return sb.toString();
   }
 
-  public boolean isKeepOptimizedOnly() {
-    return keepOptimizedOnly;
-  }
-
   public String getMaxCommitAge() {
     return maxCommitAge;
   }
@@ -189,4 +203,21 @@ public class SolrDeletionPolicy implements IndexDeletionPolicy, NamedListInitial
   public int getMaxCommitsToKeep() {
     return maxCommitsToKeep;
   }
+
+  public int getMaxOptimizedCommitsToKeep() {
+    return maxOptimizedCommitsToKeep;
+  }
+
+  public void setMaxCommitsToKeep(int maxCommitsToKeep) {
+    synchronized (this) {
+      this.maxCommitsToKeep = maxCommitsToKeep;
+    }
+  }
+
+  public void setMaxOptimizedCommitsToKeep(int maxOptimizedCommitsToKeep) {
+    synchronized (this) {
+      this.maxOptimizedCommitsToKeep = maxOptimizedCommitsToKeep;
+    }    
+  }
+
 }

src/java/org/apache/solr/handler/ReplicationHandler.java

@@ -17,16 +17,14 @@
 package org.apache.solr.handler;
 
 import org.apache.lucene.index.IndexCommit;
+import org.apache.lucene.index.IndexDeletionPolicy;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.FastOutputStream;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.core.CloseHook;
+import org.apache.solr.core.*;
-import org.apache.solr.core.IndexDeletionPolicyWrapper;
-import org.apache.solr.core.SolrCore;
-import org.apache.solr.core.SolrEventListener;
 import org.apache.solr.request.BinaryQueryResponseWriter;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrQueryResponse;
@@ -112,6 +110,13 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
    if (command.equals(CMD_INDEX_VERSION)) {
       IndexCommit commitPoint = indexCommitPoint;  // make a copy so it won't change
       if (commitPoint != null && replicationEnabled.get()) {
+        //
+        // There is a race condition here.  The commit point may be changed / deleted by the time
+        // we get around to reserving it.  This is a very small window though, and should not result
+        // in a catastrophic failure, but will result in the client getting an empty file list for
+        // the CMD_GET_FILE_LIST command.
+        //
+        core.getDeletionPolicy().setReserveDuration(commitPoint.getVersion(), reserveCommitDuration);
         rsp.add(CMD_INDEX_VERSION, commitPoint.getVersion());
         rsp.add(GENERATION, commitPoint.getGeneration());
       } else {
@@ -748,10 +753,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
 
   void refreshCommitpoint() {
     IndexCommit commitPoint = core.getDeletionPolicy().getLatestCommit();
-    if(replicateOnCommit && !commitPoint.isOptimized()){
+    if(replicateOnCommit || (replicateOnOptimize && commitPoint.isOptimized())) {
-      indexCommitPoint = commitPoint;
-    }
-    if(replicateOnOptimize && commitPoint.isOptimized()){
       indexCommitPoint = commitPoint;
     }
   }
@@ -788,6 +790,21 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
       replicateOnCommit = replicateAfter.contains("commit");
       replicateOnOptimize = replicateAfter.contains("optimize");
 
+      // if we only want to replicate on optimize, we need the deletion policy to
+      // save the last optimized commit point.
+      if (replicateOnOptimize && !replicateOnCommit) {
+        IndexDeletionPolicyWrapper wrapper = core.getDeletionPolicy();
+        IndexDeletionPolicy policy = wrapper == null ? null : wrapper.getWrappedDeletionPolicy();
+        if (policy instanceof SolrDeletionPolicy) {
+          SolrDeletionPolicy solrPolicy = (SolrDeletionPolicy)policy;
+          if (solrPolicy.getMaxOptimizedCommitsToKeep() < 1) {
+            solrPolicy.setMaxOptimizedCommitsToKeep(1);
+          }
+        } else {
+          LOG.warn("Replication can't call setMaxOptimizedCommitsToKeep on " + policy);
+        }
+      }
+
       if (replicateOnOptimize || backupOnOptimize) {
         core.getUpdateHandler().registerOptimizeCallback(getEventListener(backupOnOptimize, replicateOnOptimize));
       }
@@ -876,13 +893,13 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
        * This refreshes the latest replicateable index commit and optionally can create Snapshots as well
        */
       public void postCommit() {
-        if (getCommit) {
+        if (getCommit || snapshoot) {
           indexCommitPoint = core.getDeletionPolicy().getLatestCommit();
         }
         if (snapshoot) {
           try {
             SnapShooter snapShooter = new SnapShooter(core, null);
-            snapShooter.createSnapAsync(core.getDeletionPolicy().getLatestCommit().getFileNames(), ReplicationHandler.this);
+            snapShooter.createSnapAsync(indexCommitPoint.getFileNames(), ReplicationHandler.this);
           } catch (Exception e) {
             LOG.error("Exception while snapshooting", e);
           }

src/java/org/apache/solr/handler/SnapPuller.java

@@ -216,6 +216,11 @@ public class SnapPuller {
     List<Map<String, Object>> f = (List<Map<String, Object>>) nl.get(CMD_GET_FILE_LIST);
     if (f != null)
       filesToDownload = Collections.synchronizedList(f);
+    else {
+      filesToDownload = Collections.emptyList();
+      LOG.error("No files to download for indexversion: "+ version);
+    }
+
     f = (List<Map<String, Object>>) nl.get(CONF_FILES);
     if (f != null)
       confFilesToDownload = Collections.synchronizedList(f);
@@ -268,6 +273,8 @@ public class SnapPuller {
       LOG.info("Starting replication process");
       // get the list of files first
       fetchFileList(latestVersion);
+      // this can happen if the commit point is deleted before we fetch the file list.
+      if(filesToDownload.isEmpty()) return false;
       LOG.info("Number of files in latest index in master: " + filesToDownload.size());
 
       // Create the sync service

src/test/org/apache/solr/core/TestSolrDeletionPolicy1.java

@@ -100,7 +100,7 @@ public class TestSolrDeletionPolicy1 extends AbstractSolrTestCase {
     IndexDeletionPolicyWrapper delPolicy = h.getCore().getDeletionPolicy();
     addDocs();
     Map<Long, IndexCommit> commits = delPolicy.getCommits();
-    assertTrue(commits.size() == ((SolrDeletionPolicy) (delPolicy.getWrappedDeletionPolicy())).getMaxCommitsToKeep());
+    assertEquals(((SolrDeletionPolicy) (delPolicy.getWrappedDeletionPolicy())).getMaxOptimizedCommitsToKeep(), commits.size());
   }
 
   @Test