Skip to content

Commit

Permalink
[#23626] allow loading old dumps that do not have index pg_class OIDs
Browse files Browse the repository at this point in the history
Summary:
The original diff, D37360, failed to account for one way of setting PG class OIDs.  That broke backwards compatibility.

This diff fixes this by taking that way of setting OIDs into account as well.

In particular, the previous flag yb_ignore_heap_pg_class_oids, is generalized into yb_ignore_pg_class_oids, which ignores both heap PG class OIDs and index PG class OIDs now.

NOTE: this involves renaming a flag but the previous flag has not actually gone out in any public release so this is okay.

Fixes #23626

Test Plan:
Verify preserving all pg_class OIDs works for new version:
```
ybd --cxx-test yb-backup-cross-feature-test --gtest_filter '*.TestPreservingPgEnumOids' --test-args '--verbose_yb_backup' >& /tmp/generic.mdl.log
ybd --cxx-test yb-backup-cross-feature-test --gtest_filter '*.TestPreservingPgTypeAndClassOids' --test-args '--verbose_yb_backup' >& /tmp/generic.mdl.log
```

Testing backward compatibility:
  * spin up a universe without D36675
    * git switch --detach 516ead0^
  * create objects that will need OIDs supplied:
```
CREATE SEQUENCE foo;
CREATE TYPE public.complex AS (
	re double precision,
	im double precision
);
CREATE TYPE filler_2 AS ENUM ('A','B','C');
CREATE TABLE indexed_table (a INT, b INT);
CREATE INDEX simple_index ON indexed_table (a DESC);
CREATE TABLE table_with_index_and_constraint (k INT PRIMARY KEY, v TEXT, UNIQUE (v))
```
  * dump out the metadata using YSQL dump:
```
build/latest/postgres/bin/ysql_dump --host=127.0.0.10 --dbname=yugabyte --include-yb-metadata --serializable-deferrable --create --schema-only --file=/tmp/dump
```
  * shut that universe down and start up a new one using this diff
  * restore the metadata:
```
~/code/yugabyte-db/bin/ysqlsh -h 127.0.0.20 --dbname=template1 -c 'DROP DATABASE yugabyte;'
bin/ysqlsh -h 127.0.0.20 --dbname=template1 --file=/tmp/dump
```
  * verify no errors

To ensure check still works, add `SET yb_ignore_pg_class_oids = false;` to that dump after each occurrence of
```
SET yb_binary_restore = true;
```
verify you get an error trying to restore it:
```
ysqlsh:/tmp/dump2:81: ERROR:  pg_class heap OID value not set when in binary upgrade mode
```

Other tests:
```
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpAllWithYbMetadata' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithYbMetadata' >& /tmp/generic.mdl.log2
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpColocatedDB' >& /tmp/generic.mdl.log3
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpLegacyColocatedDB' >& /tmp/generic.mdl.log4
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithoutYbMetadata' >& /tmp/generic.mdl.log5
```

Reviewers: myang, xCluster

Reviewed By: myang

Subscribers: yql, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D37588
  • Loading branch information
mdbridge committed Aug 28, 2024
1 parent 91d000a commit 6456777
Show file tree
Hide file tree
Showing 11 changed files with 27 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/postgres/src/backend/catalog/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ heap_create_with_catalog(const char *relname,
if (!OidIsValid(relid))
{
bool heap_pg_class_oids_supplied = IsBinaryUpgrade && !yb_binary_restore;
if (yb_binary_restore && !yb_ignore_heap_pg_class_oids)
if (yb_binary_restore && !yb_ignore_pg_class_oids)
heap_pg_class_oids_supplied = true;
/* Use binary-upgrade override for pg_class.oid/relfilenode? */
if (heap_pg_class_oids_supplied &&
Expand Down
5 changes: 4 additions & 1 deletion src/postgres/src/backend/catalog/index.c
Original file line number Diff line number Diff line change
Expand Up @@ -961,8 +961,11 @@ index_create(Relation heapRelation,
*/
if (!OidIsValid(indexRelationId))
{
bool index_pg_class_oids_supplied = IsBinaryUpgrade && !yb_binary_restore;
if (yb_binary_restore && !yb_ignore_pg_class_oids)
index_pg_class_oids_supplied = true;
/* Use binary-upgrade override for pg_class.oid/relfilenode? */
if (IsBinaryUpgrade || yb_binary_restore)
if (index_pg_class_oids_supplied)
{
if (!OidIsValid(binary_upgrade_next_index_pg_class_oid))
ereport(ERROR,
Expand Down
6 changes: 3 additions & 3 deletions src/postgres/src/backend/utils/misc/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2237,12 +2237,12 @@ static struct config_bool ConfigureNamesBool[] =
},

{
{"yb_ignore_heap_pg_class_oids", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Ignores requests to set heap pg_class OIDs in yb_binary_restore mode"),
{"yb_ignore_pg_class_oids", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Ignores requests to set pg_class OIDs in yb_binary_restore mode"),
NULL,
GUC_NOT_IN_SAMPLE
},
&yb_ignore_heap_pg_class_oids,
&yb_ignore_pg_class_oids,
true,
NULL, NULL, NULL
},
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/bin/pg_dump/pg_backup_archiver.c
Original file line number Diff line number Diff line change
Expand Up @@ -3116,7 +3116,7 @@ _doSetFixedOutputState(ArchiveHandle *AH)
if (AH->public.dopt->include_yb_metadata)
{
ahprintf(AH, "SET yb_binary_restore = true;\n");
ahprintf(AH, "SET yb_ignore_heap_pg_class_oids = false;\n");
ahprintf(AH, "SET yb_ignore_pg_class_oids = false;\n");
ahprintf(AH, "SET yb_non_ddl_txn_for_sys_tables_allowed = true;\n");
}
ahprintf(AH, "SET statement_timeout = 0;\n");
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/test/regress/data/yb_ysql_dump.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
12 changes: 6 additions & 6 deletions src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ ALTER ROLE yugabyte_test WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN NOREPL
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -199,7 +199,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -257,7 +257,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -295,7 +295,7 @@ CREATE DATABASE system_platform WITH TEMPLATE = template0 ENCODING = 'UTF8' LC_C
\connect system_platform

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -348,7 +348,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 11.2-YB-2.21.1.0-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -386,7 +386,7 @@ CREATE DATABASE yugabyte WITH TEMPLATE = template0 ENCODING = 'UTF8' LC_COLLATE
\connect yugabyte

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_ignore_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/yb/yql/pggate/util/yb_guc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ bool suppress_nonpg_logs = false;

bool yb_binary_restore = false;

bool yb_ignore_heap_pg_class_oids = true;
bool yb_ignore_pg_class_oids = true;

bool yb_pushdown_strict_inequality = true;

Expand Down
7 changes: 4 additions & 3 deletions src/yb/yql/pggate/util/yb_guc.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@ extern int ysql_max_in_flight_ops;
extern bool yb_binary_restore;

/*
* Guc variable for ignoring requests to set heap pg_class oids when yb_binary_restore is set.
* Guc variable for ignoring requests to set pg_class oids when yb_binary_restore is set.
*
* If true then calls to pg_catalog.binary_upgrade_set_next_heap_pg_class_oid will have no effect.
* If true then calls to pg_catalog.binary_upgrade_set_next_{heap|index}_pg_class_oid will have no
* effect.
*/
extern bool yb_ignore_heap_pg_class_oids;
extern bool yb_ignore_pg_class_oids;

/*
* Set to true only for runs with EXPLAIN ANALYZE
Expand Down
7 changes: 4 additions & 3 deletions src/yb/yql/pggate/util/ybc_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,12 @@ extern int yb_locks_txn_locks_per_tablet;
extern bool yb_binary_restore;

/*
* Guc variable for ignoring requests to set heap pg_class oids when yb_binary_restore is set.
* Guc variable for ignoring requests to set pg_class oids when yb_binary_restore is set.
*
* If true then calls to pg_catalog.binary_upgrade_set_next_heap_pg_class_oid will have no effect.
* If true then calls to pg_catalog.binary_upgrade_set_next_{heap|index}_pg_class_oid will have no
* effect.
*/
extern bool yb_ignore_heap_pg_class_oids;
extern bool yb_ignore_pg_class_oids;

/*
* Set to true only for runs with EXPLAIN ANALYZE
Expand Down

0 comments on commit 6456777

Please sign in to comment.