You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2016/04/27 20:22:34 UTC
[50/50] [abbrv] lucene-solr:solr-5750: SOLR-5750: refactor
SnapShooter.createSnapshot surrounding logic Fixed a bug too;
createSnapshot() wasn't reserving the commit with the deletion policy
SOLR-5750: refactor SnapShooter.createSnapshot surrounding logic
Fixed a bug too; createSnapshot() wasn't reserving the commit with the deletion policy
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ca173ab1
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ca173ab1
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ca173ab1
Branch: refs/heads/solr-5750
Commit: ca173ab1af9824ba262ba206bcaab9babbb2ac89
Parents: e54718d
Author: David Smiley <ds...@apache.org>
Authored: Wed Apr 27 14:20:48 2016 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Wed Apr 27 14:20:48 2016 -0400
----------------------------------------------------------------------
.../apache/solr/handler/ReplicationHandler.java | 7 +-
.../org/apache/solr/handler/SnapShooter.java | 97 +++++++++-----------
.../solr/handler/admin/CoreAdminOperation.java | 32 ++-----
3 files changed, 53 insertions(+), 83 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ca173ab1/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
index c98fabf..4dbcef2 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -87,7 +87,6 @@ import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.update.CdcrUpdateLog;
import org.apache.solr.update.SolrIndexWriter;
-import org.apache.solr.update.UpdateLog;
import org.apache.solr.update.VersionInfo;
import org.apache.solr.util.DefaultSolrThreadFactory;
import org.apache.solr.util.NumberUtils;
@@ -195,7 +194,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
volatile IndexCommit indexCommitPoint;
- volatile NamedList<Object> snapShootDetails;
+ volatile NamedList<?> snapShootDetails;
private AtomicBoolean replicationEnabled = new AtomicBoolean(true);
@@ -509,7 +508,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
// small race here before the commit point is saved
SnapShooter snapShooter = new SnapShooter(core, params.get("location"), params.get(NAME));
snapShooter.validateCreateSnapshot();
- snapShooter.createSnapAsync(indexCommit, numberToKeep, this);
+ snapShooter.createSnapAsync(indexCommit, numberToKeep, (nl) -> snapShootDetails = nl);
} catch (Exception e) {
LOG.warn("Exception during creating a snapshot", e);
@@ -1323,7 +1322,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
}
SnapShooter snapShooter = new SnapShooter(core, null, null);
snapShooter.validateCreateSnapshot();
- snapShooter.createSnapAsync(currentCommitPoint, numberToKeep, ReplicationHandler.this);
+ snapShooter.createSnapAsync(currentCommitPoint, numberToKeep, (nl) -> snapShootDetails = nl);
} catch (Exception e) {
LOG.error("Exception while snapshooting", e);
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ca173ab1/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
index 2e20b72..92b9246 100644
--- a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
+++ b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
@@ -26,6 +26,7 @@ import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Locale;
+import java.util.function.Consumer;
import org.apache.lucene.index.IndexCommit;
import org.apache.lucene.store.Directory;
@@ -36,6 +37,7 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.DirectoryFactory;
import org.apache.solr.core.DirectoryFactory.DirContext;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
import org.apache.solr.core.SolrCore;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.util.RefCounted;
@@ -75,21 +77,6 @@ public class SnapShooter {
}
}
- void createSnapAsync(final IndexCommit indexCommit, final int numberToKeep, final ReplicationHandler replicationHandler) {
- solrCore.getDeletionPolicy().saveCommitPoint(indexCommit.getGeneration());
-
- new Thread() {
- @Override
- public void run() {
- if(snapshotName != null) {
- createSnapshot(indexCommit, replicationHandler);
- } else {
- createSnapshot(indexCommit, replicationHandler);
- deleteOldBackups(numberToKeep);
- }
- }
- }.start();
- }
public void validateDeleteSnapshot() {
boolean dirFound = false;
@@ -126,50 +113,55 @@ public class SnapShooter {
}
}
- //nocommit - copy pasted from createSnapshot. Need to reconcile tho
- public NamedList createSnapshot() {
+ public NamedList createSnapshot() throws Exception {
+ IndexDeletionPolicyWrapper deletionPolicy = solrCore.getDeletionPolicy();
RefCounted<SolrIndexSearcher> searcher = solrCore.getSearcher();
- IndexCommit indexCommit = null;
-
- NamedList<Object> details = new NamedList<>();
try {
- details.add("startTime", new Date().toString());
- LOG.info("Creating backup snapshot " + (snapshotName == null ? "<not named>" : snapshotName) + " at " + snapDir);
-
- indexCommit = searcher.get().getIndexReader().getIndexCommit();
- Collection<String> files = indexCommit.getFileNames();
-
- Directory dir = solrCore.getDirectoryFactory().get(solrCore.getIndexDir(), DirContext.DEFAULT, solrCore.getSolrConfig().indexConfig.lockType);
+ //TODO should we try solrCore.getDeletionPolicy().getLatestCommit() first?
+ IndexCommit indexCommit = searcher.get().getIndexReader().getIndexCommit();
+ deletionPolicy.saveCommitPoint(indexCommit.getGeneration());
try {
- copyFiles(dir, files, snapShotDir);
+ return createSnapshot(indexCommit);
} finally {
- solrCore.getDirectoryFactory().release(dir);
+ deletionPolicy.releaseCommitPoint(indexCommit.getGeneration());
}
-
- details.add("fileCount", files.size());
- details.add("status", "success");
- details.add("snapshotCompletedAt", new Date().toString());
- details.add("snapshotName", snapshotName);
- LOG.info("Done creating backup snapshot: " + (snapshotName == null ? "<not named>" : snapshotName) +
- " at " + snapDir);
- } catch (Exception e) {
- IndexFetcher.delTree(snapShotDir);
- LOG.error("Exception while creating snapshot", e);
- details.add("snapShootException", e.getMessage());
} finally {
- solrCore.getDeletionPolicy().releaseCommitPoint(indexCommit.getGeneration());
searcher.decref();
}
- return details;
}
- void createSnapshot(final IndexCommit indexCommit, ReplicationHandler replicationHandler) {
+ public void createSnapAsync(final IndexCommit indexCommit, final int numberToKeep, Consumer<NamedList> result) {
+ solrCore.getDeletionPolicy().saveCommitPoint(indexCommit.getGeneration());
+
+ new Thread() { //TODO should use Solr's ExecutorUtil
+ @Override
+ public void run() {
+ try {
+ result.accept(createSnapshot(indexCommit));
+ } catch (Exception e) {
+ LOG.error("Exception while creating snapshot", e);
+ NamedList snapShootDetails = new NamedList<>();
+ snapShootDetails.add("snapShootException", e.getMessage());
+ result.accept(snapShootDetails);
+ } finally {
+ solrCore.getDeletionPolicy().releaseCommitPoint(indexCommit.getGeneration());
+ }
+ if (snapshotName == null) {
+ deleteOldBackups(numberToKeep);
+ }
+ }
+ }.start();
+ }
+
+ // note: remember to reserve the indexCommit first so it won't get deleted concurrently
+ protected NamedList createSnapshot(final IndexCommit indexCommit) throws Exception {
LOG.info("Creating backup snapshot " + (snapshotName == null ? "<not named>" : snapshotName) + " at " + snapDir);
- NamedList<Object> details = new NamedList<>();
- details.add("startTime", new Date().toString());
+ boolean success = false;
try {
- Collection<String> files = indexCommit.getFileNames();
+ NamedList<Object> details = new NamedList<>();
+ details.add("startTime", new Date().toString());//bad; should be Instant.now().toString()
+ Collection<String> files = indexCommit.getFileNames();
Directory dir = solrCore.getDirectoryFactory().get(solrCore.getIndexDir(), DirContext.DEFAULT, solrCore.getSolrConfig().indexConfig.lockType);
try {
copyFiles(dir, files, snapShotDir);
@@ -179,17 +171,16 @@ public class SnapShooter {
details.add("fileCount", files.size());
details.add("status", "success");
- details.add("snapshotCompletedAt", new Date().toString());
+ details.add("snapshotCompletedAt", new Date().toString());//bad; should be Instant.now().toString()
details.add("snapshotName", snapshotName);
LOG.info("Done creating backup snapshot: " + (snapshotName == null ? "<not named>" : snapshotName) +
" at " + snapDir);
- } catch (Exception e) {
- IndexFetcher.delTree(snapShotDir);
- LOG.error("Exception while creating snapshot", e);
- details.add("snapShootException", e.getMessage());
+ success = true;
+ return details;
} finally {
- solrCore.getDeletionPolicy().releaseCommitPoint(indexCommit.getGeneration());
- replicationHandler.snapShootDetails = details;
+ if (!success) {
+ IndexFetcher.delTree(snapShotDir);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ca173ab1/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
index 2365b87..6cf4589 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
@@ -39,7 +39,6 @@ import org.apache.lucene.util.IOUtils;
import org.apache.solr.cloud.CloudDescriptor;
import org.apache.solr.cloud.SyncStrategy;
import org.apache.solr.cloud.ZkController;
-import org.apache.solr.common.NonExistentCoreException;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
@@ -82,26 +81,7 @@ import org.slf4j.LoggerFactory;
import static org.apache.solr.common.cloud.DocCollection.DOC_ROUTER;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.PATH;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.BACKUPCORE;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.FORCEPREPAREFORLEADERSHIP;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.INVOKE;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.MERGEINDEXES;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.OVERSEEROP;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.PREPRECOVERY;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REJOINLEADERELECTION;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.RELOAD;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.RENAME;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REQUESTAPPLYUPDATES;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REQUESTBUFFERUPDATES;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REQUESTRECOVERY;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REQUESTSTATUS;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.REQUESTSYNCSHARD;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.RESTORECORE;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.SPLIT;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.STATUS;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.SWAP;
-import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.UNLOAD;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.*;
import static org.apache.solr.handler.admin.CoreAdminHandler.COMPLETED;
import static org.apache.solr.handler.admin.CoreAdminHandler.CallInfo;
import static org.apache.solr.handler.admin.CoreAdminHandler.FAILED;
@@ -885,10 +865,10 @@ enum CoreAdminOperation {
try (SolrCore core = callInfo.handler.coreContainer.getCore(cname)) {
SnapShooter snapShooter = new SnapShooter(core, location, name);
snapShooter.validateCreateSnapshot();
- NamedList details = snapShooter.createSnapshot();
- if (details.get("snapShootException") != null) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to backup core=" + core.getName());
- }
+ snapShooter.createSnapshot();
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ "Failed to backup core=" + cname + " because " + e, e);
}
}
},
@@ -912,7 +892,7 @@ enum CoreAdminOperation {
}
String location = params.get("location");
- if (name == null) {
+ if (location == null) {
throw new IllegalArgumentException("location is required");
}