Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Tue, Jun 24, 2008 at 10:41:07PM -0400, Tom Lane wrote: > daveg <[EMAIL PROTECTED]> 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 JOIN pg_catalog.pg_constraint c ON (d.refclassid = c.tableoid AND d.refobjid = c.oid) WHERE i.indrelid = '22615005'::pg_catalog.oid ORDER 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 [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
daveg <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Tue, Jun 24, 2008 at 05:34:50PM -0400, Tom Lane wrote: > daveg <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
daveg <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
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 > > > > <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Joshua D. Drake wrote: > Alex Hunsaker wrote: > > On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Alex Hunsaker wrote: On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <[EMAIL PROTECTED]> 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Alex Hunsaker wrote: > On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Mon, Jun 23, 2008 at 4:51 PM, Bruce Momjian <[EMAIL PROTECTED]> 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. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
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 > > > <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
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 > > <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Alex Hunsaker wrote: > On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera > <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera <[EMAIL PROTECTED]> 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 :) ) pg_dump_restore_statement_timeout.diff Description: Binary data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
