Thread: Potential bug in pg_dump ...

Got a report the other day of a "problem" with pg_dump where, if in the
middle of a dump, someone happens to drop a table, it errors out with:
       SQL query to dump the contents of Table 'server_2' did not execute.
Explanation from backend: 'ERROR:  Relation 'server_2' does not exist'.       The query was: 'COPY "server_2" TO
stdout;'.

Now, altho I can't imagine it happening often, I know I'd be a bit annoyed
if I came in the next morning, tried to restore what was in the table that
should have been dumped *after* this one, and found that my dump didn't
work :(

This is using a v7.1.3 ... and I know there has been *alot* of changes to
pg_dump since, so this might have already been caught/dealt with ... ?

At 14:10 17/12/01 -0500, Marc G. Fournier wrote:
>
>Got a report the other day of a "problem" with pg_dump where, if in the
>middle of a dump, someone happens to drop a table, it errors out with:
>

pg_dump runs in a single TX, which should mean that metadata changes in
another process won't affect it. Have I misunderstood the way PG handles
metadata changes?


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

At 08:25 18/12/01 +1100, Philip Warner wrote:
>At 14:10 17/12/01 -0500, Marc G. Fournier wrote:
>>
>>Got a report the other day of a "problem" with pg_dump where, if in the
>>middle of a dump, someone happens to drop a table, it errors out with:
>>
>
>pg_dump runs in a single TX, which should mean that metadata changes in
>another process won't affect it. Have I misunderstood the way PG handles
>metadata changes?
>

Maybe I'm expectibg too much from the locking, since pg_dump is only
reading pg_class to get a list of tables. Perhaps it should do one of the
following:

a) Issue a LOCK TABLE for each table (seems like a bad idea)

b) Reconfirm the existance of the table before trying to dump it.

c) Ignore the problem

I favour either (b) or (c). Anyone have comments or other suggestions?


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/

Philip Warner <pjw@rhyme.com.au> writes:
> At 14:10 17/12/01 -0500, Marc G. Fournier wrote:
>> Got a report the other day of a "problem" with pg_dump where, if in the
>> middle of a dump, someone happens to drop a table, it errors out with:

> pg_dump runs in a single TX, which should mean that metadata changes in
> another process won't affect it. Have I misunderstood the way PG handles
> metadata changes?

In the case Marc is describing, pg_dump hasn't yet tried to touch the
table that someone else is dropping, so it has no lock on the table,
so the drop is allowed to occur.

A possible (partial) solution is for pg_dump to obtain a read-lock on
every table in the database as soon as it sees the table mentioned in
pg_class, rather than waiting till it's ready to read the contents of
the table.  However this cure might be worse than the disease,
particularly for people running "pg_dump -t table".
        regards, tom lane

[2001-12-17 17:06] Tom Lane said:
| Philip Warner <pjw@rhyme.com.au> writes:
| > At 14:10 17/12/01 -0500, Marc G. Fournier wrote:
| >> Got a report the other day of a "problem" with pg_dump where, if in the
| >> middle of a dump, someone happens to drop a table, it errors out with:
| 
| > pg_dump runs in a single TX, which should mean that metadata changes in
| > another process won't affect it. Have I misunderstood the way PG handles
| > metadata changes?
| 
| In the case Marc is describing, pg_dump hasn't yet tried to touch the
| table that someone else is dropping, so it has no lock on the table,
| so the drop is allowed to occur.
| 
| A possible (partial) solution is for pg_dump to obtain a read-lock on
| every table in the database as soon as it sees the table mentioned in
| pg_class, rather than waiting till it's ready to read the contents of
| the table.  However this cure might be worse than the disease,
| particularly for people running "pg_dump -t table".

How would this lock-when-seen approach cause problems with '-t'?

ISTM, that we could make getTables like
 tblinfo = getTables(&numTables, finfo, numFuncs, tablename);

so only that table gets locked when reading pg_class if tablename
isn't NULL, otherwise all tables get locked.

Aside from me not being familiar with the specifics of table locking,
avoiding the "table dropped during dump" condition looks 
straightforward and uncomplicated.   From reading the docs, an 
ACCESS SHARE lock should keep any pending ALTER TABLE from modifying
the table.

What am I overlooking?
 b

-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman

Brent Verner <brent@rcfile.org> writes:
> [2001-12-17 17:06] Tom Lane said:
> | A possible (partial) solution is for pg_dump to obtain a read-lock on
> | every table in the database as soon as it sees the table mentioned in
> | pg_class, rather than waiting till it's ready to read the contents of
> | the table.  However this cure might be worse than the disease,
> | particularly for people running "pg_dump -t table".

