From bf805d1483c1ec11982c04db4367b761f6b47abe Mon Sep 17 00:00:00 2001 From: Nathan Johnson Date: Mon, 4 Feb 2019 15:08:42 -0600 Subject: [PATCH] Add back ability to disable backup of snapshot to secondary (#3122) * The snapshot.backup.rightafter configuration variable was removed by: SHA: 6bb0ca2f854 This adds it back, though named snapshot.backup.to.secondary now instead. This global parameter, once set, will allow you to prevent automatic backups of snapshots to secondary storage, unless they're actually needed. Fixes #3096 * updates per review --- .../subsystem/api/storage/SnapshotInfo.java | 3 +++ .../storage/snapshot/SnapshotObject.java | 11 ++++++++ .../storage/snapshot/SnapshotManager.java | 3 +++ .../storage/snapshot/SnapshotManagerImpl.java | 26 ++++++++++++++----- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java index 5541b362547..ef72afc5b77 100644 --- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java +++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.storage.Snapshot; +import com.cloud.utils.exception.CloudRuntimeException; public interface SnapshotInfo extends DataObject, Snapshot { SnapshotInfo getParent(); @@ -42,4 +43,6 @@ public interface SnapshotInfo extends DataObject, Snapshot { boolean isRevertable(); long getPhysicalSize(); + + void markBackedUp() throws CloudRuntimeException; } diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java index 23e1650ffa0..65d6fa52e66 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java @@ -149,6 +149,17 @@ public class SnapshotObject implements SnapshotInfo { return physicalSize; } + @Override + public void markBackedUp() throws CloudRuntimeException{ + try { + processEvent(Event.OperationNotPerformed); + } catch (NoTransitionException ex) { + s_logger.error("no transition error: ", ex); + throw new CloudRuntimeException("Error marking snapshot backed up: " + + this.snapshot.getId() + " " + ex.getMessage()); + } + } + @Override public VolumeInfo getBaseVolume() { return volFactory.getVolume(snapshot.getVolumeId()); diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManager.java b/server/src/com/cloud/storage/snapshot/SnapshotManager.java index f3557d0a670..407ffa30213 100644 --- a/server/src/com/cloud/storage/snapshot/SnapshotManager.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManager.java @@ -56,6 +56,9 @@ public interface SnapshotManager extends Configurable { public static final ConfigKey BackupRetryInterval = new ConfigKey(Integer.class, "backup.retry.interval", "Advanced", "300", "Time in seconds between retries in backing up snapshot to secondary", false, ConfigKey.Scope.Global, null); + public static final ConfigKey BackupSnapshotAfterTakingSnapshot = new ConfigKey(Boolean.class, "snapshot.backup.to.secondary", "Snapshots", "true", + "Indicates whether to always backup primary storage snapshot to secondary storage", false, ConfigKey.Scope.Global, null); + void deletePoliciesForVolume(Long volumeId); /** diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index bd49c05f43e..07eb072e82c 100755 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -206,7 +206,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {BackupRetryAttempts, BackupRetryInterval, SnapshotHourlyMax, SnapshotDailyMax, SnapshotMonthlyMax, SnapshotWeeklyMax, usageSnapshotSelection}; + return new ConfigKey[] {BackupRetryAttempts, BackupRetryInterval, SnapshotHourlyMax, SnapshotDailyMax, SnapshotMonthlyMax, SnapshotWeeklyMax, usageSnapshotSelection, BackupSnapshotAfterTakingSnapshot}; } @Override @@ -1123,13 +1123,16 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } SnapshotInfo snapshotOnPrimary = snapshotStrategy.takeSnapshot(snapshot); - if (payload.getAsyncBackup()) { - backupSnapshotExecutor.schedule(new BackupSnapshotTask(snapshotOnPrimary, snapshotBackupRetries - 1, snapshotStrategy), 0, TimeUnit.SECONDS); + boolean backupSnapToSecondary = BackupSnapshotAfterTakingSnapshot.value() == null || + BackupSnapshotAfterTakingSnapshot.value(); + + if (backupSnapToSecondary) { + backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary); } else { - SnapshotInfo backupedSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary); - if (backupedSnapshot != null) { - snapshotStrategy.postSnapshotCreation(snapshotOnPrimary); + if(s_logger.isDebugEnabled()) { + s_logger.debug("skipping backup of snapshot " + snapshotId + " to secondary due to configuration"); } + snapshotOnPrimary.markBackedUp(); } try { @@ -1171,6 +1174,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement return snapshot; } + protected void backupSnapshotToSecondary(boolean asyncBackup, SnapshotStrategy snapshotStrategy, SnapshotInfo snapshotOnPrimary) { + if (asyncBackup) { + backupSnapshotExecutor.schedule(new BackupSnapshotTask(snapshotOnPrimary, snapshotBackupRetries - 1, snapshotStrategy), 0, TimeUnit.SECONDS); + } else { + SnapshotInfo backupedSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary); + if (backupedSnapshot != null) { + snapshotStrategy.postSnapshotCreation(snapshotOnPrimary); + } + } + } + protected class BackupSnapshotTask extends ManagedContextRunnable { SnapshotInfo snapshot; int attempts;