Fundamental scheduling bug in parallel restore of partitioned tables - Mailing list pgsql-hackers

Looking into the complaint at [1], I find that pg_restore really gets
it quite wrong when trying to do a parallel restore of partitioned
tables with foreign key constraints.  The historical way that we
got parallel restore to work correctly with FK constraints is:

1. The dump file shows dependencies of the FK constraint object on
the two tables named in the constraint.  (This matches what it
says in pg_depend.)

2. If it's a parallel restore, repoint_table_dependencies() replaces
the dependencies on the tables with dependencies on their TABLE DATA
objects.

3. Now restore of the FK constraint will not be started until all
the relevant data is loaded.

However, if we're talking about partitioned tables, there is no
TABLE DATA object for a partitioned table; only for its leaf
partitions.  So repoint_table_dependencies() does nothing for the
FK object's dependencies, meaning that it's a candidate to be
launched as soon as we begin the parallel restore phase.

This is disastrous for assorted reasons.  The ALTER ADD CONSTRAINT
command might fail outright if we've loaded data for the referencing
table but not the referenced table.  It could deadlock against other
parallel restore jobs, as reported in [1] (and which I find not
too terribly hard to reproduce here).  Even if it doesn't fail, if
it completes before we load data for the referencing table, we'll
have to do retail FK checks, greatly slowing that data load.

I think that the most intellectually rigorous solution is to
generate dummy TABLE DATA objects for partitioned tables, which
don't actually contain data but merely carry dependencies on
each of the child tables' TABLE DATA objects.  (In a multilevel
partitioned structure, this'd result in the top TABLE DATA having
indirect dependencies on all the leaf partition TABLE DATAs.)
Then repoint_table_dependencies() does the right thing with a
dependency on a partitioned table, and the dependency-driven
scheduler will take care of the rest.

There are two places we could make that happen.  The easiest
to code, likely, is to get pg_dump to create such objects and
include them in the dump.  We could possibly get pg_restore
to fake up such objects from the data it has available, but
I expect that that will be harder and slower than having
pg_dump do it.  So I'm leaning towards the first way.
The disadvantage is that existing dump files would still be
hazardous to restore in parallel.  But given that this has
been broken for some time and nobody's reported it till now,
I feel maybe that's okay.  (I don't think there would be
a backwards compatibility problem in introducing such new
objects into dumps, because AFAICS pg_restore would not need
any explicit knowledge of them.)

Thoughts?

            regards, tom lane

PS: attached is a text dump file for a trivial DB containing
two partitioned tables with an FK.  If I load this,
dump it into an -Fc-format dump file, and run something like
    pg_restore -j10 src.dump -d target
then I can reproduce the reported deadlock failure --- not
entirely reliably, but it does happen.

[1] https://www.postgresql.org/message-id/67469c1c-38bc-7d94-918a-67033f5dd731%40gmx.net

--
-- PostgreSQL database dump
--

-- Dumped from database version 18devel
-- Dumped by pg_dump version 18devel

SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET transaction_timeout = 0;
SET client_encoding = 'SQL_ASCII';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET xmloption = content;
SET client_min_messages = warning;
SET row_security = off;

SET default_tablespace = '';

--
-- Name: parent1; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.parent1 (
    id integer NOT NULL,
    b text
)
PARTITION BY LIST (id);


ALTER TABLE public.parent1 OWNER TO postgres;

SET default_table_access_method = heap;

--
-- Name: c11; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.c11 (
    id integer CONSTRAINT parent1_id_not_null NOT NULL,
    b text
);


ALTER TABLE public.c11 OWNER TO postgres;

--
-- Name: c12; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.c12 (
    id integer CONSTRAINT parent1_id_not_null NOT NULL,
    b text
);


ALTER TABLE public.c12 OWNER TO postgres;

--
-- Name: parent2; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.parent2 (
    id integer NOT NULL,
    ref integer,
    b text
)
PARTITION BY LIST (id);


ALTER TABLE public.parent2 OWNER TO postgres;

--
-- Name: c21; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.c21 (
    id integer CONSTRAINT parent2_id_not_null NOT NULL,
    ref integer,
    b text
);


ALTER TABLE public.c21 OWNER TO postgres;

--
-- Name: c22; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.c22 (
    id integer CONSTRAINT parent2_id_not_null NOT NULL,
    ref integer,
    b text
);


ALTER TABLE public.c22 OWNER TO postgres;

