You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nr...@apache.org on 2018/01/23 16:52:32 UTC
[geode] branch develop updated: GEODE-4330: Move logic for handling
temporary files during backup (#1323)
This is an automated email from the ASF dual-hosted git repository.
nreich pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 6501fb5 GEODE-4330: Move logic for handling temporary files during backup (#1323)
6501fb5 is described below
commit 6501fb55edc11291e06d285227d5714a416916b0
Author: Nick Reich <nr...@pivotal.io>
AuthorDate: Tue Jan 23 08:52:29 2018 -0800
GEODE-4330: Move logic for handling temporary files during backup (#1323)
---
.../geode/internal/cache/backup/BackupManager.java | 112 ++++++-----------
.../cache/backup/TemporaryBackupFiles.java | 140 +++++++++++++++++++++
.../cache/backup/TemporaryBackupFilesTest.java | 110 ++++++++++++++++
3 files changed, 286 insertions(+), 76 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/backup/BackupManager.java b/geode-core/src/main/java/org/apache/geode/internal/cache/backup/BackupManager.java
index 2a124ad..bb19ff2 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/backup/BackupManager.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/backup/BackupManager.java
@@ -73,16 +73,14 @@ public class BackupManager {
private final InternalCache cache;
private final CountDownLatch allowDestroys = new CountDownLatch(1);
private final BackupDefinition backupDefinition = new BackupDefinition();
- private final String diskStoreDirectoryName;
+ private final String memberId;
private volatile boolean isCancelled = false;
- private Path tempDirectory;
- private final Map<DiskStore, Map<DirectoryHolder, Path>> diskStoreDirTempDirsByDiskStore =
- new HashMap<>();
+ private TemporaryBackupFiles temporaryFiles;
public BackupManager(InternalDistributedMember sender, InternalCache gemFireCache) {
this.sender = sender;
this.cache = gemFireCache;
- diskStoreDirectoryName = DATA_STORES_TEMPORARY_DIRECTORY + System.currentTimeMillis();
+ memberId = getCleanedMemberId();
}
public void validateRequestingAdmin() {
@@ -99,6 +97,7 @@ public class BackupManager {
HashSet<PersistentID> persistentIds = new HashSet<>();
for (DiskStore store : cache.listDiskStoresIncludingRegionOwned()) {
DiskStoreImpl storeImpl = (DiskStoreImpl) store;
+
storeImpl.lockStoreBeforeBackup();
if (storeImpl.hasPersistedData()) {
persistentIds.add(storeImpl.getPersistentID());
@@ -110,18 +109,20 @@ public class BackupManager {
public HashSet<PersistentID> doBackup(File targetDir, File baselineDir, boolean abort)
throws IOException {
+ if (abort) {
+ cleanup();
+ return new HashSet<>();
+ }
+
try {
- if (abort) {
- return new HashSet<>();
- }
- tempDirectory = Files.createTempDirectory("backup_" + System.currentTimeMillis());
- File backupDir = getBackupDir(targetDir);
+ temporaryFiles = TemporaryBackupFiles.create();
+ File memberBackupDir = new File(targetDir, memberId);
// Make sure our baseline is okay for this member, then create inspector for baseline backup
baselineDir = checkBaseline(baselineDir);
BackupInspector inspector =
(baselineDir == null ? null : BackupInspector.createInspector(baselineDir));
- File storesDir = new File(backupDir, DATA_STORES_DIRECTORY);
+ File storesDir = new File(memberBackupDir, DATA_STORES_DIRECTORY);
Collection<DiskStore> diskStores = cache.listDiskStoresIncludingRegionOwned();
Map<DiskStoreImpl, DiskStoreBackup> backupByDiskStores =
@@ -130,18 +131,18 @@ public class BackupManager {
HashSet<PersistentID> persistentIds = finishDiskStoreBackups(backupByDiskStores);
if (!backupByDiskStores.isEmpty()) {
- backupAdditionalFiles(backupDir);
+ backupAdditionalFiles(memberBackupDir);
backupDefinition.setRestoreScript(restoreScript);
}
if (!backupByDiskStores.isEmpty()) {
// TODO: allow different stategies...
- BackupDestination backupDestination = new FileSystemBackupDestination(backupDir.toPath());
+ BackupDestination backupDestination =
+ new FileSystemBackupDestination(memberBackupDir.toPath());
backupDestination.backupFiles(backupDefinition);
}
return persistentIds;
-
} finally {
cleanup();
}
@@ -198,32 +199,15 @@ public class BackupManager {
private void cleanup() {
isCancelled = true;
allowDestroys.countDown();
- cleanupTemporaryFiles();
+ if (temporaryFiles != null) {
+ temporaryFiles.cleanupFiles();
+ }
releaseBackupLocks();
getDistributionManager().removeAllMembershipListener(membershipListener);
cache.clearBackupManager();
}
- private void cleanupTemporaryFiles() {
- if (tempDirectory != null) {
- try {
- FileUtils.deleteDirectory(tempDirectory.toFile());
- } catch (IOException e) {
- logger.warn("Unable to delete temporary directory created during backup, " + tempDirectory,
- e);
- }
- }
- for (Map<DirectoryHolder, Path> diskStoreDirToTempDirMap : diskStoreDirTempDirsByDiskStore
- .values()) {
- for (Path tempDir : diskStoreDirToTempDirMap.values()) {
- try {
- FileUtils.deleteDirectory(tempDir.toFile());
- } catch (IOException e) {
- logger.warn("Unable to delete temporary directory created during backup, " + tempDir, e);
- }
- }
- }
- }
+
private void releaseBackupLocks() {
for (DiskStore store : cache.listDiskStoresIncludingRegionOwned()) {
@@ -263,12 +247,12 @@ public class BackupManager {
* @return null if the backup is to be a full backup otherwise return the data store directory in
* the previous backup for this member (if incremental).
*/
- private File checkBaseline(File baselineParentDir) throws IOException {
+ private File checkBaseline(File baselineParentDir) {
File baselineDir = null;
if (null != baselineParentDir) {
// Start by looking for this memberId
- baselineDir = getBackupDir(baselineParentDir);
+ baselineDir = new File(baselineParentDir, memberId);
if (!baselineDir.exists()) {
// hmmm, did this member have a restart?
@@ -311,7 +295,7 @@ public class BackupManager {
if (isCancelled()) {
break;
}
- copyOplog(diskStore, tempDirectory.toFile(), oplog);
+ copyOplog(diskStore, temporaryFiles.getDirectory().toFile(), oplog);
// Allow the oplog to be deleted, and process any pending delete
backup.backupFinished(oplog);
@@ -392,13 +376,7 @@ public class BackupManager {
backup = new DiskStoreBackup(allOplogs, targetDir);
backupByDiskStore.put(diskStore, backup);
-
- // TODO cleanup new location definition code
- /*
- * Path diskstoreDir = getBackupDir(tempDir.toFile(),
- * diskStore.getInforFileDirIndex()).toPath(); Files.createDirectories(diskstoreDir);
- */
- backupDiskInitFile(diskStore, tempDirectory);
+ backupDiskInitFile(diskStore, temporaryFiles.getDirectory());
diskStore.getPersistentOplogSet().forceRoll(null);
if (logger.isDebugEnabled()) {
@@ -430,7 +408,7 @@ public class BackupManager {
private void addDiskStoreDirectoriesToRestoreScript(DiskStoreImpl diskStore, File targetDir) {
DirectoryHolder[] directories = diskStore.getDirectoryHolders();
for (int i = 0; i < directories.length; i++) {
- File backupDir = getBackupDir(targetDir, i);
+ File backupDir = getBackupDirForCurrentMember(targetDir, i);
restoreScript.addFile(directories[i].getDir(), backupDir);
}
}
@@ -495,12 +473,12 @@ public class BackupManager {
return oplogMap;
}
- private File getBackupDir(File targetDir, int index) {
+ private File getBackupDirForCurrentMember(File targetDir, int index) {
return new File(targetDir, BACKUP_DIR_PREFIX + index);
}
private void backupConfigFiles() throws IOException {
- Files.createDirectories(tempDirectory.resolve(CONFIG_DIRECTORY));
+ Files.createDirectories(temporaryFiles.getDirectory().resolve(CONFIG_DIRECTORY));
addConfigFileToBackup(cache.getCacheXmlURL());
addConfigFileToBackup(DistributedSystem.getPropertiesFileURL());
// TODO: should the gfsecurity.properties file be backed up?
@@ -510,7 +488,8 @@ public class BackupManager {
if (fileUrl != null) {
try {
Path source = Paths.get(fileUrl.toURI());
- Path destination = tempDirectory.resolve(CONFIG_DIRECTORY).resolve(source.getFileName());
+ Path destination =
+ temporaryFiles.getDirectory().resolve(CONFIG_DIRECTORY).resolve(source.getFileName());
Files.copy(source, destination, StandardCopyOption.COPY_ATTRIBUTES);
backupDefinition.addConfigFileToBackup(destination);
} catch (URISyntaxException e) {
@@ -520,13 +499,14 @@ public class BackupManager {
}
private void backupUserFiles(File backupDir) throws IOException {
- Files.createDirectories(tempDirectory.resolve(USER_FILES));
+ Files.createDirectories(temporaryFiles.getDirectory().resolve(USER_FILES));
List<File> backupFiles = cache.getBackupFiles();
File userBackupDir = new File(backupDir, USER_FILES);
for (File original : backupFiles) {
if (original.exists()) {
original = original.getAbsoluteFile();
- Path destination = tempDirectory.resolve(USER_FILES).resolve(original.getName());
+ Path destination =
+ temporaryFiles.getDirectory().resolve(USER_FILES).resolve(original.getName());
if (original.isDirectory()) {
FileUtils.copyDirectory(original, destination.toFile());
} else {
@@ -560,7 +540,8 @@ public class BackupManager {
for (DeployedJar jar : jarList) {
File source = new File(jar.getFileCanonicalPath());
String sourceFileName = source.getName();
- Path destination = tempDirectory.resolve(USER_FILES).resolve(sourceFileName);
+ Path destination =
+ temporaryFiles.getDirectory().resolve(USER_FILES).resolve(sourceFileName);
Files.copy(source.toPath(), destination, StandardCopyOption.COPY_ATTRIBUTES);
backupDefinition.addDeployedJarToBackup(destination);
@@ -576,12 +557,11 @@ public class BackupManager {
}
}
- private File getBackupDir(File targetDir) {
+ private String getCleanedMemberId() {
InternalDistributedMember memberId =
cache.getInternalDistributedSystem().getDistributedMember();
String vmId = memberId.toString();
- vmId = cleanSpecialCharacters(vmId);
- return new File(targetDir, vmId);
+ return cleanSpecialCharacters(vmId);
}
private void copyOplog(DiskStore diskStore, File targetDir, Oplog oplog) throws IOException {
@@ -597,7 +577,7 @@ public class BackupManager {
throws IOException {
if (file != null && file.exists()) {
try {
- Path tempDiskDir = getTempDirForDiskStore(diskStore, dirHolder);
+ Path tempDiskDir = temporaryFiles.getDiskStoreDirectory(diskStore, dirHolder);
Files.createLink(tempDiskDir.resolve(file.getName()), file.toPath());
backupDefinition.addOplogFileToBackup(diskStore, tempDiskDir.resolve(file.getName()));
} catch (IOException | UnsupportedOperationException e) {
@@ -607,26 +587,6 @@ public class BackupManager {
}
}
- private Path getTempDirForDiskStore(DiskStore diskStore, DirectoryHolder dirHolder)
- throws IOException {
- Map<DirectoryHolder, Path> tempDirByDirectoryHolder =
- diskStoreDirTempDirsByDiskStore.get(diskStore);
- if (tempDirByDirectoryHolder == null) {
- tempDirByDirectoryHolder = new HashMap<>();
- diskStoreDirTempDirsByDiskStore.put(diskStore, tempDirByDirectoryHolder);
- }
- Path directory = tempDirByDirectoryHolder.get(dirHolder);
- if (directory != null) {
- return directory;
- }
-
- File diskStoreDir = dirHolder.getDir();
- directory = diskStoreDir.toPath().resolve(diskStoreDirectoryName);
- Files.createDirectories(directory);
- tempDirByDirectoryHolder.put(dirHolder, directory);
- return directory;
- }
-
private String cleanSpecialCharacters(String string) {
return string.replaceAll("[^\\w]+", "_");
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/backup/TemporaryBackupFiles.java b/geode-core/src/main/java/org/apache/geode/internal/cache/backup/TemporaryBackupFiles.java
new file mode 100644
index 0000000..05e4edd
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/backup/TemporaryBackupFiles.java
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.backup;
+
+import static org.apache.geode.internal.cache.backup.BackupManager.DATA_STORES_TEMPORARY_DIRECTORY;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+
+import org.apache.geode.cache.DiskStore;
+import org.apache.geode.internal.cache.DirectoryHolder;
+import org.apache.geode.internal.cache.Oplog;
+import org.apache.geode.internal.logging.LogService;
+
+/**
+ * Creates and keeps track of the temporary locations used during a backup. Most temporary files are
+ * stored in a shared temporary directory, except for those for DiskStores. For each
+ * {@link DiskStore}, a temporary directory is created per disk directory within that disk directory
+ * (for enabling creation of hard-links for backing up the files of an {@link Oplog}).
+ */
+class TemporaryBackupFiles {
+ private static final Logger logger = LogService.getLogger();
+
+ private final String diskStoreDirectoryName;
+ private final Path directory;
+ private final Map<DiskStore, Map<DirectoryHolder, Path>> diskStoreDirDirsByDiskStore =
+ new HashMap<>();
+
+ /**
+ * Creates a new instance with the default structure, where temporary directories are created
+ * using the current timestamp in their name for the purpose of uniquification
+ *
+ * @return a new TemporaryBackupFiles
+ * @throws IOException If unable to create a temporary directory
+ */
+ static TemporaryBackupFiles create() throws IOException {
+ long currentTime = System.currentTimeMillis();
+ String diskStoreDirectoryName = DATA_STORES_TEMPORARY_DIRECTORY + currentTime;
+ Path temporaryDirectory = Files.createTempDirectory("backup_" + currentTime);
+ return new TemporaryBackupFiles(temporaryDirectory, diskStoreDirectoryName);
+ }
+
+ /**
+ * Constructs a new TemporaryBackupFiles that will use the specified locations for temporary files
+ *
+ * @param directory the location to create and store temporary files during backup
+ * @param diskStoreDirectoryName name of directory to create within each disk store directory for
+ * its temporary files during backup
+ */
+ TemporaryBackupFiles(Path directory, String diskStoreDirectoryName) {
+ if (directory == null) {
+ throw new IllegalArgumentException("Must provide a temporary directory location");
+ }
+ if (diskStoreDirectoryName == null || diskStoreDirectoryName.isEmpty()) {
+ throw new IllegalArgumentException("Must provide a name for temporary DiskStore directories");
+ }
+
+ this.directory = directory;
+ this.diskStoreDirectoryName = diskStoreDirectoryName;
+ }
+
+ /**
+ * Provides the temporary directory location used for all temporary backup files except those for
+ * Oplogs
+ *
+ * @return The path of the shared temporary directory
+ */
+ Path getDirectory() {
+ return directory;
+ }
+
+ /**
+ * Provides, and creates if necessary, the temporary directory used during the backup for Oplog
+ * files for the given DiskStore and DirectoryHolder
+ *
+ * @param diskStore The corresponding {@link DiskStore} to get a temporary directory for
+ * @param dirHolder The disk directory of the {@link DiskStore} to get a temporary directory for
+ * @return Path to the temporary directory
+ * @throws IOException If the temporary directory did not exist and could not be created
+ */
+ Path getDiskStoreDirectory(DiskStore diskStore, DirectoryHolder dirHolder) throws IOException {
+ Map<DirectoryHolder, Path> tempDirByDirectoryHolder =
+ diskStoreDirDirsByDiskStore.computeIfAbsent(diskStore, k -> new HashMap<>());
+ Path directory = tempDirByDirectoryHolder.get(dirHolder);
+ if (directory != null) {
+ return directory;
+ }
+
+ File diskStoreDir = dirHolder.getDir();
+ directory = diskStoreDir.toPath().resolve(diskStoreDirectoryName);
+ Files.createDirectories(directory);
+ tempDirByDirectoryHolder.put(dirHolder, directory);
+ return directory;
+ }
+
+ /**
+ * Attempts to delete all temporary directories and their contents. An attempt will be made to
+ * delete each directory, regardless of the failure to delete any particular one.
+ */
+ void cleanupFiles() {
+ if (directory != null) {
+ deleteDirectory(directory);
+ }
+
+ for (Map<DirectoryHolder, Path> diskStoreDirToTempDirMap : diskStoreDirDirsByDiskStore
+ .values()) {
+ for (Path tempDir : diskStoreDirToTempDirMap.values()) {
+ deleteDirectory(tempDir);
+ }
+ }
+ }
+
+ private void deleteDirectory(Path directory) {
+ try {
+ FileUtils.deleteDirectory(directory.toFile());
+ } catch (IOException e) {
+ logger.warn("Unable to delete temporary directory created during backup, " + this.directory,
+ e);
+ }
+ }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/backup/TemporaryBackupFilesTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/backup/TemporaryBackupFilesTest.java
new file mode 100644
index 0000000..81ef446
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/backup/TemporaryBackupFilesTest.java
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.backup;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.nio.file.Path;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.cache.DiskStore;
+import org.apache.geode.internal.cache.DirectoryHolder;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class TemporaryBackupFilesTest {
+
+ private static final String DISK_STORE_DIR_NAME = "testDiskStores";
+
+ @Rule
+ public TemporaryFolder tempDir = new TemporaryFolder();
+
+ private Path baseTempDirectory;
+ private TemporaryBackupFiles backupFiles;
+ private DiskStore diskStore;
+ private DirectoryHolder directoryHolder;
+
+ @Before
+ public void setup() throws IOException {
+ baseTempDirectory = tempDir.newFolder().toPath();
+ backupFiles = new TemporaryBackupFiles(baseTempDirectory, DISK_STORE_DIR_NAME);
+ diskStore = mock(DiskStore.class);
+ directoryHolder = mock(DirectoryHolder.class);
+ when(directoryHolder.getDir()).thenReturn(baseTempDirectory.resolve("dir1").toFile());
+ }
+
+ @Test
+ public void factoryMethodCreatesValidInstance() throws IOException {
+ TemporaryBackupFiles backupFiles = TemporaryBackupFiles.create();
+ assertThat(backupFiles.getDirectory()).exists();
+ }
+
+ @Test
+ public void cannotCreateInstanceWithoutDirectoryLocation() {
+ assertThatThrownBy(() -> new TemporaryBackupFiles(null, "test"))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void cannotCreateInstanceWithoutNameForDiskStoreDirectories() {
+ assertThatThrownBy(() -> new TemporaryBackupFiles(baseTempDirectory, null))
+ .isInstanceOf(IllegalArgumentException.class);
+ assertThatThrownBy(() -> new TemporaryBackupFiles(baseTempDirectory, ""))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void returnsCorrectTemporaryDirectory() {
+ assertThat(backupFiles.getDirectory()).isEqualTo(baseTempDirectory);
+ }
+
+ @Test
+ public void returnsCreatedDirectoryForADiskStore() throws IOException {
+ Path diskStoreDir = backupFiles.getDiskStoreDirectory(diskStore, directoryHolder);
+ assertThat(diskStoreDir)
+ .isEqualTo(directoryHolder.getDir().toPath().resolve(DISK_STORE_DIR_NAME));
+ assertThat(diskStoreDir).exists();
+ }
+
+ @Test
+ public void returnsTheSameFileEachTimeForADiskStoreAndDirHolder() throws IOException {
+ Path diskStoreDir = backupFiles.getDiskStoreDirectory(diskStore, directoryHolder);
+ assertThat(backupFiles.getDiskStoreDirectory(diskStore, directoryHolder))
+ .isSameAs(diskStoreDir);
+ }
+
+ @Test
+ public void cleansUpAllTemporaryDirectories() throws IOException {
+ DirectoryHolder directoryHolder2 = mock(DirectoryHolder.class);
+ when(directoryHolder2.getDir()).thenReturn(baseTempDirectory.resolve("dir2").toFile());
+ Path diskStoreDir = backupFiles.getDiskStoreDirectory(diskStore, directoryHolder);
+ Path diskStoreDir2 = backupFiles.getDiskStoreDirectory(diskStore, directoryHolder2);
+
+ backupFiles.cleanupFiles();
+
+ assertThat(baseTempDirectory).doesNotExist();
+ assertThat(diskStoreDir).doesNotExist();
+ assertThat(diskStoreDir2).doesNotExist();
+ }
+}
--
To stop receiving notification emails like this one, please contact
nreich@apache.org.