Skip to content

Commit

Permalink
[#20334] YSQL, Backups: Use ignore_existing_tablespaces in yb_backup.py
Browse files Browse the repository at this point in the history 
Summary:
Changes:
1. The new flag `--ignore_existing_tablespaces` is added into `yb_backup.py` script.
2. `ysql_dumpall` tool  is changed to dump Tablespaces with `if (ignore_existing_tablespaces)` statement.

The changes allow ignoring errors like `tablespace "region1_ts1" already exists`
when the backup-restore recreates the YSQL Tablespaces.

NOTE: "STOP_ON_ERROR" should be enabled to see the effect.
(set variable `ENABLE_STOP_ON_YSQL_DUMP_RESTORE_ERROR` into `True` in the `yb_backup.py`. Now it's `False`.)
Jira: DB-9319

Test Plan:
Set `ENABLE_STOP_ON_YSQL_DUMP_RESTORE_ERROR=True` in `yb_backup.py`.

New tests:
ybd --java-test  org.yb.pgsql.TestYbBackup#testFailureOnExistingTablespaces
ybd --java-test  org.yb.pgsql.TestYbBackup#testIgnoreExistingTablespaces
ybd --java-test  org.yb.pgsql.TestYsqlDump#ysqlDumpAllWithoutYbMetadata

Other related tests
ybd --java-test  org.yb.pgsql.TestYsqlDump#ysqlDumpAllWithYbMetadata

ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioning
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningNoRegions
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningOneRegion
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningRestoringIntoExisting
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningWithTablespaces
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningNoRegionsWithTablespaces
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningOneRegionWithTablespaces
ybd --java-test  org.yb.pgsql.TestYbBackup#testGeoPartitioningRestoringIntoExistingWithTablespaces

Reviewers: mihnea, tverona, yguan

Reviewed By: yguan

Subscribers: yugaware, yql

Differential Revision: https://phorge.dev.yugabyte.com/D30956
  • Loading branch information
@OlegLoginov
OlegLoginov committed Dec 22, 2023
1 parent 469f220 commit f6c3c86
Show file tree
Hide file tree
Showing 6 changed files with 405 additions and 14 deletions.
55 changes: 46 additions & 9 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbBackup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1239,8 +1239,9 @@ public String doCreateGeoPartitionedBackup(int numRegions, boolean useTablespace
}
}

public String doTestGeoPartitionedBackup(String targetDB, int numRegions, boolean useTablespaces)
throws Exception {
public String doTestGeoPartitionedBackup(
String targetDB, int numRegions, boolean useTablespaces, boolean dropTablespaces,
String restoreFlag) throws Exception {
String output = null;
try (Statement stmt = connection.createStatement()) {
output = doCreateGeoPartitionedBackup(numRegions, useTablespaces);
Expand All @@ -1258,19 +1259,25 @@ public String doTestGeoPartitionedBackup(String targetDB, int numRegions, boolea
args.add("--use_tablespaces");
}

if (restoreFlag != null) {
args.add(restoreFlag);
}

if (!targetDB.equals("yugabyte")) {
// Drop TABLEs and TABLESPACEs.
stmt.execute("DROP TABLE tbl_r1");
stmt.execute("DROP TABLE tbl_r2");
stmt.execute("DROP TABLE tbl_r3");
stmt.execute("DROP TABLE tbl");
stmt.execute("DROP TABLESPACE region1_ts");
stmt.execute("DROP TABLESPACE region2_ts");
stmt.execute("DROP TABLESPACE region3_ts");

// Check global TABLESPACEs.
assertRowSet(stmt, "SELECT spcname FROM pg_tablespace",
asSet(new Row("pg_default"), new Row("pg_global")));
if (dropTablespaces) {
stmt.execute("DROP TABLESPACE region1_ts");
stmt.execute("DROP TABLESPACE region2_ts");
stmt.execute("DROP TABLESPACE region3_ts");

// Check global TABLESPACEs.
assertRowSet(stmt, "SELECT spcname FROM pg_tablespace",
asSet(new Row("pg_default"), new Row("pg_global")));
}

args.addAll(Arrays.asList("--keyspace", "ysql." + targetDB));
}
Expand Down Expand Up @@ -1315,6 +1322,12 @@ public String doTestGeoPartitionedBackup(String targetDB, int numRegions, boolea
return output;
}

public String doTestGeoPartitionedBackup(String targetDB, int numRegions, boolean useTablespaces)
throws Exception {
return doTestGeoPartitionedBackup(
targetDB, numRegions, useTablespaces, /* dropTablespaces */ true, /* restoreFlag */ null);
}

@Test
public void testGeoPartitioning() throws Exception {
doTestGeoPartitionedBackup("db2", 3, false);
Expand Down Expand Up @@ -1355,6 +1368,30 @@ public void testGeoPartitioningRestoringIntoExistingWithTablespaces() throws Exc
doTestGeoPartitionedBackup("yugabyte", 3, true);
}

@Test
public void testFailureOnExistingTablespaces() throws Exception {
// This value must be synced with the same variable in `yb_backup.py` script.
final boolean ENABLE_STOP_ON_YSQL_DUMP_RESTORE_ERROR = false;

try {
doTestGeoPartitionedBackup("db2", 3,
/* useTablespaces */ true, /* dropTablespaces */ false, /* restoreFlag */ null);

if (ENABLE_STOP_ON_YSQL_DUMP_RESTORE_ERROR) {
fail("Backup restoring did not fail as expected");
}
} catch (YBBackupException ex) {
LOG.info("Expected exception", ex);
assertTrue(ex.getMessage().contains("tablespace \"region1_ts\" already exists"));
}
}

@Test
public void testIgnoreExistingTablespaces() throws Exception {
doTestGeoPartitionedBackup("db2", 3,
/* useTablespaces */ true, /* dropTablespaces */ false, "--ignore_existing_tablespaces");
}

@Test
public void testGeoPartitioningDeleteBackup() throws Exception {
String output = doCreateGeoPartitionedBackup(3, false);
Expand Down
16 changes: 16 additions & 0 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlDump.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,22 @@ public void ysqlDumpWithoutYbMetadata() throws Exception {
IncludeYbMetadata.OFF);
}

@Test
public void ysqlDumpAllWithoutYbMetadata() throws Exception {
// Note that we're using the same describe input as for regular ysql_dump!
ysqlDumpTester(
"ysql_dumpall" /* binaryName */,
"" /* dumpedDatabaseName */,
"sql/yb_ysql_dumpall.sql" /* inputFileRelativePath */,
"sql/yb_ysql_dump_describe.sql" /* inputDescribeFileRelativePath */,
"data/yb_ysql_dumpall_without_ybmetadata.data.sql" /* expectedDumpRelativePath */,
"expected/yb_ysql_dumpall_describe.out" /* expectedDescribeFileRelativePath */,
"results/yb_ysql_dumpall_without_ybmetadata.out" /* outputFileRelativePath */,
"results/yb_ysql_dumpall_without_ybmetadata_describe.out"
/* outputDescribeFileRelativePath */,
IncludeYbMetadata.OFF);
}

@Test
public void ysqlDumpColocatedDB() throws Exception {
ysqlDumpTester(
Expand Down
11 changes: 10 additions & 1 deletion managed/devops/bin/yb_backup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1328,9 +1328,14 @@ def parse_arguments(self):
parser.add_argument(
'--restore_time', required=False,
help='The Unix microsecond timestamp to which to restore the snapshot.')

parser.add_argument(
'--ignore_existing_tablespaces', required=False, action='store_true', default=False,
help='Whether to skip tablespace creation if tablespace already exists.')
parser.add_argument(
'--use_tablespaces', required=False, action='store_true', default=False,
help='Backup/restore YSQL TABLESPACE objects into/from the backup.')

parser.add_argument('--upload', dest='upload', action='store_true', default=True)
# Please note that we have to use this weird naming (i.e. underscore in the argument name)
# style to keep it in sync with YB processes G-flags.
Expand Down Expand Up @@ -3042,7 +3047,8 @@ def create_metadata_files(self):
if sql_tbsp_dump_path:
logging.info("[app] Creating ysql dump for tablespaces to {}".format(
sql_tbsp_dump_path))
self.run_ysql_dumpall(['--tablespaces-only', '--file=' + sql_tbsp_dump_path])
self.run_ysql_dumpall(['--tablespaces-only', '--include-yb-metadata',
'--file=' + sql_tbsp_dump_path])
dump_files.append(sql_tbsp_dump_path)
else:
ysql_dump_args.append('--no-tablespaces')
Expand Down Expand Up @@ -3496,6 +3502,9 @@ def import_ysql_dump(self, dump_file_path):

ysql_shell_args = ['--echo-all', '--file=' + dump_file_path]

if self.args.ignore_existing_tablespaces:
ysql_shell_args.append('--variable=ignore_existing_tablespaces=1')

if on_error_stop:
logging.info(
"[app] Enable ON_ERROR_STOP mode when applying '{}'".format(dump_file_path))
Expand Down
28 changes: 27 additions & 1 deletion src/postgres/src/bin/pg_dump/pg_dumpall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,17 @@ dumpTablespaces(PGconn *conn)
"ORDER BY 1");

if (PQntuples(res) > 0)
{
fprintf(OPF, "--\n-- Tablespaces\n--\n\n");

if (include_yb_metadata)
fprintf(OPF, "-- Set variable ignore_existing_tablespaces (if not already set)\n"
"\\if :{?ignore_existing_tablespaces}\n"
"\\else\n"
"\\set ignore_existing_tablespaces false\n"
"\\endif\n\n");
}

for (i = 0; i < PQntuples(res); i++)
{
PQExpBuffer buf = createPQExpBuffer();
Expand All @@ -1231,19 +1240,33 @@ dumpTablespaces(PGconn *conn)
/* needed for buildACLCommands() */
fspcname = pg_strdup(fmtId(spcname));

if (include_yb_metadata)
appendPQExpBuffer(buf,
"\\set tablespace_exists false\n"
"\\if :ignore_existing_tablespaces\n"
" SELECT EXISTS(SELECT 1 FROM pg_tablespace WHERE spcname = '%s')"
" AS tablespace_exists \\gset\n"
"\\endif\n"
"\\if :tablespace_exists\n"
" \\echo 'Tablespace %s already exists.'\n"
"\\else\n ", fspcname, fspcname);

appendPQExpBuffer(buf, "CREATE TABLESPACE %s", fspcname);
appendPQExpBuffer(buf, " OWNER %s", fmtId(spcowner));

appendPQExpBufferStr(buf, " LOCATION ");
appendStringLiteralConn(buf, spclocation, conn);
if (spcoptions && spcoptions[0] != '\0')
{
appendPQExpBuffer(buf, " WITH (");
appendPQExpBufferStr(buf, " WITH (");
ybProcessTablespaceSpcOptions(conn, &buf, spcoptions);
appendPQExpBufferStr(buf, ")");
}
appendPQExpBufferStr(buf, ";\n");

if (include_yb_metadata)
appendPQExpBufferStr(buf, "\\endif\n");

if (!skip_acls &&
!buildACLCommands(fspcname, NULL, NULL, "TABLESPACE",
spcacl, rspcacl,
Expand All @@ -1267,6 +1290,9 @@ dumpTablespaces(PGconn *conn)
"TABLESPACE", spcname,
buf);

if (include_yb_metadata)
appendPQExpBufferStr(buf, "\n");

fprintf(OPF, "%s", buf->data);

free(fspcname);
Expand Down
39 changes: 36 additions & 3 deletions src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,42 @@ ALTER ROLE yugabyte_test WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN NOREPL
-- Tablespaces
--

CREATE TABLESPACE tsp1 OWNER yugabyte_test LOCATION '';
CREATE TABLESPACE tsp2 OWNER yugabyte_test LOCATION '' WITH (replica_placement='{"num_replicas":1, "placement_blocks":[{"cloud":"cloud1","region":"datacenter1","zone":"rack1","min_num_replicas":1}]}');
CREATE TABLESPACE tsp_unused OWNER yugabyte_test LOCATION '' WITH (replica_placement='{"num_replicas":1, "placement_blocks":[{"cloud":"cloud1","region":"dc_unused","zone":"z_unused","min_num_replicas":1}]}');
-- Set variable ignore_existing_tablespaces (if not already set)
\if :{?ignore_existing_tablespaces}
\else
\set ignore_existing_tablespaces false
\endif

\set tablespace_exists false
\if :ignore_existing_tablespaces
SELECT EXISTS(SELECT 1 FROM pg_tablespace WHERE spcname = 'tsp1') AS tablespace_exists \gset
\endif
\if :tablespace_exists
\echo 'Tablespace tsp1 already exists.'
\else
CREATE TABLESPACE tsp1 OWNER yugabyte_test LOCATION '';
\endif

\set tablespace_exists false
\if :ignore_existing_tablespaces
SELECT EXISTS(SELECT 1 FROM pg_tablespace WHERE spcname = 'tsp2') AS tablespace_exists \gset
\endif
\if :tablespace_exists
\echo 'Tablespace tsp2 already exists.'
\else
CREATE TABLESPACE tsp2 OWNER yugabyte_test LOCATION '' WITH (replica_placement='{"num_replicas":1, "placement_blocks":[{"cloud":"cloud1","region":"datacenter1","zone":"rack1","min_num_replicas":1}]}');
\endif

\set tablespace_exists false
\if :ignore_existing_tablespaces
SELECT EXISTS(SELECT 1 FROM pg_tablespace WHERE spcname = 'tsp_unused') AS tablespace_exists \gset
\endif
\if :tablespace_exists
\echo 'Tablespace tsp_unused already exists.'
\else
CREATE TABLESPACE tsp_unused OWNER yugabyte_test LOCATION '' WITH (replica_placement='{"num_replicas":1, "placement_blocks":[{"cloud":"cloud1","region":"dc_unused","zone":"z_unused","min_num_replicas":1}]}');
\endif



\connect template1
Expand Down
Loading

0 comments on commit f6c3c86

Please sign in to comment.