external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove pg_depend entries from columnar metadata indexes to columnar-am (inserted in #5456) (#6628) 

DESCRIPTION: Fixes (pg_dump/pg_upgrade) dependency loop warnings caused
by pg_depend entries inserted by citus_columnar

Fixes #5510.

In the past, having columnar tables in the cluster was causing pg
upgrades to fail when attempting to access columnar metadata. This is
because, pg_dump doesn't see objects that we use for columnar-am related
booking as the dependencies of the tables using columnar-am.
To fix that; in #5456, we inserted some "normal dependency" edges (from
those objects to columnar-am) into pg_depend.

This helped us ensuring the existency of a class of metadata objects
--such as columnar.storageid_seq-- and helped fixing #5437.

However, the normal-dependency edges that we added for indexes on
columnar metadata tables --such columnar.stripe_pkey-- didn't help at
all because they were indeed causing dependency loops (#5510) and
pg_dump was not able to take those dependency edges into the account.

For this reason, this commit deletes those dependency edges so that
pg_dump stops complaining about them. Note that it's not critical to
delete those edges from pg_depend since they're not breaking pg upgrades
but were triggering some warning messages. And given that backporting
a sql change into older versions is hard a lot, we skip backporting
this.
pull/6767/head
Onur Tirtir 2023-03-15 01:24:57 +03:00 committed by GitHub
parent f68fc9e69c 9550ebd118
commit a0a41943d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 308 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/backend/columnar/.gitignore vendored Normal file
View File

@ -0,0 +1,3 @@
# The directory used to store columnar sql files after pre-processing them
# with 'cpp' in build-time, see src/backend/columnar/Makefile.
/build/

47
src/backend/columnar/Makefile
View File

@ -10,14 +10,51 @@ OBJS += \
MODULE_big = citus_columnar
EXTENSION = citus_columnar
columnar_sql_files = $(patsubst $(citus_abs_srcdir)/%,%,$(wildcard $(citus_abs_srcdir)/sql/*.sql))
columnar_downgrade_sql_files = $(patsubst $(citus_abs_srcdir)/%,%,$(wildcard $(citus_abs_srcdir)/sql/downgrades/*.sql))
DATA = $(columnar_sql_files) \
$(columnar_downgrade_sql_files)
template_sql_files = $(patsubst $(citus_abs_srcdir)/%,%,$(wildcard $(citus_abs_srcdir)/sql/*.sql))
template_downgrade_sql_files = $(patsubst $(citus_abs_srcdir)/sql/downgrades/%,%,$(wildcard $(citus_abs_srcdir)/sql/downgrades/*.sql))
generated_sql_files = $(patsubst %,$(citus_abs_srcdir)/build/%,$(template_sql_files))
generated_downgrade_sql_files += $(patsubst %,$(citus_abs_srcdir)/build/sql/%,$(template_downgrade_sql_files))
DATA_built = $(generated_sql_files)
PG_CPPFLAGS += -I$(libpq_srcdir) -I$(safestringlib_srcdir)/include
include $(citus_top_builddir)/Makefile.global
.PHONY: install-all
SQL_DEPDIR=.deps/sql
SQL_BUILDDIR=build/sql
$(generated_sql_files): $(citus_abs_srcdir)/build/%: %
@mkdir -p $(citus_abs_srcdir)/$(SQL_DEPDIR) $(citus_abs_srcdir)/$(SQL_BUILDDIR)
@# -MF is used to store dependency files(.Po) in another directory for separation
@# -MT is used to change the target of the rule emitted by dependency generation.
@# -P is used to inhibit generation of linemarkers in the output from the preprocessor.
@# -undef is used to not predefine any system-specific or GCC-specific macros.
@# `man cpp` for further information
cd $(citus_abs_srcdir) && cpp -undef -w -P -MMD -MP -MF$(SQL_DEPDIR)/$(*F).Po -MT$@ $< > $@
$(generated_downgrade_sql_files): $(citus_abs_srcdir)/build/sql/%: sql/downgrades/%
@mkdir -p $(citus_abs_srcdir)/$(SQL_DEPDIR) $(citus_abs_srcdir)/$(SQL_BUILDDIR)
@# -MF is used to store dependency files(.Po) in another directory for separation
@# -MT is used to change the target of the rule emitted by dependency generation.
@# -P is used to inhibit generation of linemarkers in the output from the preprocessor.
@# -undef is used to not predefine any system-specific or GCC-specific macros.
@# `man cpp` for further information
cd $(citus_abs_srcdir) && cpp -undef -w -P -MMD -MP -MF$(SQL_DEPDIR)/$(*F).Po -MT$@ $< > $@
.PHONY: install install-downgrades install-all
cleanup-before-install:
rm -f $(DESTDIR)$(datadir)/$(datamoduledir)/citus_columnar.control
rm -f $(DESTDIR)$(datadir)/$(datamoduledir)/columnar--*
rm -f $(DESTDIR)$(datadir)/$(datamoduledir)/citus_columnar--*
install: cleanup-before-install
# install and install-downgrades should be run sequentially
install-all: install
$(MAKE) install-downgrades
install-downgrades: $(generated_downgrade_sql_files)
$(INSTALL_DATA) $(generated_downgrade_sql_files) '$(DESTDIR)$(datadir)/$(datamoduledir)/'

18
src/backend/columnar/sql/citus_columnar--11.1-1--11.2-1.sql
View File

@ -1 +1,19 @@
-- citus_columnar--11.1-1--11.2-1
#include "udfs/columnar_ensure_am_depends_catalog/11.2-1.sql"
DELETE FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid
AND objid IN (select oid from pg_am where amname = 'columnar')
AND objsubid = 0
AND refclassid = 'pg_class'::regclass::oid
AND refobjid IN (
'columnar_internal.stripe_first_row_number_idx'::regclass::oid,
'columnar_internal.chunk_group_pkey'::regclass::oid,
'columnar_internal.chunk_pkey'::regclass::oid,
'columnar_internal.options_pkey'::regclass::oid,
'columnar_internal.stripe_first_row_number_idx'::regclass::oid,
'columnar_internal.stripe_pkey'::regclass::oid
)
AND refobjsubid = 0
AND deptype = 'n';

3
src/backend/columnar/sql/downgrades/citus_columnar--11.2-1--11.1-1.sql
View File

@ -1 +1,4 @@
-- citus_columnar--11.2-1--11.1-1
-- Note that we intentionally do not re-insert the pg_depend records that we
-- deleted via citus_columnar--11.1-1--11.2-1.sql.

43
src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/11.2-1.sql Normal file
View File

@ -0,0 +1,43 @@
CREATE OR REPLACE FUNCTION columnar_internal.columnar_ensure_am_depends_catalog()
RETURNS void
LANGUAGE plpgsql
SET search_path = pg_catalog
AS $func$
BEGIN
INSERT INTO pg_depend
WITH columnar_schema_members(relid) AS (
SELECT pg_class.oid AS relid FROM pg_class
WHERE relnamespace =
COALESCE(
(SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar_internal'),
(SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar')
)
AND relname IN ('chunk',
'chunk_group',
'options',
'storageid_seq',
'stripe')
)
SELECT -- Define a dependency edge from "columnar table access method" ..
'pg_am'::regclass::oid as classid,
(select oid from pg_am where amname = 'columnar') as objid,
0 as objsubid,
-- ... to some objects registered as regclass and that lives in
-- "columnar" schema. That contains catalog tables and the sequences
-- created in "columnar" schema.
--
-- Given the possibility of user might have created their own objects
-- in columnar schema, we explicitly specify list of objects that we
-- are interested in.
'pg_class'::regclass::oid as refclassid,
columnar_schema_members.relid as refobjid,
0 as refobjsubid,
'n' as deptype
FROM columnar_schema_members
-- Avoid inserting duplicate entries into pg_depend.
EXCEPT TABLE pg_depend;
END;
$func$;
COMMENT ON FUNCTION columnar_internal.columnar_ensure_am_depends_catalog()
IS 'internal function responsible for creating dependencies from columnar '
'table access method to the rel objects in columnar schema';

17
src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/latest.sql
View File

@ -1,4 +1,4 @@
CREATE OR REPLACE FUNCTION citus_internal.columnar_ensure_am_depends_catalog()
CREATE OR REPLACE FUNCTION columnar_internal.columnar_ensure_am_depends_catalog()
RETURNS void
LANGUAGE plpgsql
SET search_path = pg_catalog
@ -14,22 +14,17 @@ BEGIN
)
AND relname IN ('chunk',
'chunk_group',
'chunk_group_pkey',
'chunk_pkey',
'options',
'options_pkey',
'storageid_seq',
'stripe',
'stripe_first_row_number_idx',
'stripe_pkey')
'stripe')
)
SELECT -- Define a dependency edge from "columnar table access method" ..
'pg_am'::regclass::oid as classid,
(select oid from pg_am where amname = 'columnar') as objid,
0 as objsubid,
-- ... to each object that is registered to pg_class and that lives
-- in "columnar" schema. That contains catalog tables, indexes
-- created on them and the sequences created in "columnar" schema.
-- ... to some objects registered as regclass and that lives in
-- "columnar" schema. That contains catalog tables and the sequences
-- created in "columnar" schema.
--
-- Given the possibility of user might have created their own objects
-- in columnar schema, we explicitly specify list of objects that we
@ -43,6 +38,6 @@ BEGIN
EXCEPT TABLE pg_depend;
END;
$func$;
COMMENT ON FUNCTION citus_internal.columnar_ensure_am_depends_catalog()
COMMENT ON FUNCTION columnar_internal.columnar_ensure_am_depends_catalog()
IS 'internal function responsible for creating dependencies from columnar '
'table access method to the rel objects in columnar schema';

15
src/test/regress/citus_tests/run_test.py
View File

@ -15,6 +15,16 @@ import common
import config
# Returns true if given test_schedule_line is of the form:
# "test: upgrade_ ... _after .."
def schedule_line_is_upgrade_after(test_schedule_line: str) -> bool:
return (
test_schedule_line.startswith("test: upgrade_")
and "_after" in test_schedule_line
)
if __name__ == "__main__":
args = argparse.ArgumentParser()
args.add_argument(
@ -172,6 +182,11 @@ if __name__ == "__main__":
if test_file_name in deps:
dependencies = deps[test_file_name]
elif schedule_line_is_upgrade_after(test_schedule_line):
dependencies = TestDeps(
default_base_schedule(test_schedule),
[test_file_name.replace("_after", "_before")],
)
else:
dependencies = TestDeps(default_base_schedule(test_schedule))

37
src/test/regress/expected/multi_extension.out
View File

@ -1258,6 +1258,43 @@ SELECT * FROM pg_dist_cleanup;
2 | 0 | 1 | table_with_orphaned_shards_102011 | 0 | 0
(2 rows)
ALTER EXTENSION citus_columnar UPDATE TO '11.2-1';
-- Make sure that we defined dependencies from all rel objects (tables,
-- indexes, sequences ..) to columnar table access method ...
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
objid = (select oid from pg_am where amname = 'columnar') AND
objsubid = 0 AND
refclassid = 'pg_class'::regclass::oid AND
refobjsubid = 0 AND
deptype = 'n';
-- ... , so this should be empty,
(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend)
UNION
(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members);
oid
---------------------------------------------------------------------
(0 rows)
-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members
-- should have 5 entries.
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
?column?
---------------------------------------------------------------------
t
(1 row)
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
-- error out as cleanup records remain
ALTER EXTENSION citus UPDATE TO '11.0-4';
ERROR: pg_dist_cleanup is introduced in Citus 11.1

62
src/test/regress/expected/upgrade_columnar_after.out
View File

@ -228,10 +228,12 @@ BEGIN;
22
(1 row)
-- make sure that serial is preserved
-- since we run "after schedule" twice and "rollback" wouldn't undo
-- sequence changes, it can be 22 or 33, not a different value
SELECT max(id) in (22, 33) FROM text_data;
-- Make sure that serial is preserved.
--
-- Since we might run "after schedule" several times for flaky test
-- detection and "rollback" wouldn't undo sequence changes, "id" should
-- look like below:
SELECT max(id) >= 11 AND max(id) % 11 = 0 FROM text_data;
?column?
---------------------------------------------------------------------
t
@ -265,7 +267,12 @@ ROLLBACK;
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal';
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
@ -283,8 +290,8 @@ UNION
(0 rows)
-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members
-- should have 10 entries.
SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend;
-- should have 5 entries.
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
?column?
---------------------------------------------------------------------
t
@ -292,12 +299,17 @@ SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend;
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
-- Check the same for workers too.
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal';
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
@ -308,44 +320,44 @@ WHERE classid = 'pg_am'::regclass::oid AND
deptype = 'n';
$$
);
run_command_on_workers
success | result
---------------------------------------------------------------------
(localhost,10201,t,"SELECT 10")
(localhost,10202,t,"SELECT 10")
t | SELECT 5
t | SELECT 5
(2 rows)
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend)
UNION
(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members);
$$
);
run_command_on_workers
success | result
---------------------------------------------------------------------
(localhost,10201,t,"")
(localhost,10202,t,"")
t |
t |
(2 rows)
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend;
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
$$
);
run_command_on_workers
success | result
---------------------------------------------------------------------
(localhost,10201,t,t)
(localhost,10202,t,t)
t | t
t | t
(2 rows)
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
$$
);
run_command_on_workers
success | result
---------------------------------------------------------------------
(localhost,10201,t,"DROP TABLE")
(localhost,10202,t,"DROP TABLE")
t | DROP TABLE
t | DROP TABLE
(2 rows)

22
src/test/regress/expected/upgrade_columnar_before.out
View File

@ -1,5 +1,27 @@
-- Test if relying on topological sort of the objects, not their names, works
-- fine when re-creating objects during pg_upgrade.
DO
$$
BEGIN
IF EXISTS (SELECT * FROM pg_namespace WHERE nspname = 'upgrade_columnar')
THEN
-- Drop the the table leftover from the earlier run of
-- upgrade_columnar_before.sql. Similarly, drop the fake public schema
-- created before and rename the original one (renamed to citus_schema)
-- back to public.
--
-- This can only happen if upgrade_columnar_before.sql is run multiple
-- times for flaky test detection.
DROP TABLE citus_schema.new_columnar_table;
DROP SCHEMA public CASCADE;
ALTER SCHEMA citus_schema RENAME TO public;
SET LOCAL client_min_messages TO WARNING;
DROP SCHEMA upgrade_columnar CASCADE;
END IF;
END
$$
LANGUAGE plpgsql;
ALTER SCHEMA public RENAME TO citus_schema;
SET search_path TO citus_schema;
-- As mentioned in https://github.com/citusdata/citus/issues/5447, it

33
src/test/regress/sql/multi_extension.sql
View File

@ -556,6 +556,39 @@ ALTER EXTENSION citus UPDATE TO '11.2-1';
SELECT * FROM pg_dist_placement ORDER BY shardid;
SELECT * FROM pg_dist_cleanup;
ALTER EXTENSION citus_columnar UPDATE TO '11.2-1';
-- Make sure that we defined dependencies from all rel objects (tables,
-- indexes, sequences ..) to columnar table access method ...
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
objid = (select oid from pg_am where amname = 'columnar') AND
objsubid = 0 AND
refclassid = 'pg_class'::regclass::oid AND
refobjsubid = 0 AND
deptype = 'n';
-- ... , so this should be empty,
(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend)
UNION
(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members);
-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members
-- should have 5 entries.
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
-- error out as cleanup records remain
ALTER EXTENSION citus UPDATE TO '11.0-4';

38
src/test/regress/sql/upgrade_columnar_after.sql
View File

@ -101,10 +101,12 @@ BEGIN;
INSERT INTO text_data (value) SELECT generate_random_string(1024 * 10) FROM generate_series(0,10);
SELECT count(DISTINCT value) FROM text_data;
-- make sure that serial is preserved
-- since we run "after schedule" twice and "rollback" wouldn't undo
-- sequence changes, it can be 22 or 33, not a different value
SELECT max(id) in (22, 33) FROM text_data;
-- Make sure that serial is preserved.
--
-- Since we might run "after schedule" several times for flaky test
-- detection and "rollback" wouldn't undo sequence changes, "id" should
-- look like below:
SELECT max(id) >= 11 AND max(id) % 11 = 0 FROM text_data;
-- since we run "after schedule" twice, rollback the transaction
-- to avoid getting "table already exists" errors
@ -137,7 +139,12 @@ ROLLBACK;
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal';
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
@ -153,19 +160,24 @@ UNION
(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members);
-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members
-- should have 10 entries.
SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend;
-- should have 5 entries.
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
-- Check the same for workers too.
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
SELECT pg_class.oid INTO columnar_schema_members
FROM pg_class, pg_namespace
WHERE pg_namespace.oid=pg_class.relnamespace AND
pg_namespace.nspname='columnar_internal';
pg_namespace.nspname='columnar_internal' AND
pg_class.relname NOT IN ('chunk_group_pkey',
'chunk_pkey',
'options_pkey',
'stripe_first_row_number_idx',
'stripe_pkey');
SELECT refobjid INTO columnar_schema_members_pg_depend
FROM pg_depend
WHERE classid = 'pg_am'::regclass::oid AND
@ -177,7 +189,7 @@ WHERE classid = 'pg_am'::regclass::oid AND
$$
);
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend)
UNION
@ -185,13 +197,13 @@ UNION
$$
);
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend;
SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend;
$$
);
SELECT run_command_on_workers(
SELECT success, result FROM run_command_on_workers(
$$
DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend;
$$

24
src/test/regress/sql/upgrade_columnar_before.sql
View File

@ -1,5 +1,29 @@
-- Test if relying on topological sort of the objects, not their names, works
-- fine when re-creating objects during pg_upgrade.
DO
$$
BEGIN
IF EXISTS (SELECT * FROM pg_namespace WHERE nspname = 'upgrade_columnar')
THEN
-- Drop the the table leftover from the earlier run of
-- upgrade_columnar_before.sql. Similarly, drop the fake public schema
-- created before and rename the original one (renamed to citus_schema)
-- back to public.
--
-- This can only happen if upgrade_columnar_before.sql is run multiple
-- times for flaky test detection.
DROP TABLE citus_schema.new_columnar_table;
DROP SCHEMA public CASCADE;
ALTER SCHEMA citus_schema RENAME TO public;
SET LOCAL client_min_messages TO WARNING;
DROP SCHEMA upgrade_columnar CASCADE;
END IF;
END
$$
LANGUAGE plpgsql;
ALTER SCHEMA public RENAME TO citus_schema;
SET search_path TO citus_schema;