From d2c2f84331ef5c17fce7c68f142dd46fb5a0d42f Mon Sep 17 00:00:00 2001 From: Vipul Bansal Date: Sun, 23 Feb 2025 05:25:17 +0000 Subject: [PATCH] [PLAT-16801][PLAT-16549]: Restrict restore of PG-15 backups on PG-11 universes and make catalog upgrade time configurable Summary: - Implemented changes to prevent the restoration of a backup from a PostgreSQL-15 universe to a PostgreSQL-11 universe. To achieve this, YBA now stores the YSQL major version of the source universe in the backup success file during backup creation and validates it against the YSQL major version of the target universe during restoration. - Additionally, introduced configurability for the catalog upgrade timeout via a public runtime configuration. The default settings have been set to 30 minutes (or 30 attempts) with a 1-minute interval between each attempt. Test Plan: - Added unit tests - Manually verified that backups created from PostgreSQL-15 universes cannot be restored on PostgreSQL-11 universes, while backups from PostgreSQL-11 universes can be successfully restored on PostgreSQL-15 universes. Reviewers: dshubin, sanketh, hsunder Reviewed By: dshubin Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D42098 --- managed/RUNTIME-FLAGS.md | 1 + .../tasks/subtasks/RestoreBackupYbc.java | 3 ++ .../RunYsqlMajorVersionCatalogUpgrade.java | 16 +++++++- .../YsqlMajorUpgradeServerTaskBase.java | 7 ++-- .../backuprestore/ybc/YbcBackupUtil.java | 41 ++++++++++++++++++- .../yw/common/config/UniverseConfKeys.java | 8 ++++ managed/src/main/resources/reference.conf | 1 + .../backuprestore/ybc/YbcBackupUtilTest.java | 34 ++++++++++++++- 8 files changed, 103 insertions(+), 8 deletions(-) diff --git a/managed/RUNTIME-FLAGS.md b/managed/RUNTIME-FLAGS.md index 9c7d92cb275d..4d5be46cc133 100644 --- a/managed/RUNTIME-FLAGS.md +++ b/managed/RUNTIME-FLAGS.md @@ -306,3 +306,4 @@ | "Queue Wait Time for Tasks" | "yb.task.queue_wait_time" | "UNIVERSE" | "Wait time for a queued task before the running task can be evicted forcefully." | "Duration" | | "Common Name Required for Certificates" | "yb.tls.cert_manager.common_name_required" | "UNIVERSE" | "If true, YBA will add commonName to the CertificateRequest sent to cert manager." | "Boolean" | | "Skip OpenTelemetry Operator Check" | "yb.universe.skip_otel_operator_check" | "UNIVERSE" | "If true, YBA will skip checking for Opentelemetry operator installation on the cluster." | "Boolean" | +| "Wait Attempts for major catalog upgrade" | "yb.upgrade.wait_attempts_for_major_catalog_upgrade" | "UNIVERSE" | "Wait Attempts for major catalog upgrade" | "Integer" | diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RestoreBackupYbc.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RestoreBackupYbc.java index 48e728c02af5..3f003a1ba137 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RestoreBackupYbc.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RestoreBackupYbc.java @@ -337,6 +337,9 @@ private void validateBackupMetadata(Universe universe) { Set tserverAutoFlags = backupConfig.tserverAutoFlags; ybcBackupUtil.validateAutoFlagCompatibility(universe, masterAutoFlags, tserverAutoFlags); } + if (backupConfig.ysqlMajorVersion != null) { + ybcBackupUtil.validateYsqlMajorVersion(universe, backupConfig.ysqlMajorVersion); + } } catch (IOException e) { log.error("Error while validating backup metadata: ", e); throw new PlatformServiceException( diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RunYsqlMajorVersionCatalogUpgrade.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RunYsqlMajorVersionCatalogUpgrade.java index d40ac90b03ab..978449589325 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RunYsqlMajorVersionCatalogUpgrade.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/RunYsqlMajorVersionCatalogUpgrade.java @@ -4,12 +4,17 @@ import com.google.inject.Inject; import com.yugabyte.yw.commissioner.BaseTaskDependencies; +import com.yugabyte.yw.common.config.RuntimeConfGetter; +import com.yugabyte.yw.common.config.UniverseConfKeys; +import com.yugabyte.yw.models.Universe; import lombok.extern.slf4j.Slf4j; import org.yb.master.MasterAdminOuterClass.YsqlMajorCatalogUpgradeState; @Slf4j public class RunYsqlMajorVersionCatalogUpgrade extends YsqlMajorUpgradeServerTaskBase { + private final RuntimeConfGetter runtimeConfGetter; + public static class Params extends YsqlMajorUpgradeServerTaskBase.Params {} @Override @@ -18,8 +23,10 @@ protected Params taskParams() { } @Inject - protected RunYsqlMajorVersionCatalogUpgrade(BaseTaskDependencies baseTaskDependencies) { + protected RunYsqlMajorVersionCatalogUpgrade( + BaseTaskDependencies baseTaskDependencies, RuntimeConfGetter runtimeConfGetter) { super(baseTaskDependencies); + this.runtimeConfGetter = runtimeConfGetter; } @Override @@ -44,7 +51,12 @@ public void run() { log.info("Starting YSQL major version catalog upgrade"); startYsqlMajorCatalogUpgrade(); } - waitForCatalogUpgradeToFinish(); + Universe universe = Universe.getOrBadRequest(taskParams().getUniverseUUID()); + int maxAttempts = + runtimeConfGetter + .getConfForScope(universe, UniverseConfKeys.waitAttemptsForMajorCatalogUpgrade) + .intValue(); + waitForCatalogUpgradeToFinish(maxAttempts); } catch (Exception e) { log.error("Error running ysql major version catalog upgrade: ", e); throw new RuntimeException(e); diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/YsqlMajorUpgradeServerTaskBase.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/YsqlMajorUpgradeServerTaskBase.java index 7992dd3cdfc4..2551c6d02d4f 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/YsqlMajorUpgradeServerTaskBase.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/YsqlMajorUpgradeServerTaskBase.java @@ -28,7 +28,6 @@ @Slf4j public abstract class YsqlMajorUpgradeServerTaskBase extends ServerSubTaskBase { - private static final int MAX_ATTEMPTS = 20; private static final int DELAY_BETWEEN_ATTEMPTS_SEC = 60; // 1 minute public static class Params extends ServerSubTaskParams {} @@ -71,14 +70,14 @@ protected void startYsqlMajorCatalogUpgrade() throws Exception { } } - protected void waitForCatalogUpgradeToFinish() throws Exception { + protected void waitForCatalogUpgradeToFinish(int maxAttempts) throws Exception { int attempts = 0; - while (attempts < MAX_ATTEMPTS) { + while (attempts < maxAttempts) { attempts++; log.debug( "Waiting for YSQL major version catalog upgrade to finish. Attempt {} of {}", attempts, - MAX_ATTEMPTS); + maxAttempts); waitFor(Duration.ofSeconds(DELAY_BETWEEN_ATTEMPTS_SEC)); try (YBClient client = getClient()) { IsYsqlMajorCatalogUpgradeDoneResponse resp = client.isYsqlMajorCatalogUpgradeDone(); diff --git a/managed/src/main/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtil.java b/managed/src/main/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtil.java index e14321947646..765f80488f0f 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtil.java +++ b/managed/src/main/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtil.java @@ -63,6 +63,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -121,6 +122,7 @@ public class YbcBackupUtil { private final CustomerConfigService configService; private final EncryptionAtRestManager encryptionAtRestManager; private final StorageUtilFactory storageUtilFactory; + private final GFlagsValidation gFlagsValidation; @Inject public YbcBackupUtil( @@ -128,12 +130,14 @@ public YbcBackupUtil( UniverseInfoHandler universeInfoHandler, CustomerConfigService configService, EncryptionAtRestManager encryptionAtRestManager, - StorageUtilFactory storageUtilFactory) { + StorageUtilFactory storageUtilFactory, + GFlagsValidation gFlagsValidation) { this.universeInfoHandler = universeInfoHandler; this.configService = configService; this.encryptionAtRestManager = encryptionAtRestManager; this.storageUtilFactory = storageUtilFactory; this.autoFlagUtil = autoFlagUtil; + this.gFlagsValidation = gFlagsValidation; } public static final Logger LOG = LoggerFactory.getLogger(YbcBackupUtil.class); @@ -251,6 +255,9 @@ public static class YbcSuccessBackupConfig { @JsonProperty("backup_uuid") public String backupUUID; + + @JsonProperty("ysql_major_version") + public String ysqlMajorVersion; } @JsonIgnoreProperties(ignoreUnknown = true) @@ -1050,6 +1057,16 @@ public BackupServiceTaskExtendedArgs getExtendedArgsForBackup(BackupTableYbc.Par config.universeKeys = universeKeyHistory.get("universe_keys"); config.masterKeyMetadata = universeKeyHistory.get("master_key_metadata"); } + + if (tableParams.backupType != null + && tableParams.backupType.equals(TableType.PGSQL_TABLE_TYPE)) { + Optional ysqlMajorVersionOptional = + gFlagsValidation.getYsqlMajorVersion(ybdbSoftwareVersion); + if (ysqlMajorVersionOptional.isPresent()) { + config.ysqlMajorVersion = String.valueOf(ysqlMajorVersionOptional.get()); + } + } + BackupServiceTaskExtendedArgs.Builder extendedArgsBuilder = BackupServiceTaskExtendedArgs.newBuilder(); ObjectMapper mapper = new ObjectMapper(); @@ -1070,6 +1087,28 @@ public BackupServiceTaskExtendedArgs getExtendedArgsForBackup(BackupTableYbc.Par } } + public void validateYsqlMajorVersion(Universe restoreUniverse, String backupYsqlMajorVersion) + throws IOException { + if (StringUtils.isEmpty(backupYsqlMajorVersion)) { + return; + } + Optional restoreUniverseYsqlMajorVersionOptional = + gFlagsValidation.getYsqlMajorVersion( + restoreUniverse.getUniverseDetails().getPrimaryCluster().userIntent.ybSoftwareVersion); + int restoreUniverseYsqlMajorVersion = + restoreUniverseYsqlMajorVersionOptional.isPresent() + ? restoreUniverseYsqlMajorVersionOptional.get() + : 11; + if (Integer.parseInt(backupYsqlMajorVersion) > restoreUniverseYsqlMajorVersion) { + throw new PlatformServiceException( + BAD_REQUEST, + String.format( + "Cannot restore backup as the backup was taken on YSQL major version %s and the" + + " universe is running on YSQL major version %s", + backupYsqlMajorVersion, restoreUniverseYsqlMajorVersion)); + } + } + /** * Verifies that universe has already promoted the provided set of master and tserver auto flags. * diff --git a/managed/src/main/java/com/yugabyte/yw/common/config/UniverseConfKeys.java b/managed/src/main/java/com/yugabyte/yw/common/config/UniverseConfKeys.java index 88666b051ecb..d8a783b004fc 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/config/UniverseConfKeys.java +++ b/managed/src/main/java/com/yugabyte/yw/common/config/UniverseConfKeys.java @@ -1490,4 +1490,12 @@ public class UniverseConfKeys extends RuntimeConfigKeysModule { "If true, YBA will skip checking for Opentelemetry operator installation on the cluster.", ConfDataType.BooleanType, ImmutableList.of(ConfKeyTags.PUBLIC)); + public static final ConfKeyInfo waitAttemptsForMajorCatalogUpgrade = + new ConfKeyInfo<>( + "yb.upgrade.wait_attempts_for_major_catalog_upgrade", + ScopeType.UNIVERSE, + "Wait Attempts for major catalog upgrade", + "Wait Attempts for major catalog upgrade", + ConfDataType.IntegerType, + ImmutableList.of(ConfKeyTags.PUBLIC)); } diff --git a/managed/src/main/resources/reference.conf b/managed/src/main/resources/reference.conf index 65d22b151a42..91a4f0ccf157 100644 --- a/managed/src/main/resources/reference.conf +++ b/managed/src/main/resources/reference.conf @@ -752,6 +752,7 @@ yb { ysql_upgrade_timeout_sec = 1800 skip_finalize = false enable_rollback_support = true + wait_attempts_for_major_catalog_upgrade = 30 } edit { wait_for_leaders_on_preferred=true diff --git a/managed/src/test/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtilTest.java b/managed/src/test/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtilTest.java index f64990070941..bdbc76fc2929 100644 --- a/managed/src/test/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtilTest.java +++ b/managed/src/test/java/com/yugabyte/yw/common/backuprestore/ybc/YbcBackupUtilTest.java @@ -53,12 +53,14 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import junitparams.JUnitParamsRunner; import junitparams.Parameters; import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -98,7 +100,8 @@ public void setup() { universeInfoHandler, configService, encryptionAtRestManager, - mockStorageUtilFactory); + mockStorageUtilFactory, + mockGFlagsValidation); initResponseObjects(); testCustomer = ModelFactory.testCustomer(); @@ -344,12 +347,14 @@ public void testGetExtendedBackupArgs(String filePath) throws Exception { tableParams.useTablespaces = true; tableParams.customerUuid = testCustomer.getUuid(); tableParams.backupUuid = UUID.randomUUID(); + tableParams.backupType = TableType.PGSQL_TABLE_TYPE; tableParams.setUniverseUUID(defaultUniverse.getUniverseUUID()); String backupKeys = TestUtils.readResource(filePath); ObjectMapper mapper = new ObjectMapper(); ObjectNode keysNode = mapper.readValue(backupKeys, ObjectNode.class); when(encryptionAtRestManager.backupUniverseKeyHistory(tableParams.getUniverseUUID())) .thenReturn(keysNode); + when(mockGFlagsValidation.getYsqlMajorVersion(any())).thenReturn(Optional.of(15)); YbcSuccessBackupConfig backupConfig = new YbcSuccessBackupConfig(); String universeVersion = defaultUniverse.getUniverseDetails().getPrimaryCluster().userIntent.ybSoftwareVersion; @@ -358,6 +363,7 @@ public void testGetExtendedBackupArgs(String filePath) throws Exception { backupConfig.masterKeyMetadata = keysNode.get("master_key_metadata"); backupConfig.backupUUID = tableParams.backupUuid.toString(); backupConfig.customerUUID = tableParams.customerUuid.toString(); + backupConfig.ysqlMajorVersion = "15"; BackupTableYbc.Params params = new BackupTableYbc.Params(tableParams, null, defaultUniverse); BackupServiceTaskExtendedArgs extArgs = ybcBackupUtil.getExtendedArgsForBackup(params); assertEquals(true, extArgs.getUseTablespaces()); @@ -1016,4 +1022,30 @@ public void testGetRestorableWindow(String filePath) { assertEquals(pitWindow.timestampRetentionWindowEndMillis, restorableWindowEndExpected); assertEquals(pitWindow.timestampRetentionWindowStartMillis, restorableWindowStartExpected); } + + @Test + @Parameters({ + "11, 11, false", + "15, 11, true", + ", , false", + "11, 15, false", + "11, , false", + "15, , true" + }) + public void testValidateYsqlMajorVersion( + String backupYsqlMajorVersion, String universeYsqlMajoVersion, boolean error) + throws IOException { + when(mockGFlagsValidation.getYsqlMajorVersion(any())) + .thenReturn( + StringUtils.isEmpty(universeYsqlMajoVersion) + ? Optional.empty() + : Optional.of(Integer.valueOf(universeYsqlMajoVersion))); + if (error) { + assertThrows( + PlatformServiceException.class, + () -> ybcBackupUtil.validateYsqlMajorVersion(defaultUniverse, backupYsqlMajorVersion)); + } else { + ybcBackupUtil.validateYsqlMajorVersion(defaultUniverse, backupYsqlMajorVersion); + } + } }