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 2017/10/19 21:51:47 UTC
[geode] branch develop updated: GEODE-3719: Improve invocations of
disk store backups (#853)
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 472b96c GEODE-3719: Improve invocations of disk store backups (#853)
472b96c is described below
commit 472b96c7ba6489970356d70a23b41adbcf8294ed
Author: Nick Reich <nr...@pivotal.io>
AuthorDate: Thu Oct 19 14:51:45 2017 -0700
GEODE-3719: Improve invocations of disk store backups (#853)
* GEODE-3719: Improve invocations of disk store backups
* Standardize pattern for invoking a backup and the result returned
---
.../apache/geode/admin/AdminDistributedSystem.java | 1 -
.../java/org/apache/geode/admin/BackupStatus.java | 21 +----
.../admin/internal/AdminDistributedSystemImpl.java | 38 +-------
.../geode/admin/internal/BackupStatusImpl.java | 35 +------
.../org/apache/geode/internal/SystemAdmin.java | 9 +-
.../apache/geode/internal/cache/BackupUtil.java | 75 +++++++++++++++
.../geode/{admin => management}/BackupStatus.java | 9 +-
.../apache/geode/management/DiskBackupStatus.java | 37 +-------
.../internal/BackupStatusImpl.java | 6 +-
.../DiskBackupStatusImpl.java} | 58 ++++++++----
.../internal/beans/DistributedSystemBridge.java | 48 +++-------
.../cli/commands/BackupDiskStoreCommand.java | 9 +-
.../geode/internal/cache/BackupDUnitTest.java | 2 +-
.../internal/cache/IncrementalBackupDUnitTest.java | 75 +++++----------
.../PersistentPartitionedRegionTestBase.java | 21 ++---
.../geode/management/DiskBackupStatusTest.java | 105 +++++++++++++++++++++
.../geode/management/DiskManagementDUnitTest.java | 2 +-
.../apache/geode/codeAnalysis/excludedClasses.txt | 2 +
18 files changed, 292 insertions(+), 261 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/admin/AdminDistributedSystem.java b/geode-core/src/main/java/org/apache/geode/admin/AdminDistributedSystem.java
index cd4dc7e..45c4214 100755
--- a/geode-core/src/main/java/org/apache/geode/admin/AdminDistributedSystem.java
+++ b/geode-core/src/main/java/org/apache/geode/admin/AdminDistributedSystem.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.admin;
-import org.apache.geode.LogWriter;
import org.apache.geode.cache.DataPolicy;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
diff --git a/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java b/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java
index c696b07..b4c4079 100644
--- a/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java
+++ b/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java
@@ -14,12 +14,6 @@
*/
package org.apache.geode.admin;
-import java.util.Map;
-import java.util.Set;
-
-import org.apache.geode.cache.persistence.PersistentID;
-import org.apache.geode.distributed.DistributedMember;
-
/**
* The status of a backup operation, returned by
* {@link AdminDistributedSystem#backupAllMembers(java.io.File,java.io.File)}.
@@ -29,18 +23,5 @@ import org.apache.geode.distributed.DistributedMember;
* "{@docRoot}/org/apache/geode/management/package-summary.html">management</a></code>
* package instead
*/
-public interface BackupStatus {
-
- /**
- * Returns a map of disk stores that were successfully backed up. The key is an online distributed
- * member. The value is the set of disk stores on that distributed member.
- */
- Map<DistributedMember, Set<PersistentID>> getBackedUpDiskStores();
-
- /**
- * Returns the set of disk stores that were known to be offline at the time of the backup. These
- * members were not backed up. If this set is not empty the backup may not contain a complete
- * snapshot of any partitioned regions in the distributed system.
- */
- Set<PersistentID> getOfflineDiskStores();
+public interface BackupStatus extends org.apache.geode.management.BackupStatus {
}
diff --git a/geode-core/src/main/java/org/apache/geode/admin/internal/AdminDistributedSystemImpl.java b/geode-core/src/main/java/org/apache/geode/admin/internal/AdminDistributedSystemImpl.java
index 9cd9ce3..1847586 100755
--- a/geode-core/src/main/java/org/apache/geode/admin/internal/AdminDistributedSystemImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/admin/internal/AdminDistributedSystemImpl.java
@@ -28,6 +28,7 @@ import org.apache.geode.internal.Assert;
import org.apache.geode.internal.Banner;
import org.apache.geode.internal.admin.*;
import org.apache.geode.internal.admin.remote.*;
+import org.apache.geode.internal.cache.BackupUtil;
import org.apache.geode.internal.cache.persistence.PersistentMemberPattern;
import org.apache.geode.internal.i18n.LocalizedStrings;
import org.apache.geode.internal.logging.InternalLogWriter;
@@ -38,12 +39,12 @@ import org.apache.geode.internal.logging.log4j.LogMarker;
import org.apache.geode.internal.logging.log4j.LogWriterAppender;
import org.apache.geode.internal.logging.log4j.LogWriterAppenders;
import org.apache.geode.internal.util.concurrent.FutureResult;
+
import org.apache.logging.log4j.Logger;
import java.io.File;
import java.io.IOException;
import java.net.InetAddress;
-import java.text.SimpleDateFormat;
import java.util.*;
import java.util.concurrent.*;
@@ -2311,39 +2312,8 @@ public class AdminDistributedSystemImpl implements org.apache.geode.admin.AdminD
public static BackupStatus backupAllMembers(DM dm, File targetDir, File baselineDir)
throws AdminException {
- BackupStatus status = null;
- if (BackupDataStoreHelper.obtainLock(dm)) {
- try {
- Set<PersistentID> missingMembers = getMissingPersistentMembers(dm);
- Set recipients = dm.getOtherDistributionManagerIds();
-
- SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd-HH-mm-ss");
- targetDir = new File(targetDir, format.format(new Date()));
- BackupDataStoreResult result =
- BackupDataStoreHelper.backupAllMembers(dm, recipients, targetDir, baselineDir);
-
- // It's possible that when calling getMissingPersistentMembers, some members are
- // still creating/recovering regions, and at FinishBackupRequest.send, the
- // regions at the members are ready. Logically, since the members in successfulMembers
- // should override the previous missingMembers
- for (Set<PersistentID> onlineMembersIds : result.getSuccessfulMembers().values()) {
- missingMembers.removeAll(onlineMembersIds);
- }
-
- result.getExistingDataStores().keySet().removeAll(result.getSuccessfulMembers().keySet());
- for (Set<PersistentID> lostMembersIds : result.getExistingDataStores().values()) {
- missingMembers.addAll(lostMembersIds);
- }
-
- status = new BackupStatusImpl(result.getSuccessfulMembers(), missingMembers);
- } finally {
- BackupDataStoreHelper.releaseLock(dm);
- }
- } else {
- throw new AdminException(
- LocalizedStrings.DistributedSystem_BACKUP_ALREADY_IN_PROGRESS.toLocalizedString());
- }
- return status;
+ return (org.apache.geode.admin.BackupStatus) BackupUtil.backupAllMembers(dm, targetDir,
+ baselineDir);
}
public Map<DistributedMember, Set<PersistentID>> compactAllDiskStores() throws AdminException {
diff --git a/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java b/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java
index 4256c3c..f915fd4 100644
--- a/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java
@@ -14,46 +14,17 @@
*/
package org.apache.geode.admin.internal;
-import java.io.Serializable;
import java.util.Map;
import java.util.Set;
-import org.apache.geode.admin.BackupStatus;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
-/**
- * Holds the result of a backup operation.
- *
- *
- */
-public class BackupStatusImpl implements BackupStatus, Serializable {
- private static final long serialVersionUID = 3704162840296921840L;
-
- private Map<DistributedMember, Set<PersistentID>> backedUpDiskStores;
- private Set<PersistentID> offlineDiskStores;
+public class BackupStatusImpl extends org.apache.geode.management.internal.BackupStatusImpl {
+ private static final long serialVersionUID = 3704162840296921841L;
public BackupStatusImpl(Map<DistributedMember, Set<PersistentID>> backedUpDiskStores,
Set<PersistentID> offlineDiskStores) {
- super();
- this.backedUpDiskStores = backedUpDiskStores;
- this.offlineDiskStores = offlineDiskStores;
- }
-
- public Map<DistributedMember, Set<PersistentID>> getBackedUpDiskStores() {
- return backedUpDiskStores;
- }
-
- public Set<PersistentID> getOfflineDiskStores() {
- return offlineDiskStores;
- }
-
- @Override
- public String toString() {
- return "BackupStatus[backedUpDiskStores=" + backedUpDiskStores + ", offlineDiskStores="
- + offlineDiskStores + "]";
+ super(backedUpDiskStores, offlineDiskStores);
}
-
-
-
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java b/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
index e431012..8f83792 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java
@@ -65,7 +65,6 @@ import org.apache.geode.SystemFailure;
import org.apache.geode.UncreatedSystemException;
import org.apache.geode.UnstartedSystemException;
import org.apache.geode.admin.AdminException;
-import org.apache.geode.admin.BackupStatus;
import org.apache.geode.admin.internal.AdminDistributedSystemImpl;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
@@ -76,6 +75,7 @@ import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.distributed.internal.tcpserver.TcpClient;
import org.apache.geode.internal.admin.remote.TailLogResponse;
+import org.apache.geode.internal.cache.BackupUtil;
import org.apache.geode.internal.cache.DiskStoreImpl;
import org.apache.geode.internal.i18n.LocalizedStrings;
import org.apache.geode.internal.logging.DateFormatter;
@@ -87,6 +87,7 @@ import org.apache.geode.internal.statistics.StatArchiveReader.StatValue;
import org.apache.geode.internal.util.JavaCommandBuilder;
import org.apache.geode.internal.util.PluckStacks;
import org.apache.geode.internal.util.PluckStacks.ThreadStack;
+import org.apache.geode.management.BackupStatus;
/**
* Provides static methods for various system administation tasks.
@@ -618,9 +619,9 @@ public class SystemAdmin {
InternalDistributedSystem ads = getAdminCnx();
// Baseline directory should be null if it was not provided on the command line
- BackupStatus status = AdminDistributedSystemImpl.backupAllMembers(ads.getDistributionManager(),
- new File(targetDir),
- (SystemAdmin.baselineDir == null ? null : new File(SystemAdmin.baselineDir)));
+ BackupStatus status =
+ BackupUtil.backupAllMembers(ads.getDistributionManager(), new File(targetDir),
+ (SystemAdmin.baselineDir == null ? null : new File(SystemAdmin.baselineDir)));
boolean incomplete = !status.getOfflineDiskStores().isEmpty();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BackupUtil.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BackupUtil.java
new file mode 100644
index 0000000..ec6ccf7
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BackupUtil.java
@@ -0,0 +1,75 @@
+/*
+ * 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;
+
+import java.io.File;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.Set;
+
+import org.apache.geode.admin.internal.AdminDistributedSystemImpl;
+import org.apache.geode.admin.internal.BackupDataStoreHelper;
+import org.apache.geode.admin.internal.BackupDataStoreResult;
+import org.apache.geode.cache.persistence.PersistentID;
+import org.apache.geode.distributed.internal.DM;
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.management.BackupStatus;
+import org.apache.geode.management.internal.BackupStatusImpl;
+import org.apache.geode.management.ManagementException;
+
+public class BackupUtil {
+
+ private BackupUtil() {
+ // do not instantiate
+ }
+
+ public static BackupStatus backupAllMembers(DM dm, File targetDir, File baselineDir)
+ throws ManagementException {
+ BackupStatus status = null;
+ if (BackupDataStoreHelper.obtainLock(dm)) {
+ try {
+ Set<PersistentID> missingMembers =
+ AdminDistributedSystemImpl.getMissingPersistentMembers(dm);
+ Set recipients = dm.getOtherDistributionManagerIds();
+
+ SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd-HH-mm-ss");
+ targetDir = new File(targetDir, format.format(new Date()));
+ BackupDataStoreResult result =
+ BackupDataStoreHelper.backupAllMembers(dm, recipients, targetDir, baselineDir);
+
+ // It's possible that when calling getMissingPersistentMembers, some members are
+ // still creating/recovering regions, and at FinishBackupRequest.send, the
+ // regions at the members are ready. Logically, since the members in successfulMembers
+ // should override the previous missingMembers
+ for (Set<PersistentID> onlineMembersIds : result.getSuccessfulMembers().values()) {
+ missingMembers.removeAll(onlineMembersIds);
+ }
+
+ result.getExistingDataStores().keySet().removeAll(result.getSuccessfulMembers().keySet());
+ for (Set<PersistentID> lostMembersIds : result.getExistingDataStores().values()) {
+ missingMembers.addAll(lostMembersIds);
+ }
+ status = new BackupStatusImpl(result.getSuccessfulMembers(), missingMembers);
+ } finally {
+ BackupDataStoreHelper.releaseLock(dm);
+ }
+
+ } else {
+ throw new ManagementException(
+ LocalizedStrings.DistributedSystem_BACKUP_ALREADY_IN_PROGRESS.toLocalizedString());
+ }
+ return status;
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java b/geode-core/src/main/java/org/apache/geode/management/BackupStatus.java
similarity index 85%
copy from geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java
copy to geode-core/src/main/java/org/apache/geode/management/BackupStatus.java
index c696b07..28795d1 100644
--- a/geode-core/src/main/java/org/apache/geode/admin/BackupStatus.java
+++ b/geode-core/src/main/java/org/apache/geode/management/BackupStatus.java
@@ -12,22 +12,21 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-package org.apache.geode.admin;
+package org.apache.geode.management;
import java.util.Map;
import java.util.Set;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.DM;
+import org.apache.geode.internal.cache.BackupUtil;
/**
* The status of a backup operation, returned by
- * {@link AdminDistributedSystem#backupAllMembers(java.io.File,java.io.File)}.
+ * {@link BackupUtil#backupAllMembers(DM, java.io.File,java.io.File)}.
*
* @since GemFire 6.5
- * @deprecated as of 7.0 use the <code><a href=
- * "{@docRoot}/org/apache/geode/management/package-summary.html">management</a></code>
- * package instead
*/
public interface BackupStatus {
diff --git a/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java b/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java
index 3cac48f..c4b824f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java
+++ b/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java
@@ -23,47 +23,16 @@ import org.apache.geode.cache.persistence.PersistentID;
*
* @since GemFire 7.0
*/
-public class DiskBackupStatus {
-
- /**
- * Map of DistributedMember, Set<PersistentID>
- */
- private Map<String, String[]> backedUpDiskStores;
-
- /**
- * List of offline disk stores
- */
- private String[] offlineDiskStores;
-
+public interface DiskBackupStatus {
/**
* Returns a map of member names/IDs and the {@link PersistentID} of the disk stores that were
* backed up.
*/
- public Map<String, String[]> getBackedUpDiskStores() {
- return backedUpDiskStores;
- }
+ Map<String, String[]> getBackedUpDiskStores();
/**
* Returns a list of directories for the disk stores that were off-line at the time the backup
* occurred.
*/
- public String[] getOfflineDiskStores() {
- return offlineDiskStores;
- }
-
- /**
- * Sets the map of member names/IDs and the {@link PersistentID} of the disk stores that were
- * backed up.
- */
- public void setBackedUpDiskStores(Map<String, String[]> backedUpDiskStores) {
- this.backedUpDiskStores = backedUpDiskStores;
- }
-
- /**
- * Sets the list of directories for the disk stores that were off-line at the time the backup
- * occurred.
- */
- public void setOfflineDiskStores(String[] offlineDiskStores) {
- this.offlineDiskStores = offlineDiskStores;
- }
+ String[] getOfflineDiskStores();
}
diff --git a/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java b/geode-core/src/main/java/org/apache/geode/management/internal/BackupStatusImpl.java
similarity index 92%
copy from geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java
copy to geode-core/src/main/java/org/apache/geode/management/internal/BackupStatusImpl.java
index 4256c3c..3cc29b6 100644
--- a/geode-core/src/main/java/org/apache/geode/admin/internal/BackupStatusImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/BackupStatusImpl.java
@@ -12,15 +12,15 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-package org.apache.geode.admin.internal;
+package org.apache.geode.management.internal;
import java.io.Serializable;
import java.util.Map;
import java.util.Set;
-import org.apache.geode.admin.BackupStatus;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.BackupStatus;
/**
* Holds the result of a backup operation.
@@ -28,7 +28,7 @@ import org.apache.geode.distributed.DistributedMember;
*
*/
public class BackupStatusImpl implements BackupStatus, Serializable {
- private static final long serialVersionUID = 3704162840296921840L;
+ private static final long serialVersionUID = 3704172840296221840L;
private Map<DistributedMember, Set<PersistentID>> backedUpDiskStores;
private Set<PersistentID> offlineDiskStores;
diff --git a/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java b/geode-core/src/main/java/org/apache/geode/management/internal/DiskBackupStatusImpl.java
similarity index 52%
copy from geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java
copy to geode-core/src/main/java/org/apache/geode/management/internal/DiskBackupStatusImpl.java
index 3cac48f..9650ca3 100644
--- a/geode-core/src/main/java/org/apache/geode/management/DiskBackupStatus.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/DiskBackupStatusImpl.java
@@ -12,18 +12,18 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-package org.apache.geode.management;
+package org.apache.geode.management.internal;
+import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.geode.cache.persistence.PersistentID;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.DiskBackupStatus;
-/**
- * Composite data type used to distribute the status of disk backup operations.
- *
- * @since GemFire 7.0
- */
-public class DiskBackupStatus {
+public class DiskBackupStatusImpl implements DiskBackupStatus {
/**
* Map of DistributedMember, Set<PersistentID>
@@ -35,18 +35,12 @@ public class DiskBackupStatus {
*/
private String[] offlineDiskStores;
- /**
- * Returns a map of member names/IDs and the {@link PersistentID} of the disk stores that were
- * backed up.
- */
+ @Override
public Map<String, String[]> getBackedUpDiskStores() {
return backedUpDiskStores;
}
- /**
- * Returns a list of directories for the disk stores that were off-line at the time the backup
- * occurred.
- */
+ @Override
public String[] getOfflineDiskStores() {
return offlineDiskStores;
}
@@ -63,7 +57,37 @@ public class DiskBackupStatus {
* Sets the list of directories for the disk stores that were off-line at the time the backup
* occurred.
*/
- public void setOfflineDiskStores(String[] offlineDiskStores) {
- this.offlineDiskStores = offlineDiskStores;
+ public void setOfflineDiskStores(String[] offLineDiskStores) {
+ this.offlineDiskStores = offLineDiskStores;
+ }
+
+ /**
+ * Sets the map of member names/IDs and the {@link PersistentID} of the disk stores that were
+ * backed up.
+ */
+ public void generateBackedUpDiskStores(
+ Map<DistributedMember, Set<PersistentID>> backedUpDiskStores) {
+ Map<String, String[]> diskStores = new HashMap<>();
+ backedUpDiskStores.entrySet().forEach(entry -> {
+ DistributedMember member = entry.getKey();
+ Set<PersistentID> ids = entry.getValue();
+ String[] setOfDiskStr = new String[ids.size()];
+ entry.getValue().stream().map(id -> id.getDirectory()).collect(Collectors.toList())
+ .toArray(setOfDiskStr);
+ diskStores.put(member.getId(), setOfDiskStr);
+ });
+ setBackedUpDiskStores(diskStores);
+ }
+
+ /**
+ * Sets the list of directories for the disk stores that were off-line at the time the backup
+ * occurred.
+ */
+ public void generateOfflineDiskStores(Set<PersistentID> offLineDiskStores) {
+ String[] diskStores = new String[offLineDiskStores.size()];
+ offLineDiskStores.stream().map(id -> id.getDirectory()).collect(Collectors.toList())
+ .toArray(diskStores);
+ setOfflineDiskStores(diskStores);
}
}
+
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
index 770695a..681b2b5 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
@@ -52,6 +52,7 @@ import org.apache.geode.management.NetworkMetrics;
import org.apache.geode.management.OSMetrics;
import org.apache.geode.management.PersistentMemberDetails;
import org.apache.geode.management.RegionMXBean;
+import org.apache.geode.management.internal.DiskBackupStatusImpl;
import org.apache.geode.management.internal.FederationComponent;
import org.apache.geode.management.internal.MBeanJMXAdapter;
import org.apache.geode.management.internal.ManagementConstants;
@@ -81,6 +82,8 @@ import java.util.SortedSet;
import java.util.TreeSet;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
import javax.management.InstanceNotFoundException;
import javax.management.ListenerNotFoundException;
import javax.management.MBeanServer;
@@ -497,26 +500,15 @@ public class DistributedSystemBridge {
}
DM dm = cache.getDistributionManager();
- Set<PersistentID> missingMembers = MissingPersistentIDsRequest.send(dm);
Set recipients = dm.getOtherDistributionManagerIds();
BackupDataStoreResult result =
BackupDataStoreHelper.backupAllMembers(dm, recipients, targetDir, baselineDir);
- Iterator<DistributedMember> it = result.getSuccessfulMembers().keySet().iterator();
-
- Map<String, String[]> backedUpDiskStores = new HashMap<>();
- while (it.hasNext()) {
- DistributedMember member = it.next();
- Set<PersistentID> setOfDisk = result.getSuccessfulMembers().get(member);
- String[] setOfDiskStr = new String[setOfDisk.size()];
- int j = 0;
- for (PersistentID id : setOfDisk) {
- setOfDiskStr[j] = id.getDirectory();
- j++;
- }
- backedUpDiskStores.put(member.getId(), setOfDiskStr);
- }
+
+ DiskBackupStatusImpl diskBackupStatus = new DiskBackupStatusImpl();
+ Map<DistributedMember, Set<PersistentID>> successfulMembers = result.getSuccessfulMembers();
+ diskBackupStatus.generateBackedUpDiskStores(successfulMembers);
// It's possible that when calling getMissingPersistentMembers, some
// members
@@ -525,27 +517,13 @@ public class DistributedSystemBridge {
// regions at the members are ready. Logically, since the members in
// successfulMembers
// should override the previous missingMembers
- for (Set<PersistentID> onlineMembersIds : result.getSuccessfulMembers().values()) {
- missingMembers.removeAll(onlineMembersIds);
- }
-
- result.getExistingDataStores().keySet().removeAll(result.getSuccessfulMembers().keySet());
- String[] setOfMissingDiskStr = null;
-
- if (result.getExistingDataStores().size() > 0) {
- setOfMissingDiskStr = new String[result.getExistingDataStores().size()];
- int j = 0;
- for (Set<PersistentID> lostMembersIds : result.getExistingDataStores().values()) {
- for (PersistentID id : lostMembersIds) {
- setOfMissingDiskStr[j] = id.getDirectory();
- j++;
- }
- }
- }
+ Set<PersistentID> successfulIds = result.getSuccessfulMembers().values().stream()
+ .flatMap(Set::stream).collect(Collectors.toSet());
+ Set<PersistentID> missingIds =
+ result.getExistingDataStores().values().stream().flatMap(Set::stream)
+ .filter((v) -> !successfulIds.contains(v)).collect(Collectors.toSet());
- DiskBackupStatus diskBackupStatus = new DiskBackupStatus();
- diskBackupStatus.setBackedUpDiskStores(backedUpDiskStores);
- diskBackupStatus.setOfflineDiskStores(setOfMissingDiskStr);
+ diskBackupStatus.generateOfflineDiskStores(missingIds);
return diskBackupStatus;
} finally {
BackupDataStoreHelper.releaseLock(dm);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/BackupDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/BackupDiskStoreCommand.java
index 6fc5df1..26f7d8b 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/BackupDiskStoreCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/BackupDiskStoreCommand.java
@@ -22,12 +22,12 @@ import java.util.Set;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
-import org.apache.geode.admin.BackupStatus;
-import org.apache.geode.admin.internal.AdminDistributedSystemImpl;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.internal.DM;
+import org.apache.geode.internal.cache.BackupUtil;
import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.BackupStatus;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
@@ -61,10 +61,9 @@ public class BackupDiskStoreCommand implements GfshCommand {
BackupStatus backupStatus;
if (baselineDir != null && !baselineDir.isEmpty()) {
- backupStatus = AdminDistributedSystemImpl.backupAllMembers(dm, new File(targetDir),
- new File(baselineDir));
+ backupStatus = BackupUtil.backupAllMembers(dm, new File(targetDir), new File(baselineDir));
} else {
- backupStatus = AdminDistributedSystemImpl.backupAllMembers(dm, new File(targetDir), null);
+ backupStatus = BackupUtil.backupAllMembers(dm, new File(targetDir), null);
}
Map<DistributedMember, Set<PersistentID>> backedupMemberDiskstoreMap =
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/BackupDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/BackupDUnitTest.java
index bcbeff6..d3445df 100755
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/BackupDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/BackupDUnitTest.java
@@ -20,7 +20,6 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import org.apache.commons.io.FileUtils;
-import org.apache.geode.admin.BackupStatus;
import org.apache.geode.admin.internal.PrepareBackupRequest;
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.DataPolicy;
@@ -40,6 +39,7 @@ import org.apache.geode.distributed.internal.DistributionMessageObserver;
import org.apache.geode.distributed.internal.ReplyMessage;
import org.apache.geode.internal.admin.remote.AdminFailureResponse;
import org.apache.geode.internal.cache.partitioned.PersistentPartitionedRegionTestBase;
+import org.apache.geode.management.BackupStatus;
import org.apache.geode.test.dunit.Assert;
import org.apache.geode.test.dunit.AsyncInvocation;
import org.apache.geode.test.dunit.DUnitEnv;
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
index 08fecc2..2c831c1 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
@@ -38,10 +38,6 @@ import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.apache.geode.admin.AdminDistributedSystem;
-import org.apache.geode.admin.AdminDistributedSystemFactory;
-import org.apache.geode.admin.AdminException;
-import org.apache.geode.admin.BackupStatus;
-import org.apache.geode.admin.DistributedSystemConfig;
import org.apache.geode.admin.internal.AdminDistributedSystemImpl;
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.CacheFactory;
@@ -52,11 +48,14 @@ import org.apache.geode.cache.RegionShortcut;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.test.compiler.ClassBuilder;
+import org.apache.geode.distributed.internal.DM;
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.internal.DeployedJar;
import org.apache.geode.internal.util.IOUtils;
import org.apache.geode.internal.util.TransformUtils;
+import org.apache.geode.management.BackupStatus;
+import org.apache.geode.management.ManagementException;
+import org.apache.geode.test.compiler.ClassBuilder;
import org.apache.geode.test.dunit.Host;
import org.apache.geode.test.dunit.LogWriterUtils;
import org.apache.geode.test.dunit.SerializableCallable;
@@ -200,7 +199,7 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
}
/**
- * Invokes {@link AdminDistributedSystem#backupAllMembers(File)} on a member.
+ * Invokes {@link BackupUtil#backupAllMembers(DM, File, File)} on a member.
*
* @param vm a member of the distributed system
* @return the status of the backup.
@@ -209,28 +208,20 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
return (BackupStatus) vm.invoke(new SerializableCallable("Backup all members.") {
@Override
public Object call() {
- DistributedSystemConfig config;
- AdminDistributedSystem adminDS = null;
try {
- config = AdminDistributedSystemFactory.defineDistributedSystem(getSystem(), "");
- adminDS = AdminDistributedSystemFactory.getDistributedSystem(config);
- adminDS.connect();
- return adminDS.backupAllMembers(getBaselineDir());
+ return BackupUtil.backupAllMembers(getSystem().getDistributionManager(), getBaselineDir(),
+ null);
- } catch (AdminException e) {
+ } catch (ManagementException e) {
throw new RuntimeException(e);
- } finally {
- if (adminDS != null) {
- adminDS.disconnect();
- }
}
}
});
}
/**
- * Invokes {@link AdminDistributedSystem#backupAllMembers(File, File)} on a member.
- *
+ * Invokes {@link BackupUtil#backupAllMembers(DM, File, File)} on a member.
+ *
* @param vm a member of the distributed system.
* @return a status of the backup operation.
*/
@@ -238,28 +229,20 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
return (BackupStatus) vm.invoke(new SerializableCallable("Backup all members.") {
@Override
public Object call() {
- DistributedSystemConfig config;
- AdminDistributedSystem adminDS = null;
try {
- config = AdminDistributedSystemFactory.defineDistributedSystem(getSystem(), "");
- adminDS = AdminDistributedSystemFactory.getDistributedSystem(config);
- adminDS.connect();
- return adminDS.backupAllMembers(getIncrementalDir(), getBaselineBackupDir());
+ return BackupUtil.backupAllMembers(getSystem().getDistributionManager(),
+ getIncrementalDir(), getBaselineBackupDir());
- } catch (AdminException e) {
+ } catch (ManagementException e) {
throw new RuntimeException(e);
- } finally {
- if (adminDS != null) {
- adminDS.disconnect();
- }
}
}
});
}
/**
- * Invokes {@link AdminDistributedSystem#backupAllMembers(File, File)} on a member.
- *
+ * Invokes {@link BackupUtil#backupAllMembers(DM, File, File)} on a member.
+ *
* @param vm a member of the distributed system.
* @return a status of the backup operation.
*/
@@ -267,20 +250,12 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
return (BackupStatus) vm.invoke(new SerializableCallable("Backup all members.") {
@Override
public Object call() {
- DistributedSystemConfig config;
- AdminDistributedSystem adminDS = null;
try {
- config = AdminDistributedSystemFactory.defineDistributedSystem(getSystem(), "");
- adminDS = AdminDistributedSystemFactory.getDistributedSystem(config);
- adminDS.connect();
- return adminDS.backupAllMembers(getIncremental2Dir(), getIncrementalBackupDir());
+ return BackupUtil.backupAllMembers(getSystem().getDistributionManager(),
+ getIncremental2Dir(), getIncrementalBackupDir());
- } catch (AdminException e) {
+ } catch (ManagementException e) {
throw new RuntimeException(e);
- } finally {
- if (adminDS != null) {
- adminDS.disconnect();
- }
}
}
});
@@ -995,20 +970,12 @@ public class IncrementalBackupDUnitTest extends JUnit4CacheTestCase {
@Override
public Object call() {
- AdminDistributedSystem adminDS = null;
try {
- DistributedSystemConfig config =
- AdminDistributedSystemFactory.defineDistributedSystem(getSystem(), "");
- adminDS = AdminDistributedSystemFactory.getDistributedSystem(config);
- adminDS.connect();
- return adminDS.backupAllMembers(getIncrementalDir(), this.baselineDir);
+ return BackupUtil.backupAllMembers(getSystem().getDistributionManager(),
+ getIncrementalDir(), this.baselineDir);
- } catch (AdminException e) {
+ } catch (ManagementException e) {
throw new RuntimeException(e);
- } finally {
- if (adminDS != null) {
- adminDS.disconnect();
- }
}
}
};
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java
index 592bc44..ded783c 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java
@@ -22,7 +22,6 @@ import org.apache.commons.io.filefilter.RegexFileFilter;
import org.apache.geode.admin.AdminDistributedSystem;
import org.apache.geode.admin.AdminDistributedSystemFactory;
import org.apache.geode.admin.AdminException;
-import org.apache.geode.admin.BackupStatus;
import org.apache.geode.admin.DistributedSystemConfig;
import org.apache.geode.cache.AttributesFactory;
import org.apache.geode.cache.Cache;
@@ -37,6 +36,7 @@ import org.apache.geode.cache.partition.PartitionRegionInfo;
import org.apache.geode.cache.persistence.ConflictingPersistentDataException;
import org.apache.geode.cache.persistence.PersistentID;
import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.BackupUtil;
import org.apache.geode.internal.cache.DiskRegion;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.PartitionedRegion;
@@ -46,6 +46,8 @@ import org.apache.geode.internal.cache.control.InternalResourceManager.ResourceO
import org.apache.geode.internal.cache.persistence.PersistenceAdvisor;
import org.apache.geode.internal.cache.persistence.PersistenceAdvisorImpl;
import org.apache.geode.internal.cache.persistence.PersistentMemberID;
+import org.apache.geode.management.BackupStatus;
+import org.apache.geode.management.ManagementException;
import org.apache.geode.test.dunit.Assert;
import org.apache.geode.test.dunit.AsyncInvocation;
import org.apache.geode.test.dunit.Invoke;
@@ -56,7 +58,6 @@ import org.apache.geode.test.dunit.VM;
import org.apache.geode.test.dunit.Wait;
import org.apache.geode.test.dunit.WaitCriterion;
import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
-import org.awaitility.Awaitility;
import java.io.BufferedReader;
import java.io.File;
@@ -837,21 +838,11 @@ public abstract class PersistentPartitionedRegionTestBase extends JUnit4CacheTes
return (BackupStatus) vm.invoke(new SerializableCallable("Backup all members") {
public Object call() {
- DistributedSystemConfig config;
- AdminDistributedSystem adminDS = null;
try {
- config = AdminDistributedSystemFactory.defineDistributedSystem(getSystem(), "");
- adminDS = AdminDistributedSystemFactory.getDistributedSystem(config);
- adminDS.connect();
- adminDS.waitToBeConnected(MAX_WAIT);
- return adminDS.backupAllMembers(getBackupDir());
-
- } catch (Exception e) {
+ return BackupUtil.backupAllMembers(getSystem().getDistributionManager(), getBackupDir(),
+ null);
+ } catch (ManagementException e) {
throw new RuntimeException(e);
- } finally {
- if (adminDS != null) {
- adminDS.disconnect();
- }
}
}
});
diff --git a/geode-core/src/test/java/org/apache/geode/management/DiskBackupStatusTest.java b/geode-core/src/test/java/org/apache/geode/management/DiskBackupStatusTest.java
new file mode 100644
index 0000000..867fc69
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/DiskBackupStatusTest.java
@@ -0,0 +1,105 @@
+/*
+ * 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.management;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.persistence.PersistentID;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.internal.DiskBackupStatusImpl;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class DiskBackupStatusTest {
+
+ @Test
+ public void valuesNotSetTest() {
+ DiskBackupStatusImpl status = new DiskBackupStatusImpl();
+ assertThat(status.getBackedUpDiskStores()).isNull();
+ assertThat(status.getOfflineDiskStores()).isNull();
+ }
+
+ @Test
+ public void returnsSetValues() {
+ DiskBackupStatusImpl status = new DiskBackupStatusImpl();
+ String[] testOfflineDiskStores = new String[] {"test", "array"};
+ status.setOfflineDiskStores(testOfflineDiskStores);
+ assertThat(status.getOfflineDiskStores()).isEqualTo(testOfflineDiskStores);
+
+ Map<String, String[]> testBackedUpDiskStores = new HashMap<>();
+ testBackedUpDiskStores.put("key1", new String[] {"value1"});
+ status.setBackedUpDiskStores(testBackedUpDiskStores);
+ assertThat(status.getBackedUpDiskStores()).isEqualTo(testBackedUpDiskStores);
+ }
+
+ @Test
+ public void generatesCorrectBackupUpDiskStores() {
+ Map<DistributedMember, Set<PersistentID>> backedUpDiskStores = new HashMap<>();
+
+ DistributedMember member1 = generateTestMember("member1");
+ Set<PersistentID> idSet1 = generateTestIDs(1);
+ backedUpDiskStores.put(member1, idSet1);
+
+ DistributedMember member2 = generateTestMember("member2");
+ Set<PersistentID> idSet2 = generateTestIDs(2);
+ backedUpDiskStores.put(member2, idSet2);
+
+ DiskBackupStatusImpl status = new DiskBackupStatusImpl();
+ status.generateBackedUpDiskStores(backedUpDiskStores);
+
+ Map<String, String[]> storedDiskStores = status.getBackedUpDiskStores();
+ assertThat(storedDiskStores).containsOnlyKeys("member1", "member2");
+ assertThat(storedDiskStores.get("member1").length).isEqualTo(1);
+ assertThat(storedDiskStores.get("member2").length).isEqualTo(2);
+ assertThat(storedDiskStores.get("member2")).contains("DirectoryForId0", "DirectoryForId1");
+ }
+
+ @Test
+ public void generatesCorrectOfflineDiskStores() {
+ Set<PersistentID> ids = generateTestIDs(2);
+ DiskBackupStatusImpl status = new DiskBackupStatusImpl();
+ status.generateOfflineDiskStores(ids);
+
+ String[] storedIds = status.getOfflineDiskStores();
+ assertThat(storedIds.length).isEqualTo(2);
+ assertThat(storedIds).contains("DirectoryForId0", "DirectoryForId1");
+ }
+
+ private DistributedMember generateTestMember(String name) {
+ DistributedMember member = mock(DistributedMember.class);
+ when(member.getId()).thenReturn(name);
+ return member;
+ }
+
+ private Set<PersistentID> generateTestIDs(int idsToGenerate) {
+ Set<PersistentID> ids = new HashSet<>();
+ for (int i = 0; i < idsToGenerate; i++) {
+ PersistentID id = mock(PersistentID.class);
+ when(id.getDirectory()).thenReturn("DirectoryForId" + i);
+ ids.add(id);
+ }
+ return ids;
+ }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/DiskManagementDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/DiskManagementDUnitTest.java
index 724fad7..2a628fe 100644
--- a/geode-core/src/test/java/org/apache/geode/management/DiskManagementDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/DiskManagementDUnitTest.java
@@ -293,7 +293,7 @@ public class DiskManagementDUnitTest implements Serializable {
DiskBackupStatus status = bean.backupAllMembers(backupDir.getAbsolutePath(), null);
assertThat(status.getBackedUpDiskStores().keySet().size()).isEqualTo(memberCount);
- assertThat(status.getOfflineDiskStores()).isEqualTo(null); // TODO: fix GEODE-1946
+ assertThat(status.getOfflineDiskStores()).isEmpty(); // TODO: fix GEODE-1946
});
}
diff --git a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
index 76bf0ab..3085c11 100644
--- a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
+++ b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
@@ -161,3 +161,5 @@ org/apache/geode/internal/security/shiro/GeodeAuthenticationToken
org/apache/geode/internal/cache/InitialImageOperation$GIITestHook
org/apache/geode/internal/AvailablePort$Keeper
org/apache/geode/internal/admin/remote/DistributionLocatorId
+org/apache/geode/admin/internal/BackupStatusImpl
+org/apache/geode/management/internal/BackupStatusImpl
--
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].