--
-- Name: c11; Type: TABLE ATTACH; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent1 ATTACH PARTITION public.c11 FOR VALUES IN (1);


--
-- Name: c12; Type: TABLE ATTACH; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent1 ATTACH PARTITION public.c12 FOR VALUES IN (2);


--
-- Name: c21; Type: TABLE ATTACH; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent2 ATTACH PARTITION public.c21 FOR VALUES IN (1);


--
-- Name: c22; Type: TABLE ATTACH; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent2 ATTACH PARTITION public.c22 FOR VALUES IN (2);


--
-- Data for Name: c11; Type: TABLE DATA; Schema: public; Owner: postgres
--

COPY public.c11 (id, b) FROM stdin;
1    foo
\.


--
-- Data for Name: c12; Type: TABLE DATA; Schema: public; Owner: postgres
--

COPY public.c12 (id, b) FROM stdin;
\.


--
-- Data for Name: c21; Type: TABLE DATA; Schema: public; Owner: postgres
--

COPY public.c21 (id, ref, b) FROM stdin;
\.


--
-- Data for Name: c22; Type: TABLE DATA; Schema: public; Owner: postgres
--

COPY public.c22 (id, ref, b) FROM stdin;
2    1    bar
\.


--
-- Statistics for Name: c11; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c11',
    'relpages', '1'::integer,
    'reltuples', '1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: c12; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c12',
    'relpages', '0'::integer,
    'reltuples', '-1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: c21; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c21',
    'relpages', '0'::integer,
    'reltuples', '-1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: c22; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c22',
    'relpages', '1'::integer,
    'reltuples', '1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: parent1; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'parent1',
    'relpages', '0'::integer,
    'reltuples', '-1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: parent2; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'parent2',
    'relpages', '0'::integer,
    'reltuples', '-1'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Name: parent1 parent1_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent1
    ADD CONSTRAINT parent1_pkey PRIMARY KEY (id);


--
-- Name: c11 c11_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.c11
    ADD CONSTRAINT c11_pkey PRIMARY KEY (id);


--
-- Statistics for Name: c11_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c11_pkey',
    'relpages', '1'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Name: c12 c12_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.c12
    ADD CONSTRAINT c12_pkey PRIMARY KEY (id);


--
-- Statistics for Name: c12_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c12_pkey',
    'relpages', '1'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Name: parent2 parent2_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.parent2
    ADD CONSTRAINT parent2_pkey PRIMARY KEY (id);


--
-- Name: c21 c21_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.c21
    ADD CONSTRAINT c21_pkey PRIMARY KEY (id);


--
-- Statistics for Name: c21_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c21_pkey',
    'relpages', '1'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Name: c22 c22_pkey; Type: CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE ONLY public.c22
    ADD CONSTRAINT c22_pkey PRIMARY KEY (id);


--
-- Statistics for Name: c22_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'c22_pkey',
    'relpages', '1'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: parent1_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'parent1_pkey',
    'relpages', '0'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Statistics for Name: parent2_pkey; Type: STATISTICS DATA; Schema: public; Owner: -
--

SELECT * FROM pg_catalog.pg_restore_relation_stats(
    'version', '180000'::integer,
    'schemaname', 'public',
    'relname', 'parent2_pkey',
    'relpages', '0'::integer,
    'reltuples', '0'::real,
    'relallvisible', '0'::integer,
    'relallfrozen', '0'::integer
);


--
-- Name: c11_pkey; Type: INDEX ATTACH; Schema: public; Owner: postgres
--

ALTER INDEX public.parent1_pkey ATTACH PARTITION public.c11_pkey;


--
-- Name: c12_pkey; Type: INDEX ATTACH; Schema: public; Owner: postgres
--

ALTER INDEX public.parent1_pkey ATTACH PARTITION public.c12_pkey;


--
-- Name: c21_pkey; Type: INDEX ATTACH; Schema: public; Owner: postgres
--

ALTER INDEX public.parent2_pkey ATTACH PARTITION public.c21_pkey;


--
-- Name: c22_pkey; Type: INDEX ATTACH; Schema: public; Owner: postgres
--

ALTER INDEX public.parent2_pkey ATTACH PARTITION public.c22_pkey;


--
-- Name: parent2 parent2_ref_fkey; Type: FK CONSTRAINT; Schema: public; Owner: postgres
--

ALTER TABLE public.parent2
    ADD CONSTRAINT parent2_ref_fkey FOREIGN KEY (ref) REFERENCES public.parent1(id);


--
-- PostgreSQL database dump complete
--