From 70f96d1e4612bebef69636aa3ffdd8f24b50c833 Mon Sep 17 00:00:00 2001 From: yifanguan Date: Wed, 2 Mar 2022 16:53:41 -0500 Subject: [PATCH] [BACKPORT 2.12][#4873] YSQL: Support backup/restore for multi-tablet range-split relations Summary: In this backport diff, formatted string's type field for `yb_table_properties.num_tablets` is changed from `PRIu64` to `%u` because its type is `uint32_t` instead of `uint64_t` on this backport branch. The expected output files of test: TestYsqlDump are also changed accordingly based on branch 2.12. This test passed on Jenkins. After 96beb9e77e5d6e4032106bdbc52fcd15df5f6c05, restoring a backup will re-partition relations to match the splits from their actual tablet snapshots if they differ from the pre-created ones. As a result, even if the split information from the ysql_dump file is missing or not accurate, restore should go through correctly. Therefore, as a stop-gap fix for backup-restore make the error when dumping a multi-tablet range-split relation (table or index) just a warning instead. This diff also fixes an issue in TestYSQLRangeSplitConstraint index-data validation code. Previously that data read was actually using the table instead of the index. Fully supporting range-split tables in ysql_dump (SPLIT AT clause) is still needed for regular ysql_dump export flow and may also improve performance of restore. So this will be addressed in a follow-up commit. Original Commit: 3156825304c9ad4b7ea20464bab99a272dba758d Original Differential Revision: https://phabricator.dev.yugabyte.com/D15700 Test Plan: YBBackupTest.TestYSQLRangeSplitConstraint org.yb.pgsql.TestYsqlDump Reviewers: mihnea, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D15758 --- ent/src/yb/tools/yb-backup-test_ent.cc | 34 ++-- .../src/backend/utils/adt/ruleutils.c | 12 +- src/postgres/src/bin/pg_dump/pg_dump.c | 6 +- .../test/regress/expected/yb_ysql_dump.out | 184 +++++++++++++++++- .../expected/yb_ysql_dump_verifier.out | 9 +- .../src/test/regress/sql/yb_ysql_dump.sql | 39 ++++ 6 files changed, 263 insertions(+), 21 deletions(-) diff --git a/ent/src/yb/tools/yb-backup-test_ent.cc b/ent/src/yb/tools/yb-backup-test_ent.cc index daacb7cb50ac..6e6e8291b000 100644 --- a/ent/src/yb/tools/yb-backup-test_ent.cc +++ b/ent/src/yb/tools/yb-backup-test_ent.cc @@ -891,9 +891,11 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS_OR_MAC(TestYCQLBackupWithDefi // constraint as 1 hash tablet. Restore should restore the unique constraint index as 3 tablets // since the tablet snapshot files are already split into 3 tablets. // -// TODO(jason): enable test when issue #4873 ([YSQL] Support backup for pre-split multi-tablet range -// tables) is fixed. -TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) { +// TODO(yguan): after the SPLIT AT clause is fully supported by ysql_dump this test needs to +// be revisited as the table may no longer need re-partitioning. +// Therefore, to exercise CatalogManager::RepartitionTable this test may need +// to be updated similar to TestYSQLManualTabletSplit. +TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS_OR_MAC(TestYSQLRangeSplitConstraint)) { const string table_name = "mytbl"; const string index_name = "myidx"; @@ -903,21 +905,29 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) { ASSERT_NO_FATALS(CreateIndex( Format("CREATE UNIQUE INDEX $0 ON $1 (v ASC) SPLIT AT VALUES (('foo'), ('qux'))", index_name, table_name))); + + // Commenting out the ALTER .. ADD UNIQUE constraint case as this case is not supported. + // Vanilla Postgres disallows adding indexes with non-default (DESC) sort option as constraints. + // In YB we have added HASH and changed default (for first column) from ASC to HASH. + // + // See #11583 for details -- we should revisit this test after that is fixed. + /* ASSERT_NO_FATALS(RunPsqlCommand( Format("ALTER TABLE $0 ADD UNIQUE USING INDEX $1", table_name, index_name), "ALTER TABLE")); + */ // Write data in each partition of the index. ASSERT_NO_FATALS(InsertRows( - Format("INSERT INTO $0 (v) VALUES ('bar'), ('jar'), ('tar')", table_name), 3)); + Format("INSERT INTO $0 (v) VALUES ('tar'), ('bar'), ('jar')", table_name), 3)); ASSERT_NO_FATALS(RunPsqlCommand( - Format("SELECT * FROM $0 ORDER BY k", table_name), + Format("SELECT * FROM $0 ORDER BY v", table_name), R"#( k | v ---+----- - 1 | bar - 2 | jar - 3 | tar + 2 | bar + 3 | jar + 1 | tar (3 rows) )#" )); @@ -937,13 +947,13 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) { // Verify data. ASSERT_NO_FATALS(RunPsqlCommand( - Format("SELECT * FROM $0 ORDER BY k", table_name), + Format("SELECT * FROM $0 ORDER BY v", table_name), R"#( k | v ---+----- - 1 | bar - 2 | jar - 3 | tar + 2 | bar + 3 | jar + 1 | tar (3 rows) )#" )); diff --git a/src/postgres/src/backend/utils/adt/ruleutils.c b/src/postgres/src/backend/utils/adt/ruleutils.c index becb5d55fee8..84d739d47da2 100644 --- a/src/postgres/src/backend/utils/adt/ruleutils.c +++ b/src/postgres/src/backend/utils/adt/ruleutils.c @@ -1494,10 +1494,16 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, /* For range-partitioned tables */ if (yb_table_properties.num_tablets > 1) { - ereport(ERROR, + ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("Exporting SPLIT clause for range-partitioned " - "tables is not yet supported"))); + errmsg("exporting SPLIT clause for range-split relations is not yet " + "supported"), + errdetail("Index '%s' will be created with default (1) tablets " + "instead of %u.", + generate_relation_name(indrelid, NIL), + yb_table_properties.num_tablets), + errhint("See https://github.com/yugabyte/yugabyte-db/issues/4873." + " Click '+' on the description to raise its priority."))); } } } diff --git a/src/postgres/src/bin/pg_dump/pg_dump.c b/src/postgres/src/bin/pg_dump/pg_dump.c index f467cbddec68..5c8e14790e22 100644 --- a/src/postgres/src/bin/pg_dump/pg_dump.c +++ b/src/postgres/src/bin/pg_dump/pg_dump.c @@ -16333,8 +16333,10 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) else if (properties.num_tablets > 1) { /* For range-table. */ - fprintf(stderr, "Pre-split range tables are not supported yet.\n"); - exit_nicely(1); + write_msg(NULL, "WARNING: exporting SPLIT clause for range-split relations is not " + "yet supported. Table '%s' will be created with default (1) " + "tablets instead of %u.\n", + qualrelname, properties.num_tablets); } /* else - single shard table - supported, no need to add anything */ } diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump.out b/src/postgres/src/test/regress/expected/yb_ysql_dump.out index 031c58beeb64..2e74da53d94b 100644 --- a/src/postgres/src/test/regress/expected/yb_ysql_dump.out +++ b/src/postgres/src/test/regress/expected/yb_ysql_dump.out @@ -2,8 +2,8 @@ -- YSQL database dump -- --- Dumped from database version 11.2-YB-2.11.3.0-b0 --- Dumped by ysql_dump version 11.2-YB-2.11.3.0-b0 +-- Dumped from database version 11.2-YB-2.12.2.0-b0 +-- Dumped by ysql_dump version 11.2-YB-2.12.2.0-b0 SET yb_binary_restore = true; SET statement_timeout = 0; @@ -616,6 +616,118 @@ TABLEGROUP grp2; ALTER TABLE public.tgroup_options_and_tgroup OWNER TO tablegroup_test_user; +-- +-- Name: th1; Type: TABLE; Schema: public; Owner: yugabyte_test +-- + + +-- For binary upgrade, must preserve pg_type oid +SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16512'::pg_catalog.oid); + + +-- For binary upgrade, must preserve pg_type array oid +SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16511'::pg_catalog.oid); + +CREATE TABLE public.th1 ( + a integer, + b text, + c double precision +) +SPLIT INTO 2 TABLETS; + + +ALTER TABLE public.th1 OWNER TO yugabyte_test; + +-- +-- Name: th2; Type: TABLE; Schema: public; Owner: yugabyte_test +-- + + +-- For binary upgrade, must preserve pg_type oid +SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16515'::pg_catalog.oid); + + +-- For binary upgrade, must preserve pg_type array oid +SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16514'::pg_catalog.oid); + +CREATE TABLE public.th2 ( + a integer NOT NULL, + b text NOT NULL, + c double precision, + CONSTRAINT th2_pkey PRIMARY KEY((a) HASH, b ASC) +) +SPLIT INTO 3 TABLETS; + + +ALTER TABLE public.th2 OWNER TO yugabyte_test; + +-- +-- Name: th3; Type: TABLE; Schema: public; Owner: yugabyte_test +-- + + +-- For binary upgrade, must preserve pg_type oid +SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16520'::pg_catalog.oid); + + +-- For binary upgrade, must preserve pg_type array oid +SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16519'::pg_catalog.oid); + +CREATE TABLE public.th3 ( + a integer NOT NULL, + b text NOT NULL, + c double precision, + CONSTRAINT th3_pkey PRIMARY KEY((a, b) HASH) +) +SPLIT INTO 4 TABLETS; + + +ALTER TABLE public.th3 OWNER TO yugabyte_test; + +-- +-- Name: tr1; Type: TABLE; Schema: public; Owner: yugabyte_test +-- + + +-- For binary upgrade, must preserve pg_type oid +SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16525'::pg_catalog.oid); + + +-- For binary upgrade, must preserve pg_type array oid +SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16524'::pg_catalog.oid); + +CREATE TABLE public.tr1 ( + a integer NOT NULL, + b text, + c double precision, + CONSTRAINT tr1_pkey PRIMARY KEY(a ASC) +); + + +ALTER TABLE public.tr1 OWNER TO yugabyte_test; + +-- +-- Name: tr2; Type: TABLE; Schema: public; Owner: yugabyte_test +-- + + +-- For binary upgrade, must preserve pg_type oid +SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16530'::pg_catalog.oid); + + +-- For binary upgrade, must preserve pg_type array oid +SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16529'::pg_catalog.oid); + +CREATE TABLE public.tr2 ( + a integer NOT NULL, + b text NOT NULL, + c double precision NOT NULL, + CONSTRAINT tr2_pkey PRIMARY KEY(a DESC, b ASC, c DESC) +); + + +ALTER TABLE public.tr2 OWNER TO yugabyte_test; + -- -- Name: uaccount; Type: TABLE; Schema: public; Owner: regress_rls_alice -- @@ -845,6 +957,46 @@ COPY public.tgroup_options_and_tgroup (a) FROM stdin; \. +-- +-- Data for Name: th1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test +-- + +COPY public.th1 (a, b, c) FROM stdin; +\. + + +-- +-- Data for Name: th2; Type: TABLE DATA; Schema: public; Owner: yugabyte_test +-- + +COPY public.th2 (a, b, c) FROM stdin; +\. + + +-- +-- Data for Name: th3; Type: TABLE DATA; Schema: public; Owner: yugabyte_test +-- + +COPY public.th3 (a, b, c) FROM stdin; +\. + + +-- +-- Data for Name: tr1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test +-- + +COPY public.tr1 (a, b, c) FROM stdin; +\. + + +-- +-- Data for Name: tr2; Type: TABLE DATA; Schema: public; Owner: yugabyte_test +-- + +COPY public.tr2 (a, b, c) FROM stdin; +\. + + -- -- Data for Name: uaccount; Type: TABLE DATA; Schema: public; Owner: regress_rls_alice -- @@ -902,6 +1054,34 @@ CREATE INDEX tbl8_idx4 ON public.tbl8 USING lsm (b DESC); CREATE INDEX tbl8_idx5 ON public.tbl8 USING lsm (c HASH) SPLIT INTO 3 TABLETS; +-- +-- Name: th2_c_b_idx; Type: INDEX; Schema: public; Owner: yugabyte_test +-- + +CREATE INDEX th2_c_b_idx ON public.th2 USING lsm (c HASH, b DESC) SPLIT INTO 4 TABLETS; + + +-- +-- Name: th3_c_b_idx; Type: INDEX; Schema: public; Owner: yugabyte_test +-- + +CREATE INDEX th3_c_b_idx ON public.th3 USING lsm ((c, b) HASH) SPLIT INTO 3 TABLETS; + + +-- +-- Name: tr2_c_b_a_idx; Type: INDEX; Schema: public; Owner: yugabyte_test +-- + +CREATE INDEX tr2_c_b_a_idx ON public.tr2 USING lsm (c ASC, b DESC, a ASC); + + +-- +-- Name: tr2_c_idx; Type: INDEX; Schema: public; Owner: yugabyte_test +-- + +CREATE INDEX tr2_c_idx ON public.tr2 USING lsm (c DESC); + + -- -- Name: uaccount account_policies; Type: POLICY; Schema: public; Owner: regress_rls_alice -- diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out b/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out index 860dcfbf5f73..b60124b733b2 100644 --- a/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out +++ b/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out @@ -1,5 +1,5 @@ List of relations - Schema | Name | Type | Owner + Schema | Name | Type | Owner --------+------------------------------+----------+---------------------- public | chat_user | table | yugabyte_test public | rls_private | table | yugabyte_test @@ -25,8 +25,13 @@ public | tgroup_one_option_and_tgroup | table | tablegroup_test_user public | tgroup_options | table | tablegroup_test_user public | tgroup_options_and_tgroup | table | tablegroup_test_user + public | th1 | table | yugabyte_test + public | th2 | table | yugabyte_test + public | th3 | table | yugabyte_test + public | tr1 | table | yugabyte_test + public | tr2 | table | yugabyte_test public | uaccount | table | regress_rls_alice -(25 rows) +(30 rows) List of tablespaces Name | Owner | Location diff --git a/src/postgres/src/test/regress/sql/yb_ysql_dump.sql b/src/postgres/src/test/regress/sql/yb_ysql_dump.sql index dc9c12842f52..14602805c56c 100644 --- a/src/postgres/src/test/regress/sql/yb_ysql_dump.sql +++ b/src/postgres/src/test/regress/sql/yb_ysql_dump.sql @@ -77,3 +77,42 @@ BEGIN; UPDATE pg_class SET reloptions = array_prepend('a=b', reloptions) WHERE relname = 'tgroup_after_options'; UPDATE pg_class SET reloptions = array_prepend('a=b', reloptions) WHERE relname = 'tgroup_in_between_options'; COMMIT; + +RESET SESSION AUTHORIZATION; + +------------------------------------------------ +-- Test table and index explicit splitting. +------------------------------------------------ + +------------------------------------ +-- Tables + +-- Table without a primary key +CREATE TABLE th1 (a int, b text, c float) SPLIT INTO 2 TABLETS; + +-- Hash-partitioned table with range components +CREATE TABLE th2 (a int, b text, c float, PRIMARY KEY (a HASH, b ASC)) SPLIT INTO 3 TABLETS; + +-- Hash-partitioned table without range components +CREATE TABLE th3 (a int, b text, c float, PRIMARY KEY ((a, b) HASH)) SPLIT INTO 4 TABLETS; + +-- Range-partitioned table with single-column key +CREATE TABLE tr1 (a int, b text, c float, PRIMARY KEY (a ASC)) SPLIT AT VALUES ((1), (100)); + +-- Range-partitioned table with multi-column key +CREATE TABLE tr2 (a int, b text, c float, PRIMARY KEY (a DESC, b ASC, c DESC)) SPLIT AT VALUES ((100, 'a', 2.5), (50, 'n'), (1, 'z', -5.12)); + +------------------------------------ +-- Indexes + +-- Hash-partitioned table with range components +CREATE INDEX on th2(c HASH, b DESC) SPLIT INTO 4 TABLETS; + +-- Hash-partitioned table without range components +CREATE INDEX on th3((c, b) HASH) SPLIT INTO 3 TABLETS; + +-- Range-partitioned table with single-column key +CREATE INDEX ON tr2(c DESC) SPLIT AT VALUES ((100.5), (1.5)); + +-- Range-partitioned table with multi-column key +CREATE INDEX ON tr2(c ASC, b DESC, a ASC) SPLIT AT VALUES ((-5.12, 'z', 1), (-0.75, 'l'), (2.5, 'a', 100));