You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2015/02/12 10:37:46 UTC
svn commit: r1659181 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/CHANGES.txt solr/core/
solr/core/src/java/org/apache/solr/handler/SnapShooter.java
solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java
Author: shalin
Date: Thu Feb 12 09:37:46 2015
New Revision: 1659181
URL: http://svn.apache.org/r1659181
Log:
SOLR-6214: Snapshots numberToKeep param only keeps n-1 backups
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1659181&r1=1659180&r2=1659181&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Thu Feb 12 09:37:46 2015
@@ -80,6 +80,9 @@ Bug Fixes
* SOLR-7101: JmxMonitoredMap can throw an exception in clear when queryNames fails.
(Mark Miller, Wolfgang Hoschek)
+* SOLR-6214: Snapshots numberToKeep param only keeps n-1 backups.
+ (Mathias H., Ramana, Varun Thacker via shalin)
+
Optimizations
----------------------
* SOLR-7049: Move work done by the LIST Collections API call to the Collections
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/SnapShooter.java?rev=1659181&r1=1659180&r2=1659181&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/SnapShooter.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/handler/SnapShooter.java Thu Feb 12 09:37:46 2015
@@ -62,8 +62,6 @@ public class SnapShooter {
else {
File base = new File(core.getCoreDescriptor().getInstanceDir());
snapDir = org.apache.solr.util.FileUtils.resolvePath(base, location).getAbsolutePath();
- File dir = new File(snapDir);
- if (!dir.exists()) dir.mkdirs();
}
this.snapshotName = snapshotName;
@@ -84,8 +82,8 @@ public class SnapShooter {
if(snapshotName != null) {
createSnapshot(indexCommit, replicationHandler);
} else {
- deleteOldBackups(numberToKeep);
createSnapshot(indexCommit, replicationHandler);
+ deleteOldBackups(numberToKeep);
}
}
}.start();
@@ -158,23 +156,23 @@ public class SnapShooter {
private void deleteOldBackups(int numberToKeep) {
File[] files = new File(snapDir).listFiles();
List<OldBackupDirectory> dirs = new ArrayList<>();
- for(File f : files) {
+ for (File f : files) {
OldBackupDirectory obd = new OldBackupDirectory(f);
if(obd.dir != null) {
dirs.add(obd);
}
}
- if(numberToKeep > dirs.size()) {
+ if (numberToKeep > dirs.size() -1) {
return;
}
Collections.sort(dirs);
int i=1;
- for(OldBackupDirectory dir : dirs) {
- if( i++ > numberToKeep-1 ) {
+ for (OldBackupDirectory dir : dirs) {
+ if (i++ > numberToKeep) {
SnapPuller.delTree(dir.dir);
}
- }
+ }
}
protected void deleteNamedSnapshot(ReplicationHandler replicationHandler) {
@@ -199,7 +197,7 @@ public class SnapShooter {
File dir;
Date timestamp;
final Pattern dirNamePattern = Pattern.compile("^snapshot[.](.*)$");
-
+
OldBackupDirectory(File dir) {
if(dir.isDirectory()) {
Matcher m = dirNamePattern.matcher(dir.getName());
@@ -221,7 +219,7 @@ public class SnapShooter {
}
public static final String DATE_FMT = "yyyyMMddHHmmssSSS";
-
+
private static void copyFiles(Directory sourceDir, Collection<String> files, File destDir) throws IOException {
try (FSDirectory dir = new SimpleFSDirectory(destDir.toPath(), NoLockFactory.INSTANCE)) {
@@ -230,5 +228,5 @@ public class SnapShooter {
}
}
}
-
+
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java?rev=1659181&r1=1659180&r2=1659181&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java Thu Feb 12 09:37:46 2015
@@ -24,6 +24,7 @@ import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.Iterator;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -164,15 +165,17 @@ public class TestReplicationHandlerBacku
int nDocs = indexDocs();
- Path[] snapDir = new Path[2];
+ Path[] snapDir = new Path[5]; //One extra for the backup on commit
+ //First snapshot location
+ snapDir[0] = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot*").iterator().next();
boolean namedBackup = random().nextBoolean();
String firstBackupTimestamp = null;
String[] backupNames = null;
if (namedBackup) {
- backupNames = new String[2];
+ backupNames = new String[4];
}
- for (int i = 0; i < 2; i++) {
+ for (int i = 0; i < 4; i++) {
BackupCommand backupCommand;
final String backupName = TestUtil.randomSimpleString(random(), 1, 20);
if (!namedBackup) {
@@ -196,21 +199,43 @@ public class TestReplicationHandlerBacku
}
if (!namedBackup) {
- snapDir[i] = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot*").iterator().next();
+ snapDir[i+1] = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot*").iterator().next();
} else {
- snapDir[i] = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot." + backupName).iterator().next();
+ snapDir[i+1] = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot." + backupName).iterator().next();
}
- verify(snapDir[i], nDocs);
-
- }
+ verify(snapDir[i+1], nDocs);
- if (!namedBackup && Files.exists(snapDir[0])) {
- fail("The first backup should have been cleaned up because " + backupKeepParamName + " was set to 1.");
}
//Test Deletion of named backup
- if(namedBackup) {
+ if (namedBackup) {
testDeleteNamedBackup(backupNames);
+ } else {
+ //5 backups got created. 4 explicitly and one because a commit was called.
+ // Only the last two should still exist.
+ int count =0;
+ Iterator<Path> iter = Files.newDirectoryStream(Paths.get(master.getDataDir()), "snapshot*").iterator();
+ while (iter.hasNext()) {
+ iter.next();
+ count ++;
+ }
+
+ //There will be 2 backups, otherwise 1
+ if (backupKeepParamName.equals(ReplicationHandler.NUMBER_BACKUPS_TO_KEEP_REQUEST_PARAM)) {
+ assertEquals(2, count);
+
+ if (Files.exists(snapDir[0]) || Files.exists(snapDir[1]) || Files.exists(snapDir[2])) {
+ fail("Backup should have been cleaned up because " + backupKeepParamName + " was set to 2.");
+ }
+ } else {
+ assertEquals(1, count);
+
+ if (Files.exists(snapDir[0]) || Files.exists(snapDir[1]) || Files.exists(snapDir[2])
+ || Files.exists(snapDir[3])) {
+ fail("Backup should have been cleaned up because " + backupKeepParamName + " was set to 2.");
+ }
+ }
+
}
}
@@ -263,7 +288,7 @@ public class TestReplicationHandlerBacku
"&name=" + backupName;
} else {
masterUrl = buildUrl(masterJetty.getLocalPort(), context) + "/" + DEFAULT_TEST_CORENAME + "/replication?command=" + cmd +
- (addNumberToKeepInRequest ? "&" + backupKeepParamName + "=1" : "");
+ (addNumberToKeepInRequest ? "&" + backupKeepParamName + "=2" : "");
}
InputStream stream = null;