Обсуждение: BUG #8257: Multi-Core Restore fails when containing index comments

The following bug has been logged on the website:

Bug reference:      8257
Logged by:          Lloyd Albin
Email address:      lalbin@fhcrc.org
PostgreSQL version: 9.2.4
Operating system:   SUSE Linux (64-bit)
Description:

I have found the restore will fail when using pg_restore's -j option, with
more than one core, on a dump that contains a COMMENT INDEX.


I have tried this using both Postgres 9.0.12 & 9.2.4


I have created a test case that shows the problem.


Create a database called test_db.


CREATE DATABASE test_db
  WITH OWNER = postgres
    ENCODING = 'UTF8'
    TEMPLATE = template0;


Run this next section to add the table, index, and index comment to the
test_db database.


CREATE TABLE public.tbl_test (
  pkey TEXT NOT NULL,
  CONSTRAINT tbl_test_pkey PRIMARY KEY(pkey)
);


COMMENT ON INDEX public.tbl_test_pkey
IS 'Index Comment';


Once this test database is created, create a backup of the database.


pg_dump -Fc test_db > test_db.dump


Drop the test database and now lets restore it.


pg_restore -C -j 3 -d postgres -Fc test_db.dump


pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2525; 0 0 COMMENT INDEX
tbl_test_pkey postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation
"tbl_test_pkey" does not exist
    Command was: COMMENT ON INDEX tbl_test_pkey IS 'Index Comment';






pg_restore: [archiver] worker process failed: exit code 1


If you use -j 1 or skip the -j option the restore will restore without any
errors.


Lloyd Albin
Statistical Center for HIV/AIDS Research and Prevention (SCHARP)
Vaccine and Infectious Disease Division (VIDD)
Fred Hutchinson Cancer Research Center (FHCRC)

Hi Lloyd,

On 2013-06-26 23:43:00 +0000, lalbin@fhcrc.org wrote:
> I have found the restore will fail when using pg_restore's -j option, with
> more than one core, on a dump that contains a COMMENT INDEX.

> Run this next section to add the table, index, and index comment to the
> test_db database.

> CREATE TABLE public.tbl_test (
>   pkey TEXT NOT NULL,
>   CONSTRAINT tbl_test_pkey PRIMARY KEY(pkey)
> );

> COMMENT ON INDEX public.tbl_test_pkey
> IS 'Index Comment';

> Once this test database is created, create a backup of the database.
> pg_dump -Fc test_db > test_db.dump

The problem is that pg_dump makes the comment depend on the index
instead of the constraint:

; Selected TOC Entries:
...
170; 1259 69261 TABLE public tbl_test andres
;    depends on: 6
1941; 0 69261 TABLE DATA public tbl_test andres
;     depends on: 170
1833; 2606 69268 CONSTRAINT public tbl_test_pkey andres
;     depends on: 170 170
1950; 0 0 COMMENT public INDEX tbl_test_pkey andres
;     depends on: 1832

There is no object 1832 in the dump since that was ommitted in favor of
the constraint 1833 which internally creates the index. So what we need
to do is to make the comment depend on the constraint instead.

With the attached patch we get:

170; 1259 69261 TABLE public tbl_test andres
;    depends on: 6
1941; 0 69261 TABLE DATA public tbl_test andres
;     depends on: 170
1833; 2606 69268 CONSTRAINT public tbl_test_pkey andres
;     depends on: 170 170
1950; 0 0 COMMENT public INDEX tbl_test_pkey andres
;     depends on: 1833

unsurprisingly after that restore completes.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres@2ndquadrant.com> writes:
> The problem is that pg_dump makes the comment depend on the index
> instead of the constraint:

Yeah, I figured that out yesterday, but hadn't gotten to writing a patch
yet.

> ... So what we need
> to do is to make the comment depend on the constraint instead.

Your proposed patch will only fix the problem for dumps created after
it ships.  In the past, we've tried to deal with this type of issue by
having pg_restore fix up the dependencies when reading a dump, so that
it would still work on existing dumps.

I'm afraid there may be no way to do that in this case --- it doesn't
look like there's enough info in the dump to tell where the dependency
link should have led.  But we should think about it a little before
taking the easy way out.

            regards, tom lane

On 2013-06-27 10:29:14 -0400, Tom Lane wrote:
> > ... So what we need
> > to do is to make the comment depend on the constraint instead.

> Your proposed patch will only fix the problem for dumps created after
> it ships.  In the past, we've tried to deal with this type of issue by
> having pg_restore fix up the dependencies when reading a dump, so that
> it would still work on existing dumps.

Yes :(. On the other hand, it's probably not too common to create
comments on indexes that haven't been created explicitly.

> I'm afraid there may be no way to do that in this case --- it doesn't
> look like there's enough info in the dump to tell where the dependency
> link should have led.  But we should think about it a little before
> taking the easy way out.

The only thing I could think of - but which I thought to be too kludgey
- was to simply delay the creation of all comments and restore them
together with ACLs. I don't think we can have dependencies towards
comments.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-06-27 10:29:14 -0400, Tom Lane wrote:
>> Your proposed patch will only fix the problem for dumps created after
>> it ships.  In the past, we've tried to deal with this type of issue by
>> having pg_restore fix up the dependencies when reading a dump, so that
>> it would still work on existing dumps.

> Yes :(. On the other hand, it's probably not too common to create
> comments on indexes that haven't been created explicitly.

Perhaps.  The lack of previous complaints does suggest this situation
isn't so common.

>> I'm afraid there may be no way to do that in this case --- it doesn't
>> look like there's enough info in the dump to tell where the dependency
>> link should have led.  But we should think about it a little before
>> taking the easy way out.

> The only thing I could think of - but which I thought to be too kludgey
> - was to simply delay the creation of all comments and restore them
> together with ACLs.

I don't like that either, though we may be forced into it if we find
more bugs in comment dependencies.

Anyway, fixing pg_dump's logic is not wrong; I was just hoping we could
also think of a workaround on the pg_restore side.

            regards, tom lane

Andres Freund <andres@2ndquadrant.com> writes:
> There is no object 1832 in the dump since that was ommitted in favor of
> the constraint 1833 which internally creates the index. So what we need
> to do is to make the comment depend on the constraint instead.
> With the attached patch we get: [ the right thing ]

Applied with minor cosmetic changes.

            regards, tom lane