From 8d4f2ecd41312e57422901952cbad234d293060b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 23 Apr 2009 00:23:46 +0000 Subject: [PATCH] Change the default value of max_prepared_transactions to zero, and add documentation warnings against setting it nonzero unless active use of prepared transactions is intended and a suitable transaction manager has been installed. This should help to prevent the type of scenario we've seen several times now where a prepared transaction is forgotten and eventually causes severe maintenance problems (or even anti-wraparound shutdown). The only real reason we had the default be nonzero in the first place was to support regression testing of the feature. To still be able to do that, tweak pg_regress to force a nonzero value during "make check". Since we cannot force a nonzero value in "make installcheck", add a variant regression test "expected" file that shows the results that will be obtained when max_prepared_transactions is zero. Also, extend the HINT messages for transaction wraparound warnings to mention the possibility that old prepared transactions are causing the problem. All per today's discussion. --- doc/src/sgml/config.sgml | 18 +- doc/src/sgml/ref/prepare_transaction.sgml | 48 ++-- src/backend/access/transam/twophase.c | 11 +- src/backend/access/transam/varsup.c | 11 +- src/backend/utils/misc/guc.c | 4 +- src/backend/utils/misc/postgresql.conf.sample | 4 +- src/test/regress/expected/prepared_xacts.out | 2 + .../regress/expected/prepared_xacts_1.out | 223 ++++++++++++++++++ src/test/regress/pg_regress.c | 34 ++- src/test/regress/sql/prepared_xacts.sql | 1 + 10 files changed, 304 insertions(+), 52 deletions(-) create mode 100644 src/test/regress/expected/prepared_xacts_1.out diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fcff12ae26..6102c70f47 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1,4 +1,4 @@ - + Server Configuration @@ -785,18 +785,18 @@ SET ENABLE_SEQSCAN TO OFF; prepared state simultaneously (see ). - Setting this parameter to zero disables the prepared-transaction - feature. - The default is five transactions. + Setting this parameter to zero (which is the default) + disables the prepared-transaction feature. This parameter can only be set at server start. - If you are not using prepared transactions, this parameter may as - well be set to zero. If you are using them, you will probably - want max_prepared_transactions to be at least - as large as , to avoid unwanted - failures at the prepare step. + If you are not planning to use prepared transactions, this parameter + should be set to zero to prevent accidental creation of prepared + transactions. If you are using prepared transactions, you will + probably want max_prepared_transactions to be at + least as large as , so that every + session can have a prepared transaction pending. diff --git a/doc/src/sgml/ref/prepare_transaction.sgml b/doc/src/sgml/ref/prepare_transaction.sgml index 82973c4751..b8b8d8aca8 100644 --- a/doc/src/sgml/ref/prepare_transaction.sgml +++ b/doc/src/sgml/ref/prepare_transaction.sgml @@ -1,5 +1,5 @@ @@ -30,7 +30,7 @@ PREPARE TRANSACTION transaction_id PREPARE TRANSACTION prepares the current transaction - for two-phase commit. After this command, the transaction is no longer + for two-phase commit. After this command, the transaction is no longer associated with the current session; instead, its state is fully stored on disk, and there is a very high probability that it can be committed successfully, even if a database crash occurs before the commit is @@ -100,7 +100,7 @@ PREPARE TRANSACTION transaction_id If the transaction modified any run-time parameters with SET (without the LOCAL option), those effects persist after PREPARE TRANSACTION, and will not - be affected by any later COMMIT PREPARED or + be affected by any later COMMIT PREPARED or ROLLBACK PREPARED. Thus, in this one respect PREPARE TRANSACTION acts more like COMMIT than ROLLBACK. @@ -112,26 +112,28 @@ PREPARE TRANSACTION transaction_id system view. - - From a performance standpoint, it is unwise to leave transactions in - the prepared state for a long time: this will for instance interfere with - the ability of VACUUM to reclaim storage. Keep in mind also - that the transaction continues to hold whatever locks it held. - The intended - usage of the feature is that a prepared transaction will normally be - committed or rolled back as soon as an external transaction manager - has verified that other databases are also prepared to commit. - + + + It is unwise to leave transactions in the prepared state for a long time. + This will interfere with the ability of VACUUM to reclaim + storage, and in extreme cases could cause the database to shut down + to prevent transaction ID wraparound (see ). Keep in mind also that the transaction + continues to hold whatever locks it held. The intended usage of the + feature is that a prepared transaction will normally be committed or + rolled back as soon as an external transaction manager has verified that + other databases are also prepared to commit. + - - If you make any serious use of prepared transactions, you will probably - want to increase the value of , as the default setting is - quite small (to avoid wasting resources for those who don't use it). - It is recommendable to make it at least equal to - , so that every session can have - a prepared transaction pending. - + + If you have not set up an external transaction manager to track prepared + transactions and ensure they get closed out promptly, it is best to keep + the prepared-transaction feature disabled by setting + to zero. This will + prevent accidental creation of prepared transactions that might then + be forgotten and eventually cause problems. + + @@ -139,7 +141,7 @@ PREPARE TRANSACTION transaction_id Prepare the current transaction for two-phase commit, using foobar as the transaction identifier: - + PREPARE TRANSACTION 'foobar'; diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index b738a4cef4..4685ccdf10 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.51 2009/01/01 17:23:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.52 2009/04/23 00:23:45 tgl Exp $ * * NOTES * Each global transaction is associated with a global transaction @@ -68,7 +68,7 @@ #define TWOPHASE_DIR "pg_twophase" /* GUC variable, can't be changed after startup */ -int max_prepared_xacts = 5; +int max_prepared_xacts = 0; /* * This struct describes one global transaction that is in prepared state @@ -228,6 +228,13 @@ MarkAsPreparing(TransactionId xid, const char *gid, errmsg("transaction identifier \"%s\" is too long", gid))); + /* fail immediately if feature is disabled */ + if (max_prepared_xacts == 0) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("prepared transactions are disabled"), + errhint("Set max_prepared_transactions to a nonzero value."))); + LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); /* diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index 5c0e627987..029b2f2deb 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -6,7 +6,7 @@ * Copyright (c) 2000-2009, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/varsup.c,v 1.83 2009/01/01 17:23:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/varsup.c,v 1.84 2009/04/23 00:23:45 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -86,14 +86,16 @@ GetNewTransactionId(bool isSubXact) (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("database is not accepting commands to avoid wraparound data loss in database \"%s\"", NameStr(ShmemVariableCache->limit_datname)), - errhint("Stop the postmaster and use a standalone backend to vacuum database \"%s\".", + errhint("Stop the postmaster and use a standalone backend to vacuum database \"%s\".\n" + "You might also need to commit or roll back old prepared transactions.", NameStr(ShmemVariableCache->limit_datname)))); else if (TransactionIdFollowsOrEquals(xid, ShmemVariableCache->xidWarnLimit)) ereport(WARNING, (errmsg("database \"%s\" must be vacuumed within %u transactions", NameStr(ShmemVariableCache->limit_datname), ShmemVariableCache->xidWrapLimit - xid), - errhint("To avoid a database shutdown, execute a database-wide VACUUM in \"%s\".", + errhint("To avoid a database shutdown, execute a database-wide VACUUM in \"%s\".\n" + "You might also need to commit or roll back old prepared transactions.", NameStr(ShmemVariableCache->limit_datname)))); } @@ -299,7 +301,8 @@ SetTransactionIdLimit(TransactionId oldest_datfrozenxid, (errmsg("database \"%s\" must be vacuumed within %u transactions", NameStr(*oldest_datname), xidWrapLimit - curXid), - errhint("To avoid a database shutdown, execute a database-wide VACUUM in \"%s\".", + errhint("To avoid a database shutdown, execute a database-wide VACUUM in \"%s\".\n" + "You might also need to commit or roll back old prepared transactions.", NameStr(*oldest_datname)))); } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 7313dc0e58..23fb43647d 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.502 2009/04/07 23:27:34 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.503 2009/04/23 00:23:45 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -1504,7 +1504,7 @@ static struct config_int ConfigureNamesInt[] = NULL }, &max_prepared_xacts, - 5, 0, INT_MAX, NULL, NULL + 0, 0, INT_MAX / 4, NULL, NULL }, #ifdef LOCK_DEBUG diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 1e9379ac2f..3f7b43f0cc 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -106,10 +106,12 @@ #shared_buffers = 32MB # min 128kB # (change requires restart) #temp_buffers = 8MB # min 800kB -#max_prepared_transactions = 5 # can be 0 or more +#max_prepared_transactions = 0 # zero disables the feature # (change requires restart) # Note: Increasing max_prepared_transactions costs ~600 bytes of shared memory # per transaction slot, plus lock space (see max_locks_per_transaction). +# It is not advisable to set max_prepared_transactions nonzero unless you +# actively intend to use prepared transactions. #work_mem = 1MB # min 64kB #maintenance_work_mem = 16MB # min 1MB #max_stack_depth = 2MB # min 100kB diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out index 535dbe4cbf..292962ab7b 100644 --- a/src/test/regress/expected/prepared_xacts.out +++ b/src/test/regress/expected/prepared_xacts.out @@ -214,4 +214,6 @@ SELECT gid FROM pg_prepared_xacts; -- Clean up DROP TABLE pxtest2; +DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled +ERROR: table "pxtest3" does not exist DROP TABLE pxtest4; diff --git a/src/test/regress/expected/prepared_xacts_1.out b/src/test/regress/expected/prepared_xacts_1.out new file mode 100644 index 0000000000..991a11bdfa --- /dev/null +++ b/src/test/regress/expected/prepared_xacts_1.out @@ -0,0 +1,223 @@ +-- +-- PREPARED TRANSACTIONS (two-phase commit) +-- +-- We can't readily test persistence of prepared xacts within the +-- regression script framework, unfortunately. Note that a crash +-- isn't really needed ... stopping and starting the postmaster would +-- be enough, but we can't even do that here. +-- create a simple table that we'll use in the tests +CREATE TABLE pxtest1 (foobar VARCHAR(10)); +INSERT INTO pxtest1 VALUES ('aaa'); +-- Test PREPARE TRANSACTION +BEGIN; +UPDATE pxtest1 SET foobar = 'bbb' WHERE foobar = 'aaa'; +SELECT * FROM pxtest1; + foobar +-------- + bbb +(1 row) + +PREPARE TRANSACTION 'foo1'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +-- Test pg_prepared_xacts system view +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- Test ROLLBACK PREPARED +ROLLBACK PREPARED 'foo1'; +ERROR: prepared transaction with identifier "foo1" does not exist +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- Test COMMIT PREPARED +BEGIN; +INSERT INTO pxtest1 VALUES ('ddd'); +SELECT * FROM pxtest1; + foobar +-------- + aaa + ddd +(2 rows) + +PREPARE TRANSACTION 'foo2'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +COMMIT PREPARED 'foo2'; +ERROR: prepared transaction with identifier "foo2" does not exist +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +-- Test duplicate gids +BEGIN; +UPDATE pxtest1 SET foobar = 'eee' WHERE foobar = 'ddd'; +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +PREPARE TRANSACTION 'foo3'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +BEGIN; +INSERT INTO pxtest1 VALUES ('fff'); +SELECT * FROM pxtest1; + foobar +-------- + aaa + fff +(2 rows) + +-- This should fail, because the gid foo3 is already in use +PREPARE TRANSACTION 'foo3'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +ROLLBACK PREPARED 'foo3'; +ERROR: prepared transaction with identifier "foo3" does not exist +SELECT * FROM pxtest1; + foobar +-------- + aaa +(1 row) + +-- Clean up +DROP TABLE pxtest1; +-- Test subtransactions +BEGIN; + CREATE TABLE pxtest2 (a int); + INSERT INTO pxtest2 VALUES (1); + SAVEPOINT a; + INSERT INTO pxtest2 VALUES (2); + ROLLBACK TO a; + SAVEPOINT b; + INSERT INTO pxtest2 VALUES (3); +PREPARE TRANSACTION 'regress-one'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +CREATE TABLE pxtest3(fff int); +-- Test shared invalidation +BEGIN; + DROP TABLE pxtest3; + CREATE TABLE pxtest4 (a int); + INSERT INTO pxtest4 VALUES (1); + INSERT INTO pxtest4 VALUES (2); + DECLARE foo CURSOR FOR SELECT * FROM pxtest4; + -- Fetch 1 tuple, keeping the cursor open + FETCH 1 FROM foo; + a +--- + 1 +(1 row) + +PREPARE TRANSACTION 'regress-two'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +-- No such cursor +FETCH 1 FROM foo; +ERROR: cursor "foo" does not exist +-- Table doesn't exist, the creation hasn't been committed yet +SELECT * FROM pxtest2; +ERROR: relation "pxtest2" does not exist +LINE 1: SELECT * FROM pxtest2; + ^ +-- There should be two prepared transactions +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- pxtest3 should be locked because of the pending DROP +set statement_timeout to 2000; +SELECT * FROM pxtest3; + fff +----- +(0 rows) + +reset statement_timeout; +-- Disconnect, we will continue testing in a different backend +\c - +-- There should still be two prepared transactions +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- pxtest3 should still be locked because of the pending DROP +set statement_timeout to 2000; +SELECT * FROM pxtest3; + fff +----- +(0 rows) + +reset statement_timeout; +-- Commit table creation +COMMIT PREPARED 'regress-one'; +ERROR: prepared transaction with identifier "regress-one" does not exist +\d pxtest2 +SELECT * FROM pxtest2; +ERROR: relation "pxtest2" does not exist +LINE 1: SELECT * FROM pxtest2; + ^ +-- There should be one prepared transaction +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- Commit table drop +COMMIT PREPARED 'regress-two'; +ERROR: prepared transaction with identifier "regress-two" does not exist +SELECT * FROM pxtest3; + fff +----- +(0 rows) + +-- There should be no prepared transactions +SELECT gid FROM pg_prepared_xacts; + gid +----- +(0 rows) + +-- Clean up +DROP TABLE pxtest2; +ERROR: table "pxtest2" does not exist +DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled +DROP TABLE pxtest4; +ERROR: table "pxtest4" does not exist diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 7bd297f20c..b51d6ec2b5 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/test/regress/pg_regress.c,v 1.61 2009/02/12 13:26:03 petere Exp $ + * $PostgreSQL: pgsql/src/test/regress/pg_regress.c,v 1.62 2009/04/23 00:23:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2037,6 +2037,8 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc if (temp_install) { + FILE *pg_conf; + /* * Prepare the temp installation */ @@ -2092,20 +2094,29 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc exit_nicely(2); } - /* add any extra config specified to the postgresql.conf */ + /* + * Adjust the default postgresql.conf as needed for regression testing. + * The user can specify a file to be appended; in any case we set + * max_prepared_transactions to enable testing of prepared xacts. + * (Note: to reduce the probability of unexpected shmmax failures, + * don't set max_prepared_transactions any higher than actually + * needed by the prepared_xacts regression test.) + */ + snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_install); + pg_conf = fopen(buf, "a"); + if (pg_conf == NULL) + { + fprintf(stderr, _("\n%s: could not open \"%s\" for adding extra config: %s\n"), progname, buf, strerror(errno)); + exit_nicely(2); + } + fputs("\n# Configuration added by pg_regress\n\n", pg_conf); + fputs("max_prepared_transactions = 2\n", pg_conf); + if (temp_config != NULL) { FILE *extra_conf; - FILE *pg_conf; char line_buf[1024]; - snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_install); - pg_conf = fopen(buf, "a"); - if (pg_conf == NULL) - { - fprintf(stderr, _("\n%s: could not open \"%s\" for adding extra config: %s\n"), progname, buf, strerror(errno)); - exit_nicely(2); - } extra_conf = fopen(temp_config, "r"); if (extra_conf == NULL) { @@ -2115,9 +2126,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc while (fgets(line_buf, sizeof(line_buf), extra_conf) != NULL) fputs(line_buf, pg_conf); fclose(extra_conf); - fclose(pg_conf); } + fclose(pg_conf); + /* * Check if there is a postmaster running already. */ diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql index 337567271d..39d323a15b 100644 --- a/src/test/regress/sql/prepared_xacts.sql +++ b/src/test/regress/sql/prepared_xacts.sql @@ -134,4 +134,5 @@ SELECT gid FROM pg_prepared_xacts; -- Clean up DROP TABLE pxtest2; +DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled DROP TABLE pxtest4;