Thread: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hello, O.k. this appeared easy enough for even me to do it. So I did. It seems to work but I wasn't 100% positive on "where" I put the code changes. Please take a look. Sincerely, Joshua D. Drake - -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL political pundit | Mocker of Dolphins -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFH1sVyATb/zqfZUUQRAkO/AJ4jncdM3NbbwXCVngitkadxxTAGawCeMBeZ Lnr2zCdV1WLijQl+dE5yUgU= =Qu8m -----END PGP SIGNATURE-----
Attachment
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Tue, 11 Mar 2008 10:46:23 -0700 "Joshua D. Drake" <jd@commandprompt.com> wrote: And the -c version :) (thanks bruce) Joshua D. Drake - -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL political pundit | Mocker of Dolphins -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFH1spfATb/zqfZUUQRAv2EAJ92/8EBxBbqLDlOX5wUYdN3ElG5OQCghZ2Z tUIrN/MYVgP6rc4QXONDrFg= =2oJ5 -----END PGP SIGNATURE-----
Attachment
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Tue, 11 Mar 2008 11:07:27 -0700 "Joshua D. Drake" <jd@commandprompt.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On Tue, 11 Mar 2008 10:46:23 -0700 > "Joshua D. Drake" <jd@commandprompt.com> wrote: > > And the -c version :) (thanks bruce) > > Joshua D. Drake > What is the feedback on this patch? Is there anything I need to do to get it into the next commit fest? Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Alvaro Herrera
Date:
Joshua D. Drake wrote: > What is the feedback on this patch? Is there anything I need to do to > get it into the next commit fest? Please add it to the commitfest wiki page. http://wiki.postgresql.org/wiki/CommitFest:May -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 13:36:50 -0400 Alvaro Herrera <alvherre@commandprompt.com> wrote: > Joshua D. Drake wrote: > > > What is the feedback on this patch? Is there anything I need to do > > to get it into the next commit fest? > > Please add it to the commitfest wiki page. > > http://wiki.postgresql.org/wiki/CommitFest:May > Done: http://wiki.postgresql.org/wiki/CommitFest:May#Pending_patches Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Heikki Linnakangas
Date:
Joshua D. Drake wrote: > What is the feedback on this patch? Is there anything I need to do to > get it into the next commit fest? Yes, go add it to the wiki page ;-): http://wiki.postgresql.org/wiki/CommitFest:May I agree that we should do that, but the thread on -hackers ("Autovacuum vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane and Peter Eisentraut argued that we shouldn't, but neither provided a plausible use case where a statement_timeout on restoring a dump would be useful. I'm thinking we should apply the patch unless someone comes up with one. To quote Tom: > I think we need to be careful to distinguish three situations: > > * statement_timeout during pg_dump > * statement_timeout during pg_restore > * statement_timeout during psql reading a pg_dump script file This patch addresses the third situation, but leaves open the 1st and the 2nd. IMO, we should set statement_timeout = 0 in them as well, unless someone comes up with plausible use case for using a non-zero statement_timeout. Ps. If you want to save the committer a couple of minutes of valuable time, you can fix the indentation to use tabs instead of spaces, and remove the spurious whitespace change on the empty line. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 21:04:17 +0300 Heikki Linnakangas <heikki@enterprisedb.com> wrote: > To quote Tom: > > I think we need to be careful to distinguish three situations: > > > > * statement_timeout during pg_dump > > * statement_timeout during pg_restore > > * statement_timeout during psql reading a pg_dump script file > > This patch addresses the third situation, but leaves open the 1st and > the 2nd. IMO, we should set statement_timeout = 0 in them as well, > unless someone comes up with plausible use case for using a non-zero > statement_timeout. My patch addresses all three, unless I am misunderstanding your meaning. The patch does the following: After connection with pg_dump it executes set statement_timeout = 0; This fixed the pg_dump timeout issue. It also writes set statement_timeout = 0 into the archive file, which fixed pg_restore and psql. > > Ps. If you want to save the committer a couple of minutes of valuable > time, you can fix the indentation to use tabs instead of spaces, and > remove the spurious whitespace change on the empty line. > I can do that. Thanks for the feedback. Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Heikki Linnakangas
Date:
Joshua D. Drake wrote: > On Wed, 16 Apr 2008 21:04:17 +0300 > Heikki Linnakangas <heikki@enterprisedb.com> wrote: >> To quote Tom: >>> I think we need to be careful to distinguish three situations: >>> >>> * statement_timeout during pg_dump >>> * statement_timeout during pg_restore >>> * statement_timeout during psql reading a pg_dump script file >> This patch addresses the third situation, but leaves open the 1st and >> the 2nd. IMO, we should set statement_timeout = 0 in them as well, >> unless someone comes up with plausible use case for using a non-zero >> statement_timeout. > > My patch addresses all three, unless I am misunderstanding your > meaning. The patch does the following: > > After connection with pg_dump it executes set statement_timeout = 0; > This fixed the pg_dump timeout issue. > > It also writes set statement_timeout = 0 into the archive file, which > fixed pg_restore and psql. Oh, ok, I misread the patch. Sorry for the noise. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 21:20:17 +0300 Heikki Linnakangas <heikki@enterprisedb.com> wrote: > > My patch addresses all three, unless I am misunderstanding your > > meaning. The patch does the following: > > > > After connection with pg_dump it executes set statement_timeout = 0; > > This fixed the pg_dump timeout issue. > > > > It also writes set statement_timeout = 0 into the archive file, > > which fixed pg_restore and psql. > > Oh, ok, I misread the patch. Sorry for the noise. > :) Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Greg Sabino Mullane"
Date:
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > I agree that we should do that, but the thread on -hackers ("Autovacuum > vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane > and Peter Eisentraut argued that we shouldn't, but neither provided a > plausible use case where a statement_timeout on restoring a dump would > be useful. I'm thinking we should apply the patch unless someone comes > up with one. I don't think it's fair to simply discard the use cases provided as "implausible" and demand one more to your liking. I strongly dislike having a giant dump file written that has non-vital configuration variables embedded in the top of it, precluding any user choice whatsoever[1]. As before, where are the reports of all the people having their manual restorations interrupted by a statement_timeout? [1] Short of editing a potentially GB-size file, or using some sed/shell shenanigans to strip out the suspect setting. - -- Greg Sabino Mullane greg@turnstep.com End Point Corporation PGP Key: 0x14964AC8 200804161814 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAkgGetEACgkQvJuQZxSWSsg+4ACghvlBkIth1aBiGpFPFPj+HWgf iyEAnj+WK9MQL+ZQqKoTcLOe/lvoNkkV =Nlyg -----END PGP SIGNATURE-----
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 22:17:30 -0000 "Greg Sabino Mullane" <greg@turnstep.com> wrote: > I don't think it's fair to simply discard the use cases provided as > "implausible" and demand one more to your liking. I strongly dislike > having a giant dump file written that has non-vital configuration > variables embedded in the top of it, precluding any user choice > whatsoever[1]. As before, where are the reports of all the people > having their manual restorations interrupted by a statement_timeout? Calling me, wondering why in the world it is happening. Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 15:22:31 -0700 "Joshua D. Drake" <jd@commandprompt.com> wrote: > On Wed, 16 Apr 2008 22:17:30 -0000 > "Greg Sabino Mullane" <greg@turnstep.com> wrote: > > > I don't think it's fair to simply discard the use cases provided as > > "implausible" and demand one more to your liking. I strongly dislike > > having a giant dump file written that has non-vital configuration > > variables embedded in the top of it, precluding any user choice > > whatsoever[1]. As before, where are the reports of all the people > > having their manual restorations interrupted by a statement_timeout? > > Calling me, wondering why in the world it is happening. Sorry couldn't help myself. Anyway, in an attempt to be productive, I will say that your "where are all the reports" is about as valid as, "Where are all the users besides yourself arguing about this having to edit a backup file?" This is a real problem and unless we can find more people to substantiate your claim, I think the consensus is to ensure that people don't get bit by statement timeout when attempting to do a restore. I vote in favor of the one less foot gun approach. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Alex Hunsaker"
Date:
On Wed, Apr 16, 2008 at 4:32 PM, Joshua D. Drake <jd@commandprompt.com> wrote: > On Wed, 16 Apr 2008 15:22:31 -0700 > > "Joshua D. Drake" <jd@commandprompt.com> wrote: > > > > On Wed, 16 Apr 2008 22:17:30 -0000 > > "Greg Sabino Mullane" <greg@turnstep.com> wrote: > > > > > I don't think it's fair to simply discard the use cases provided as > > > "implausible" and demand one more to your liking. I strongly dislike > > > having a giant dump file written that has non-vital configuration > > > variables embedded in the top of it, precluding any user choice > > > whatsoever[1]. As before, where are the reports of all the people > > > having their manual restorations interrupted by a statement_timeout? > > > > Calling me, wondering why in the world it is happening. > > Sorry couldn't help myself. Anyway, in an attempt to be productive, I > will say that your "where are all the reports" is about as valid as, > "Where are all the users besides yourself arguing about this having to > edit a backup file?" > > This is a real problem and unless we can find more people to > substantiate your claim, I think the consensus is to ensure that people > don't get bit by statement timeout when attempting to do a restore. > > I vote in favor of the one less foot gun approach. Sorry if i missed the obvious reason not to do this... but if its a command line option the user can choose. Why not something like this (i did it for pg_dump only...) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ed1b33d..bf9365a 100644 *** a/src/bin/pg_dump/pg_dump.c --- /bsrc/bin/pg_dump/pg_dump.c *************** main(int argc, char **argv) *** 225,230 **** --- 225,231 ---- int outputNoOwner = 0; static int use_setsessauth = 0; staticint disable_triggers = 0; + static int use_statement_timeout = 0; char *outputSuperuser = NULL; RestoreOptions *ropt; *************** main(int argc, char **argv) *** 267,272 **** --- 268,274 ---- {"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1}, {"disable-triggers", no_argument, &disable_triggers, 1}, {"use-set-session-authorization",no_argument, &use_setsessauth, 1}, + {"use-statement-timeout", no_argument, &use_statement_timeout, 1}, {NULL, 0, NULL, 0} }; *************** main(int argc, char **argv) *** 419,424 **** --- 421,428 ---- disable_triggers = 1; else if (strcmp(optarg, "use-set-session-authorization") == 0) use_setsessauth = 1; + else if (strcmp(optarg, "use-statement-timeout") == 0) + use_statement_timeout = 1; else { fprintf(stderr, *************** main(int argc, char **argv) *** 571,576 **** --- 575,583 ---- */ do_sql_command(g_conn, "BEGIN"); + if (!use_statement_timeout) + do_sql_command(g_conn, "SET statement_timeout = 0;"); + do_sql_command(g_conn, "SET TRANSACTION ISOLATION LEVEL SERIALIZABLE"); /* Select the appropriate subquery to convert user IDs to names */ *************** help(const char *progname) *** 771,776 **** --- 778,784 ---- printf(_(" --use-set-session-authorization\n" " use SESSION AUTHORIZATION commands instead of\n" " ALTER OWNER commands to set ownership\n")); + printf(_(" --use-statement-timeout respect statement_timeout\n")); printf(_("\nConnection options:\n")); printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Andrew Dunstan
Date:
Alex Hunsaker wrote: > > > Sorry if i missed the obvious reason not to do this... but if its a > command line option the user can choose. Why not something like this > (i did it for pg_dump only...) > > > Actually, it's probably more important to be selectable at restore time than at dump time, so if you're doing just one ... This whole thing set me wondering whether or not we should provide a more general command-line facility to psql and pg_restore, and maybe others, to do some session setup before running their commands. Of course, there's no reason we couldn't have both. cheers andrew
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
On Wed, 16 Apr 2008 18:50:28 -0400 Andrew Dunstan <andrew@dunslane.net> wrote: > > > Alex Hunsaker wrote: > > > > > > Sorry if i missed the obvious reason not to do this... but if its a > > command line option the user can choose. Why not something like > > this (i did it for pg_dump only...) > > > > > > > Actually, it's probably more important to be selectable at restore > time than at dump time, so if you're doing just one ... > > This whole thing set me wondering whether or not we should provide a > more general command-line facility to psql and pg_restore, and maybe > others, to do some session setup before running their commands. > > Of course, there's no reason we couldn't have both. That is an interesting idea. Something like: pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? Sincerely, Joshua D. Drake > > cheers > > andrew > -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Alvaro Herrera
Date:
Joshua D. Drake escribió: > On Wed, 16 Apr 2008 18:50:28 -0400 > Andrew Dunstan <andrew@dunslane.net> wrote: > > Actually, it's probably more important to be selectable at restore > > time than at dump time, so if you're doing just one ... I think the patch posted by Joshua at the start of this thread does that. > > This whole thing set me wondering whether or not we should provide a > > more general command-line facility to psql and pg_restore, and maybe > > others, to do some session setup before running their commands. > > That is an interesting idea. Something like: > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? We already have it -- it's called PGOPTIONS. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Alex Hunsaker"
Date:
On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Joshua D. Drake escribió: > > > That is an interesting idea. Something like: > > > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? > > We already have it -- it's called PGOPTIONS. > Ok but is not the purpose of the patch to turn off statement_timeout by *default* in pg_restore/pg_dump? Here is an updated patch for I posted above (with the command line option --use-statement-timeout) for pg_dump and pg_restore. (sorry If I hijacked your patch Josh :) )
Attachment
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Heikki Linnakangas
Date:
Greg Sabino Mullane wrote: >> I agree that we should do that, but the thread on -hackers ("Autovacuum >> vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane >> and Peter Eisentraut argued that we shouldn't, but neither provided a >> plausible use case where a statement_timeout on restoring a dump would >> be useful. I'm thinking we should apply the patch unless someone comes >> up with one. > > I don't think it's fair to simply discard the use cases provided as > "implausible" and demand one more to your liking. Sorry if I missed it in the original thread, but what is the use case you have in mind? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Peter Eisentraut
Date:
Heikki Linnakangas wrote: > Sorry if I missed it in the original thread, but what is the use case > you have in mind? I think the bottom line is just that having statement_timeout a global setting is stupid for a variety of reasons (dump, restore, vacuum, locks, incidental delays) that we should discourage it (or prevent it, as proposed elsewhere) rather than working around it in countless individual places.
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes: > Heikki Linnakangas wrote: >> Sorry if I missed it in the original thread, but what is the use case >> you have in mind? > I think the bottom line is just that having statement_timeout a global setting > is stupid for a variety of reasons (dump, restore, vacuum, locks, incidental > delays) that we should discourage it (or prevent it, as proposed elsewhere) > rather than working around it in countless individual places. I'm not convinced that there's no use-case for global statement_timeout, and even less convinced that there won't be anyone setting one anyway. Unless we are prepared to somehow *prevent* such a setting from being put in place, the proposed patch seems reasonable to me. Unless you have a use-case in which it's actually desirable for the dump or restore to fail. I'm having a tough time imagining one though. regards, tom lane
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Greg Sabino Mullane"
Date:
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > Sorry if I missed it in the original thread, but what is the > use case you have in mind? A use case would be dumping a large table and wanting to load it into the database, but wanting to stop the job if it is still running an hour from now, when a maintenance window is scheduled to start. However, I think my objection is more philosophical, as we've changed from having pg_dump make a SQL representation of part or all of your database, into also having it force what it thinks should be the right environment as well. Yes, timeout can be a foot gun, but it's a foot gun that off by default, and must be explicitly turned on. The fact that a setting that kills long-running queries ends up killing long-running queries via psql -f seems worth a documentation warning, not a change in the textual representation of a database. I checked the archives and have yet to find a single instance in -bugs of anyone complaining about this. The closest I found was someone having problems with psql and -c, but they were specifically aware of the timeout and were trying (unseccessfully) to disable it first. For the record, I have no problem with disabling the timeout in both pg_dump and pg_restore. - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200804171250 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAkgHf/sACgkQvJuQZxSWSsiXOwCggD1P/SgPwOO3gJdlXKP5bU3l dWgAnRK5FNixLy8ajgkfI3Y/UpDyoeZB =qaA5 -----END PGP SIGNATURE-----
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Heikki Linnakangas
Date:
Greg Sabino Mullane wrote: > A use case would be dumping a large table and wanting to > load it into the database, but wanting to stop the job if it > is still running an hour from now, when a maintenance window > is scheduled to start. statement_timeout is pretty useless for that purpose, because it limits the time on a per-statement basis. It would cancel the COPY of any tables larger than X, but if you have multiple tables (a partitioned table, perhaps) just below the threshold, they would all be dumped even though the cumulative time is well beyond statement_timeout. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Martijn van Oosterhout
Date:
On Thu, Apr 17, 2008 at 03:18:54PM +0200, Peter Eisentraut wrote: > I think the bottom line is just that having statement_timeout a global setting > is stupid for a variety of reasons (dump, restore, vacuum, locks, incidental > delays) that we should discourage it (or prevent it, as proposed elsewhere) > rather than working around it in countless individual places. maintainence_statement_timeout? :) Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines.
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Andrew Dunstan
Date:
Joshua D. Drake wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On Tue, 11 Mar 2008 10:46:23 -0700 > "Joshua D. Drake" <jd@commandprompt.com> wrote: > > And the -c version :) (thanks bruce) > > > Committed with slight editorializing. Statement timeout was only introduced in 7.3, whereas pg_dump can dump from much older versions of Postgres. Also, the indentation needed fixing. wiki updated. cheers andrew
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Euler Taveira de Oliveira
Date:
Andrew Dunstan wrote: > Committed with slight editorializing. Statement timeout was only > introduced in 7.3, whereas pg_dump can dump from much older versions of > Postgres. > You forget a ; in this committ [1]. [1] http://archives.postgresql.org/pgsql-committers/2008-05/msg00028.php -- Euler Taveira de Oliveira http://www.timbira.com/
Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Andrew Dunstan
Date:
Euler Taveira de Oliveira wrote: > Andrew Dunstan wrote: > >> Committed with slight editorializing. Statement timeout was only >> introduced in 7.3, whereas pg_dump can dump from much older versions >> of Postgres. >> > You forget a ; in this committ [1]. > > [1] http://archives.postgresql.org/pgsql-committers/2008-05/msg00028.php > I need to stop doing things late at night. fixed. cheers andrew.
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Bruce Momjian
Date:
Alex Hunsaker wrote: > On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Joshua D. Drake escribi?: > > > > > That is an interesting idea. Something like: > > > > > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? > > > > We already have it -- it's called PGOPTIONS. > > > > Ok but is not the purpose of the patch to turn off statement_timeout > by *default* in pg_restore/pg_dump? > > Here is an updated patch for I posted above (with the command line > option --use-statement-timeout) for pg_dump and pg_restore. I would like to get do this without adding a new --use-statement-timeout flag. Is anyone going to want to honor statement_timeout during pg_dump/pg_restore? I thought we were just going to disable it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
daveg
Date:
On Mon, Jun 23, 2008 at 06:51:28PM -0400, Bruce Momjian wrote: > Alex Hunsaker wrote: > > On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera > > <alvherre@commandprompt.com> wrote: > > > Joshua D. Drake escribi?: > > > > > > > That is an interesting idea. Something like: > > > > > > > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? > > > > > > We already have it -- it's called PGOPTIONS. > > > > > > > Ok but is not the purpose of the patch to turn off statement_timeout > > by *default* in pg_restore/pg_dump? > > > > Here is an updated patch for I posted above (with the command line > > option --use-statement-timeout) for pg_dump and pg_restore. > > I would like to get do this without adding a new --use-statement-timeout > flag. Is anyone going to want to honor statement_timeout during > pg_dump/pg_restore? I thought we were just going to disable it. I have a patch in the queue to use set statement timeout while pg_dump is taking locks to avoid pg_dump hanging for other long running transactions that may have done ddl. Do I need to repost for discussion now? -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Bruce Momjian
Date:
daveg wrote: > On Mon, Jun 23, 2008 at 06:51:28PM -0400, Bruce Momjian wrote: > > Alex Hunsaker wrote: > > > On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera > > > <alvherre@commandprompt.com> wrote: > > > > Joshua D. Drake escribi?: > > > > > > > > > That is an interesting idea. Something like: > > > > > > > > > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? > > > > > > > > We already have it -- it's called PGOPTIONS. > > > > > > > > > > Ok but is not the purpose of the patch to turn off statement_timeout > > > by *default* in pg_restore/pg_dump? > > > > > > Here is an updated patch for I posted above (with the command line > > > option --use-statement-timeout) for pg_dump and pg_restore. > > > > I would like to get do this without adding a new --use-statement-timeout > > flag. Is anyone going to want to honor statement_timeout during > > pg_dump/pg_restore? I thought we were just going to disable it. > > I have a patch in the queue to use set statement timeout while pg_dump is > taking locks to avoid pg_dump hanging for other long running transactions > that may have done ddl. Do I need to repost for discussion now? I see it now, but I forgot how it would interact with this patch. We would have to prevent --use-statement-timeout when lock timeout was being used, but my point is that I see no value in having --use-statement-timeout. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Alex Hunsaker"
Date:
On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <bruce@momjian.us> wrote: > I would like to get do this without adding a new --use-statement-timeout > flag. Is anyone going to want to honor statement_timeout during > pg_dump/pg_restore? I thought we were just going to disable it. I believe so. This was when not everyone was convinced. Im fairly certain Josh original patch is in the commit fest. So feel free to drop this one.
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Bruce Momjian
Date:
Alex Hunsaker wrote: > On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <bruce@momjian.us> wrote: > > I would like to get do this without adding a new --use-statement-timeout > > flag. Is anyone going to want to honor statement_timeout during > > pg_dump/pg_restore? I thought we were just going to disable it. > > I believe so. This was when not everyone was convinced. Im fairly > certain Josh original patch is in the commit fest. So feel free to > drop this one. I certainly don't see any version of Drake's patch in the July commitfest: http://wiki.postgresql.org/wiki/CommitFest I am thinking I will just remove the option and commit it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
"Joshua D. Drake"
Date:
Alex Hunsaker wrote: > On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <bruce@momjian.us> wrote: >> I would like to get do this without adding a new --use-statement-timeout >> flag. Is anyone going to want to honor statement_timeout during >> pg_dump/pg_restore? I thought we were just going to disable it. > > I believe so. This was when not everyone was convinced. Im fairly > certain Josh original patch is in the commit fest. So feel free to > drop this one. > My patch has been committed. Joshua D. Drake
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Bruce Momjian
Date:
Joshua D. Drake wrote: > Alex Hunsaker wrote: > > On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> I would like to get do this without adding a new --use-statement-timeout > >> flag. Is anyone going to want to honor statement_timeout during > >> pg_dump/pg_restore? I thought we were just going to disable it. > > > > I believe so. This was when not everyone was convinced. Im fairly > > certain Josh original patch is in the commit fest. So feel free to > > drop this one. > > > > My patch has been committed. Ah, I see, but with no switch. Thanks. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
daveg
Date:
On Mon, Jun 23, 2008 at 07:30:53PM -0400, Bruce Momjian wrote: > daveg wrote: > > On Mon, Jun 23, 2008 at 06:51:28PM -0400, Bruce Momjian wrote: > > > Alex Hunsaker wrote: > > > > On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera > > > > <alvherre@commandprompt.com> wrote: > > > > > Joshua D. Drake escribi?: > > > > > > > > > > > That is an interesting idea. Something like: > > > > > > > > > > > > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ? > > > > > > > > > > We already have it -- it's called PGOPTIONS. > > > > > > > > > > > > > Ok but is not the purpose of the patch to turn off statement_timeout > > > > by *default* in pg_restore/pg_dump? > > > > > > > > Here is an updated patch for I posted above (with the command line > > > > option --use-statement-timeout) for pg_dump and pg_restore. > > > > > > I would like to get do this without adding a new --use-statement-timeout > > > flag. Is anyone going to want to honor statement_timeout during > > > pg_dump/pg_restore? I thought we were just going to disable it. > > > > I have a patch in the queue to use set statement timeout while pg_dump is > > taking locks to avoid pg_dump hanging for other long running transactions > > that may have done ddl. Do I need to repost for discussion now? > > I see it now, but I forgot how it would interact with this patch. We > would have to prevent --use-statement-timeout when lock timeout was > being used, but my point is that I see no value in having > --use-statement-timeout. lock-timeout sets statement_timeout to a small value while locks are being taken on all the tables. Then it resets it to default. So it could reset it to whatever the new default is. Do I need to adjust my patch or something? -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Tom Lane
Date:
daveg <daveg@sonic.net> writes: > lock-timeout sets statement_timeout to a small value while locks are being > taken on all the tables. Then it resets it to default. So it could reset it > to whatever the new default is. "reset to default" is *surely* not the right behavior; resetting to the setting that had been in effect might be a bit sane. But the whole design sounds pretty broken to start with. The timer management code already understands the concept of scheduling the interrupt for the nearest of a couple of potentially active timeouts. ISTM any patch intended to add a feature like this ought to extend that logic rather than hack around changing the values of global variables. regards, tom lane
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
daveg
Date:
On Tue, Jun 24, 2008 at 05:34:50PM -0400, Tom Lane wrote: > daveg <daveg@sonic.net> writes: > > lock-timeout sets statement_timeout to a small value while locks are being > > taken on all the tables. Then it resets it to default. So it could reset it > > to whatever the new default is. > > "reset to default" is *surely* not the right behavior; resetting to the > setting that had been in effect might be a bit sane. But the whole > design sounds pretty broken to start with. The timer management code > already understands the concept of scheduling the interrupt for the > nearest of a couple of potentially active timeouts. ISTM any patch > intended to add a feature like this ought to extend that logic rather > than hack around changing the values of global variables. Are we talking about the same patch? Because I don't know what you are refering to with "timer management code" and "scheduling the interrupt" in the context of pg_dump. -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
Tom Lane
Date:
daveg <daveg@sonic.net> writes: > Are we talking about the same patch? Maybe not --- I thought you were talking about a backend-side behavioral change. > Because I don't know what you are > refering to with "timer management code" and "scheduling the interrupt" in > the context of pg_dump. I'm not sure that I see a good argument for making pg_dump deliberately fail, if that's what you're proposing. Maybe I'm just too old-school, but there simply is not any other higher priority for a database than safeguarding your data. regards, tom lane
Re: [PATCHES] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
From
daveg
Date:
On Tue, Jun 24, 2008 at 10:41:07PM -0400, Tom Lane wrote: > daveg <daveg@sonic.net> writes: > > Are we talking about the same patch? > > Maybe not --- I thought you were talking about a backend-side behavioral > change. > > > Because I don't know what you are > > refering to with "timer management code" and "scheduling the interrupt" in > > the context of pg_dump. > > I'm not sure that I see a good argument for making pg_dump deliberately > fail, if that's what you're proposing. Maybe I'm just too old-school, > but there simply is not any other higher priority for a database than > safeguarding your data. We agree about that. The intent of my patch it to give the user a chance to take corrective action in a case where pg_dump cannot be relied on to succeed. The problem is that pg_dump can get blocked behind locks and then fail hours later when the locks are released because some table it had not locked yet changed. In the worst case: - no backup, - no notice until too late to restart the backup, - lost production due to other processes waiting on locks pg_dump holds. So the intent of the patch is to optionally allow pg_dump to fail quickly if it cannot get all the access share locks it needs. This gives the user an opportunity to notice and retry in a timely way. Please see http://archives.postgresql.org/pgsql-patches/2008-05/msg00351.php for the orginal patch and problem description. A sample failure instance from a very heavy batch environment with a lot of materialized views being maintained concurrently with pg_dump. DB size is about 300 GB: --- 20080410 14:53:49 dumpdb c04_20080410_public: dumping c04 to /backups/c04_20080410_public pg_dump: SQL command failed pg_dump: Error message from server: ERROR: cache lookup failed for index 22619852 pg_dump: The command was: SELECT t.tableoid, t.oid, t.relname as indexname, pg_catalog.pg_get_indexdef(i.indexrelid) as indexdef,t.relnatts as indnkeys, i.indkey, i.indisclustered, c.contype, c.conname, c.tableoid as contableoid, c.oid as conoid,(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) as tablespace, array_to_string(t.reloptions,', ') as options FROM pg_catalog.pg_index i JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid)LEFT JOIN pg_catalog.pg_depend d ON (d.classid = t.tableoid AND d.objid = t.oid AND d.deptype = 'i') LEFT JOINpg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) WHERE i.indrelid = '22615005'::pg_catalog.oidORDER BY indexname 20080411 06:12:17 dumpdb FATAL: c04_20080410_public: dump failed --- Note that the dump started at 14:53, but did not fail until 06:12 the next day, and it never got to the actual copy out phase. Meanwhile other DDL using processes were hung on the access share locks aready held by pg_dump. Regards -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.