> How would this lock-when-seen approach cause problems with '-t'?

Locking the whole DB when you only want to dump one table might be seen
as a denial of service.  Also, consider the possibility that you don't
have the right to lock every table in the DB.

If we can arrange to lock only those tables that will end up getting
dumped, then these problems go away.  I have not looked to see if that's
a difficult change or not.
        regards, tom lane

[2002-01-09 18:55] Tom Lane said:
| Brent Verner <brent@rcfile.org> writes:
| > [2001-12-17 17:06] Tom Lane said:
| > | A possible (partial) solution is for pg_dump to obtain a read-lock on
| > | every table in the database as soon as it sees the table mentioned in
| > | pg_class, rather than waiting till it's ready to read the contents of
| > | the table.  However this cure might be worse than the disease,
| > | particularly for people running "pg_dump -t table".
| 
| > How would this lock-when-seen approach cause problems with '-t'?
| 
| Locking the whole DB when you only want to dump one table might be seen
| as a denial of service.  Also, consider the possibility that you don't
| have the right to lock every table in the DB.
| 
| If we can arrange to lock only those tables that will end up getting
| dumped, then these problems go away.  I have not looked to see if that's
| a difficult change or not.

We can try to lock one or lock all very easily.  An ACCESS SHARE
lock is granted to the user having SELECT privs, if they don't have
SELECT privs, they'll not have much luck dumping data anyway.

thanks. brent


-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman

[2002-01-09 19:19] Brent Verner said:
| [2002-01-09 18:55] Tom Lane said:
| | Brent Verner <brent@rcfile.org> writes:
| | > [2001-12-17 17:06] Tom Lane said:
| | > | A possible (partial) solution is for pg_dump to obtain a read-lock on
| | > | every table in the database as soon as it sees the table mentioned in
| | > | pg_class, rather than waiting till it's ready to read the contents of
| | > | the table.  However this cure might be worse than the disease,
| | > | particularly for people running "pg_dump -t table".
| |
| | > How would this lock-when-seen approach cause problems with '-t'?
| |
| | Locking the whole DB when you only want to dump one table might be seen
| | as a denial of service.  Also, consider the possibility that you don't
| | have the right to lock every table in the DB.
| |
| | If we can arrange to lock only those tables that will end up getting
| | dumped, then these problems go away.  I have not looked to see if that's
| | a difficult change or not.
|
| We can try to lock one or lock all very easily.  An ACCESS SHARE
| lock is granted to the user having SELECT privs, if they don't have
| SELECT privs, they'll not have much luck dumping data anyway.

Attached is a patch that implements table locking for pg_dump.

If a tablename is specified with '-t tablename', only that table will
be locked; otherwise, all tables will be locked.  Locks are with
ACCESS SHARE to block only concurrent AccessExclusiveLock operations
on the table.

comments?

thanks.
  brent

--
"Develop your talent, man, and leave the world something. Records are
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman

Brent Verner <brent@rcfile.org> writes:
> Attached is a patch that implements table locking for pg_dump.

Checked and applied, with some small tweaking.  I broke the outer loop
of getTables() into two loops, one that extracts data from the pg_class
SELECT result and locks the target tables, and a second one that does
the rest of the stuff that that routine does.  This is to minimize the
time window between doing the pg_class SELECT and locking the tables.

In testing this thing, I noticed that pg_dump spends a really
unreasonable amount of time on schema extraction.  For example, on the
regression database the actual COPY commands take less than a quarter of
the runtime.  (Of course, regression deliberately doesn't contain huge
volumes of data, so this case may be unrepresentative of real-world
situations.)  The retail queries to get table attributes, descriptions,
etc are probably the cause.  Something to think about improving
someday...
        regards, tom lane

[2002-01-11 18:30] Tom Lane said:
| Brent Verner <brent@rcfile.org> writes:
| > Attached is a patch that implements table locking for pg_dump.
| 
| Checked and applied, with some small tweaking.  I broke the outer loop
| of getTables() into two loops, one that extracts data from the pg_class
| SELECT result and locks the target tables, and a second one that does
| the rest of the stuff that that routine does.  This is to minimize the
| time window between doing the pg_class SELECT and locking the tables.

ACK.  Now I understand what you meant by the "more than zero time"
to lock the tables :-)

thanks. brent

-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman