Return-Path: X-Original-To: apmail-lucene-commits-archive@www.apache.org Delivered-To: apmail-lucene-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 273BA102F6 for ; Thu, 12 Feb 2015 09:37:47 +0000 (UTC) Received: (qmail 99606 invoked by uid 500); 12 Feb 2015 09:37:47 -0000 Mailing-List: contact commits-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@lucene.apache.org Delivered-To: mailing list commits@lucene.apache.org Received: (qmail 99596 invoked by uid 99); 12 Feb 2015 09:37:47 -0000 Received: from eris.apache.org (HELO hades.apache.org) (140.211.11.105) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Feb 2015 09:37:47 +0000 Received: from hades.apache.org (localhost [127.0.0.1]) by hades.apache.org (ASF Mail Server at hades.apache.org) with ESMTP id DA1B8AC006D for ; Thu, 12 Feb 2015 09:37:46 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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 Date: Thu, 12 Feb 2015 09:37:46 -0000 To: commits@lucene.apache.org From: shalin@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20150212093746.DA1B8AC006D@hades.apache.org> 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 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 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 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;