Re: [Fwd: Re: [HACKERS] Transactions and temp tables]
Bruce Momjian wrote: Heikki Linnakangas wrote: IMHO, this is just getting too kludgey. We came up with pretty good ideas on how to handle temp tables properly, by treating the same as non-temp tables. That should eliminate all the problems the latest patch did, and also the issues with sequences, and allow all access to temp tables, not just a limited subset. I don't think it's worthwhile to apply the kludge as a stopgap measure, let's do it properly in 8.5. ... Can someone tell me how this should be worded as a TODO item? There already is a todo item about this: Allow prepared transactions with temporary tables created and dropped in the same transaction, and when an ON COMMIT DELETE ROWS temporary table is accessed I added a link to the email describing the most recent idea on how this should be implemented. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [Fwd: Re: [HACKERS] Transactions and temp tables]
Heikki Linnakangas wrote: Emmanuel Cecchet wrote: I just saw that this new patch was not considered because the previous version ended being rejected. Note that this version of the patch aims at supporting ONLY temp tables that are created AND dropped in the same transaction. We need to be able to use temp tables in transactions that are doing 2PC, but the temp table lifespan does not need to cross transaction boundaries. Please let me know if this patch could be integrated in 8.4. IMHO, this is just getting too kludgey. We came up with pretty good ideas on how to handle temp tables properly, by treating the same as non-temp tables. That should eliminate all the problems the latest patch did, and also the issues with sequences, and allow all access to temp tables, not just a limited subset. I don't think it's worthwhile to apply the kludge as a stopgap measure, let's do it properly in 8.5. As a workaround, you can use a regular table instead of a temporary one. If you create and drop the regular table in the same transaction (that's the same limitation that latest patch has), you won't end up with a bogus table in your database if the connection is dropped unexpectedly. If your application uses multiple connections simultaenously, you'll need a little bit of code in the application so that you don't try to create a table with the same name in all backends. You could also create a different schema for each connection, and do set search_path='semitempschemaX, public', so that you can use the same table name and still have separate tables for each connections. Can someone tell me how this should be worded as a TODO item? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [Fwd: Re: [HACKERS] Transactions and temp tables]
Hi Heikki, The point of using temp tables was performance. Using regular tables in our case would hurt performance too much. Well if we cannot get a temporary fix in 8.4, we will maintain a separate patch to get that functionality just for temp tables that are created and dropped in the same transaction. Hopefully we will be able to come up with a working solution in 8.5. Thanks for your help, Emmanuel Emmanuel Cecchet wrote: I just saw that this new patch was not considered because the previous version ended being rejected. Note that this version of the patch aims at supporting ONLY temp tables that are created AND dropped in the same transaction. We need to be able to use temp tables in transactions that are doing 2PC, but the temp table lifespan does not need to cross transaction boundaries. Please let me know if this patch could be integrated in 8.4. IMHO, this is just getting too kludgey. We came up with pretty good ideas on how to handle temp tables properly, by treating the same as non-temp tables. That should eliminate all the problems the latest patch did, and also the issues with sequences, and allow all access to temp tables, not just a limited subset. I don't think it's worthwhile to apply the kludge as a stopgap measure, let's do it properly in 8.5. As a workaround, you can use a regular table instead of a temporary one. If you create and drop the regular table in the same transaction (that's the same limitation that latest patch has), you won't end up with a bogus table in your database if the connection is dropped unexpectedly. If your application uses multiple connections simultaenously, you'll need a little bit of code in the application so that you don't try to create a table with the same name in all backends. You could also create a different schema for each connection, and do set search_path='semitempschemaX, public', so that you can use the same table name and still have separate tables for each connections. (sorry for the late reply) -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Emmanuel Cecchet Aster Data Systems Web: http://www.asterdata.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [Fwd: Re: [HACKERS] Transactions and temp tables]
Emmanuel Cecchet wrote: I just saw that this new patch was not considered because the previous version ended being rejected. Note that this version of the patch aims at supporting ONLY temp tables that are created AND dropped in the same transaction. We need to be able to use temp tables in transactions that are doing 2PC, but the temp table lifespan does not need to cross transaction boundaries. Please let me know if this patch could be integrated in 8.4. IMHO, this is just getting too kludgey. We came up with pretty good ideas on how to handle temp tables properly, by treating the same as non-temp tables. That should eliminate all the problems the latest patch did, and also the issues with sequences, and allow all access to temp tables, not just a limited subset. I don't think it's worthwhile to apply the kludge as a stopgap measure, let's do it properly in 8.5. As a workaround, you can use a regular table instead of a temporary one. If you create and drop the regular table in the same transaction (that's the same limitation that latest patch has), you won't end up with a bogus table in your database if the connection is dropped unexpectedly. If your application uses multiple connections simultaenously, you'll need a little bit of code in the application so that you don't try to create a table with the same name in all backends. You could also create a different schema for each connection, and do set search_path='semitempschemaX, public', so that you can use the same table name and still have separate tables for each connections. (sorry for the late reply) -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: I think that the Assert in is_temp_rel(Oid) in tablecmds.c should be replaced by if (on_commits == NULL) return false; As the use case below shows, a regular table can be created and hold a LOCKTAG_RELATION lock that will trigger the call to is_temp_rel in is_preparable_locktag. The assert will break if no temp table was accessed. Yes, you're right. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: There is a problem with temp tables with on delete rows that are created inside a transaction. Take the 2pc_on_delete_rows_transaction.sql test case and change the creation statement, instead of create temp table foo(x int) on commit delete rows; try create temp table foo(x serial primary key) on commit delete rows; The test will fail. It looks like the onCommit field is not properly updated when serial or primary key is used in that context. I did not figure out why. A serial column uses a sequence behind the scenes. Hmm. Seems like we would need to treat sequences and indexes the same as tables with ON COMMIT DELETE ROWS, i.e release the locks early and don't error out. All in all, this is getting pretty messy. My patch felt a bit hackish to begin with, and having to add special cases for sequences and indexes would make it even more so. And what about temporary views? I'm starting to feel that instead of special-casing temp relations, we need to move into the opposite direction and make temp relations more like regular relations. Unfortunately, that's not going to happen in the 8.4 timeframe :-(. Let's try the other approach in 8.5. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
I would really like to have support for temp tables at least for the case where the table is created and dropped in the same transaction. But I guess that the other limitations on index, sequences and views would still hold, right? manu Heikki Linnakangas wrote: Emmanuel Cecchet wrote: There is a problem with temp tables with on delete rows that are created inside a transaction. Take the 2pc_on_delete_rows_transaction.sql test case and change the creation statement, instead of create temp table foo(x int) on commit delete rows; try create temp table foo(x serial primary key) on commit delete rows; The test will fail. It looks like the onCommit field is not properly updated when serial or primary key is used in that context. I did not figure out why. A serial column uses a sequence behind the scenes. Hmm. Seems like we would need to treat sequences and indexes the same as tables with ON COMMIT DELETE ROWS, i.e release the locks early and don't error out. All in all, this is getting pretty messy. My patch felt a bit hackish to begin with, and having to add special cases for sequences and indexes would make it even more so. And what about temporary views? I'm starting to feel that instead of special-casing temp relations, we need to move into the opposite direction and make temp relations more like regular relations. Unfortunately, that's not going to happen in the 8.4 timeframe :-(. Let's try the other approach in 8.5. -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Heikki, I think that the Assert in is_temp_rel(Oid) in tablecmds.c should be replaced by if (on_commits == NULL) return false; As the use case below shows, a regular table can be created and hold a LOCKTAG_RELATION lock that will trigger the call to is_temp_rel in is_preparable_locktag. The assert will break if no temp table was accessed. As we were also trying to list potential issues, if the temp table uses a SERIAL type, will there be potentially a problem with the sequence at prepare time? Emmanuel The following test fails with your patch on my system. Could you check if you can reproduce? psql (8.4devel) Type help for help. test=# begin; BEGIN test=# create table paul(x int); CREATE TABLE test=# insert into paul values(1); INSERT 0 1 test=# prepare transaction 'persistentTableShouldSucceed'; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. --- LOG: database system is ready to accept connections TRAP: FailedAssertion(!(on_commits != ((void *)0)), File: tablecmds.c, Line: 7823) LOG: server process (PID 15969) was terminated by signal 6: Aborted LOG: terminating any other active server processes FATAL: the database system is in recovery mode Thanks, manu -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Heikki, The following test fails with your patch on my system. Could you check if you can reproduce? psql (8.4devel) Type help for help. test=# begin; BEGIN test=# create table paul(x int); CREATE TABLE test=# insert into paul values(1); INSERT 0 1 test=# prepare transaction 'persistentTableShouldSucceed'; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. --- LOG: database system is ready to accept connections TRAP: FailedAssertion(!(on_commits != ((void *)0)), File: tablecmds.c, Line: 7823) LOG: server process (PID 15969) was terminated by signal 6: Aborted LOG: terminating any other active server processes FATAL: the database system is in recovery mode Thanks, manu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Heikki Linnakangas wrote: Emmanuel Cecchet wrote: I still quite did not get what the big deal was if an ON COMMIT DELETE ROWS temp table was created inside a transaction. In case the transaction that created a temp table rolls back, the table needs to be removed. Removing a temporary table belonging to another backend is problematic; the local buffers in the original backend need to be dropped, as well as the entry in the on commit actions list. Why the new checks you are doing in lock.c would not work with dropped temp tables? Could it be possible to drop the lock as soon as the temp table is dropped inside a transaction? If you release the lock early on a table that you drop, another transactions would be free to access the table, even though it's about to be dropped. I will try to find more time to review the patch tonight. Thanks! Thinking about this whole thing yet more, I wonder if we could have a more holistic approach and make temporary tables work just like regular ones. The problems we've identified this far are: 1. If the prepared transaction keeps the temp table locked, the backend can't exit, because the shutdown hook tries to drop all temp tables. 2. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to drop all the local buffers from the original backend's private buffer cache. 3. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to remove the on-commit entry from the original backend's private list. Is there more? I think we can solve all the above problems: 1. Modify RemoveTempRelations so that it doesn't block if it can't immediately acquire lock on the to-be-removed object. That way the original backend can exit even if a prepared transaction is holding a lock on a temporary object. To avoid conflict with a new backend that's assigned the same backendid, divorce the temporary namespace naming from backendid so that the temporary namespace name stays reserved for the prepared transaction. Is that going to cause any problem with DROP CASCADE operations or trying to later drop a child table if the parent table is locked? I did hit that issue when I tried to modify RemoveTempRelations but I was probably not very smart at it. 2. Flush and drop all local buffers on temporary tables that have been created or dropped in the transaction at PREPARE TRANSACTION already. Would there be any issue if someone was trying to use a READ_UNCOMMITTED isolation level to access the temp table data? 3. Add on-commit field to pg_class, and only keep a list of temporary tables that have been accessed in the current transaction in backend-private memory. Yes, this seems doable. We will certainly have to keep a list per transaction id in case multiple prepared but uncommitted transactions have accessed different temp table on that backend. Have you already started to code some of this? I am looking at adding new tests to check all cases including all types of temp tables (normal/on delete/on drop, created inside or outside a transaction, prepare/postmaster stop/commit/rollback, inherit/index/vaccuum). That should get all the use cases covered. Let me know what you think. Thanks, Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: I still quite did not get what the big deal was if an ON COMMIT DELETE ROWS temp table was created inside a transaction. In case the transaction that created a temp table rolls back, the table needs to be removed. Removing a temporary table belonging to another backend is problematic; the local buffers in the original backend need to be dropped, as well as the entry in the on commit actions list. Why the new checks you are doing in lock.c would not work with dropped temp tables? Could it be possible to drop the lock as soon as the temp table is dropped inside a transaction? If you release the lock early on a table that you drop, another transactions would be free to access the table, even though it's about to be dropped. I will try to find more time to review the patch tonight. Thanks! Thinking about this whole thing yet more, I wonder if we could have a more holistic approach and make temporary tables work just like regular ones. The problems we've identified this far are: 1. If the prepared transaction keeps the temp table locked, the backend can't exit, because the shutdown hook tries to drop all temp tables. 2. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to drop all the local buffers from the original backend's private buffer cache. 3. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to remove the on-commit entry from the original backend's private list. Is there more? I think we can solve all the above problems: 1. Modify RemoveTempRelations so that it doesn't block if it can't immediately acquire lock on the to-be-removed object. That way the original backend can exit even if a prepared transaction is holding a lock on a temporary object. To avoid conflict with a new backend that's assigned the same backendid, divorce the temporary namespace naming from backendid so that the temporary namespace name stays reserved for the prepared transaction. 2. Flush and drop all local buffers on temporary tables that have been created or dropped in the transaction at PREPARE TRANSACTION already. 3. Add on-commit field to pg_class, and only keep a list of temporary tables that have been accessed in the current transaction in backend-private memory. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi Heikki, I will try to make more tests. I still quite did not get what the big deal was if an ON COMMIT DELETE ROWS temp table was created inside a transaction. Why the new checks you are doing in lock.c would not work with dropped temp tables? Could it be possible to drop the lock as soon as the temp table is dropped inside a transaction? I will try to find more time to review the patch tonight. Best, Emmanuel Heikki Linnakangas wrote: Somehow this feels pretty baroque, though. Perhaps a better approach would be to add a new AtPrepare_OnCommitActions function to tablecmds.c, that gets called before AtPrepare_Locks. It would scan through the on_commits list, and release all locks for the PREPARE-safe temp tables, and throw the error if necessary. I'll try that next. Here's what I ended up with. I morphed the on commit action registration into tracking of all temporary relations. This only allows access to ON COMMIT DELETE ROWS temp tables. Accessing other temporary tables, and creating or dropping tables in the transaction is still forbidden. It took me a couple of iterations to handle toast tables and indexes correctly. More testing would be appreciated with more complex cases like VACUUM FULL, subtransactions etc. -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Emmanuel Cecchet wrote: As I have not found yet an elegant solution to deal with the DROP CASCADE issue, here is a simpler patch that handles temp tables that are dropped at commit time. This is a good first step and we will try to elaborate further to support ON COMMIT DELETE ROWS. The problem with stopping the postmaster seems to still be there.. All the problems are centered around locking. We need to address that and decide what is sane locking behavior wrt. temp tables and 2PC. First, there's the case where a temp table is created and dropped in the same transaction. It seems perfectly sane to me to simply drop all locks on the dropped table at PREPARE TRANSACTION. Does anyone see a problem with that? If not, we might as well do that for all tables, not just temporary ones. It seems just as safe for non-temporary tables. This seems good to me. Any access to the table after PREPARE TRANSACTION but before COMMIT on that backend would return a relation not found which is what we expect. For a regular table, I don't know if that makes a difference if the prepared transaction rollbacks? I don't think there's any difference with temp tables and regular ones from locking point of view. Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. There too, could we simply drop the locks at PREPARE TRANSACTION? I can't immediately see anything wrong with that. As there is no data anyway, I don't think the locks are going to change anything. But in the most recent stripped-down version of the patch, on delete rows is no more supported, I only allow on commit drop. I did not find the flag to see if a temp table was created with the on delete rows option. Hmm. I think we can use the on_commits list in tablecmds.c for that. Do you want me to look at the locking code or will you have time to do it? Hints will be welcome if you want me to do it. I can give it a shot for change. Attached is a patch that allows the ON COMMIT DELETE ROWS case. The beef of the patch is: - An entry is made into the on_commits list in tablecmds.c for all temp tables, even if there's no ON COMMIT action - There's a new function, check_prepare_safe_temp_table(Oid relid) in tablecmds.c, that uses the on_commits list to determine if the access to the given relation is PREPARE-safe. That is, it's not a temp table, or it's an access to an ON COMMIT DELETE ROWS temp table and the temp table wasn't created or dropped in the same transaction. - MyXactMadeTempRelUpdate variable is gone. The check is driven from the lock manager again, like it was in 8.1, by calling the new check_prepare_sage_temp_table function for all relation locks in AtPrepare_Locks(). - changed the on_commits linked list in tablecmds.c into a hash table for performance Somehow this feels pretty baroque, though. Perhaps a better approach would be to add a new AtPrepare_OnCommitActions function to tablecmds.c, that gets called before AtPrepare_Locks. It would scan through the on_commits list, and release all locks for the PREPARE-safe temp tables, and throw the error if necessary. I'll try that next. BTW, there's a very relevant thread here: http://archives.postgresql.org/pgsql-hackers/2008-03/msg00063.php if you haven't read it already. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 33d5fab..b94e2bd 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -876,10 +876,6 @@ relation_open(Oid relationId, LOCKMODE lockmode) if (!RelationIsValid(r)) elog(ERROR, could not open relation with OID %u, relationId); - /* Make note that we've accessed a temporary relation */ - if (r-rd_istemp) - MyXactAccessedTempRel = true; - pgstat_initstats(r); return r; @@ -924,10 +920,6 @@ try_relation_open(Oid relationId, LOCKMODE lockmode) if (!RelationIsValid(r)) elog(ERROR, could not open relation with OID %u, relationId); - /* Make note that we've accessed a temporary relation */ - if (r-rd_istemp) - MyXactAccessedTempRel = true; - pgstat_initstats(r); return r; @@ -974,10 +966,6 @@ relation_open_nowait(Oid relationId, LOCKMODE lockmode) if (!RelationIsValid(r)) elog(ERROR, could not open relation with OID %u, relationId); - /* Make note that we've accessed a temporary relation */ - if (r-rd_istemp) - MyXactAccessedTempRel = true; - pgstat_initstats(r); return r; diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index b7d1285..c859874 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -67,14 +67,6 @@ int CommitDelay = 0; /* precommit delay in microseconds */ int CommitSiblings = 5; /* # concurrent xacts needed to sleep */ /* - * MyXactAccessedTempRel is set when a temporary relation is accessed. - * We don't allow PREPARE TRANSACTION in that
Re: [HACKERS] Transactions and temp tables
Heikki Linnakangas wrote: Somehow this feels pretty baroque, though. Perhaps a better approach would be to add a new AtPrepare_OnCommitActions function to tablecmds.c, that gets called before AtPrepare_Locks. It would scan through the on_commits list, and release all locks for the PREPARE-safe temp tables, and throw the error if necessary. I'll try that next. Here's what I ended up with. I morphed the on commit action registration into tracking of all temporary relations. This only allows access to ON COMMIT DELETE ROWS temp tables. Accessing other temporary tables, and creating or dropping tables in the transaction is still forbidden. It took me a couple of iterations to handle toast tables and indexes correctly. More testing would be appreciated with more complex cases like VACUUM FULL, subtransactions etc. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com *** src/backend/access/heap/heapam.c --- src/backend/access/heap/heapam.c *** *** 51,56 --- 51,57 #include access/xlogutils.h #include catalog/catalog.h #include catalog/namespace.h + #include commands/tablecmds.h #include miscadmin.h #include pgstat.h #include storage/bufmgr.h *** *** 878,884 relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 879,885 /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *** *** 926,932 try_relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 927,933 /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *** *** 976,982 relation_open_nowait(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 977,983 /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *** src/backend/access/transam/xact.c --- src/backend/access/transam/xact.c *** *** 67,80 int CommitDelay = 0; /* precommit delay in microseconds */ int CommitSiblings = 5; /* # concurrent xacts needed to sleep */ /* - * MyXactAccessedTempRel is set when a temporary relation is accessed. - * We don't allow PREPARE TRANSACTION in that case. (This is global - * so that it can be set from heapam.c.) - */ - bool MyXactAccessedTempRel = false; - - - /* * transaction states - transaction state from server perspective */ typedef enum TransState --- 67,72 *** *** 1467,1473 StartTransaction(void) XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters --- 1459,1464 *** *** 1798,1823 PrepareTransaction(void) /* NOTIFY and flatfiles will be handled below */ - /* - * Don't allow PREPARE TRANSACTION if we've accessed a temporary table - * in this transaction. Having the prepared xact hold locks on another - * backend's temp table seems a bad idea --- for instance it would prevent - * the backend from exiting. There are other problems too, such as how - * to clean up the source backend's local buffers and ON COMMIT state - * if the prepared xact includes a DROP of a temp table. - * - * We must check this after executing any ON COMMIT actions, because - * they might still access a temp relation. - * - * XXX In principle this could be relaxed to allow some useful special - * cases, such as a temp table created and dropped all within the - * transaction. That seems to require much more bookkeeping though. - */ - if (MyXactAccessedTempRel) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg(cannot PREPARE a transaction that has operated on temporary tables))); - /* Prevent cancel/die interrupt while cleaning up */ HOLD_INTERRUPTS(); --- 1789,1794 *** *** 1861,1866 PrepareTransaction(void) --- 1832,1838 AtPrepare_Notify(); AtPrepare_UpdateFlatFiles(); AtPrepare_Inval(); + AtPrepare_on_commit_actions(); AtPrepare_Locks(); AtPrepare_PgStat(); *** src/backend/catalog/heap.c --- src/backend/catalog/heap.c *** *** 39,44 --- 39,45 #include catalog/heap.h #include catalog/index.h #include catalog/indexing.h + #include catalog/namespace.h #include catalog/pg_attrdef.h
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: As I have not found yet an elegant solution to deal with the DROP CASCADE issue, here is a simpler patch that handles temp tables that are dropped at commit time. This is a good first step and we will try to elaborate further to support ON COMMIT DELETE ROWS. The problem with stopping the postmaster seems to still be there.. All the problems are centered around locking. We need to address that and decide what is sane locking behavior wrt. temp tables and 2PC. First, there's the case where a temp table is created and dropped in the same transaction. It seems perfectly sane to me to simply drop all locks on the dropped table at PREPARE TRANSACTION. Does anyone see a problem with that? If not, we might as well do that for all tables, not just temporary ones. It seems just as safe for non-temporary tables. Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. There too, could we simply drop the locks at PREPARE TRANSACTION? I can't immediately see anything wrong with that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi Heikki, Emmanuel Cecchet wrote: As I have not found yet an elegant solution to deal with the DROP CASCADE issue, here is a simpler patch that handles temp tables that are dropped at commit time. This is a good first step and we will try to elaborate further to support ON COMMIT DELETE ROWS. The problem with stopping the postmaster seems to still be there.. All the problems are centered around locking. We need to address that and decide what is sane locking behavior wrt. temp tables and 2PC. First, there's the case where a temp table is created and dropped in the same transaction. It seems perfectly sane to me to simply drop all locks on the dropped table at PREPARE TRANSACTION. Does anyone see a problem with that? If not, we might as well do that for all tables, not just temporary ones. It seems just as safe for non-temporary tables. This seems good to me. Any access to the table after PREPARE TRANSACTION but before COMMIT on that backend would return a relation not found which is what we expect. For a regular table, I don't know if that makes a difference if the prepared transaction rollbacks? Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. There too, could we simply drop the locks at PREPARE TRANSACTION? I can't immediately see anything wrong with that. As there is no data anyway, I don't think the locks are going to change anything. But in the most recent stripped-down version of the patch, on delete rows is no more supported, I only allow on commit drop. I did not find the flag to see if a temp table was created with the on delete rows option. Do you want me to look at the locking code or will you have time to do it? Hints will be welcome if you want me to do it. Thanks, Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi, As I have not found yet an elegant solution to deal with the DROP CASCADE issue, here is a simpler patch that handles temp tables that are dropped at commit time. This is a good first step and we will try to elaborate further to support ON COMMIT DELETE ROWS. I have also added a compilation of the tests I have even if some are not really relevant anymore without the support for empty temp tables but we will probably reuse them later. Thanks in advance for the feedback, Emmanuel Emmanuel Cecchet wrote: Heikki Linnakangas wrote: Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. An empty temp table at PREPARE time would be similar to an ON COMMIT DELETE ROW table. I think you'll want to check explicitly that the table is defined with ON COMMIT DELETE ROWS, instead of checking that it's empty. Where can I find the field containing the CREATE options for the temp table? Yeah, thanks to MVCC, it's possible that the table looks empty to the transaction being prepared, using SnapshotNow, but there's some tuples that are still visible to other transactions. For example: CREATE TEMPORARY TABLE foo (id int4); INSERT INTO foo VALUES (1); begin; DELETE FROM foo; PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is empty, according to SnapshotNow SELECT * FROM foo; -- Still shows the one row, because the deleting transaction hasn't committed yet. Is that a problem? If your transaction isolation level is not serializable the SELECT will not block and return the current snapshot. From the transaction standpoint, it is fine that the transaction can prepare or am I missing something? Actually, I did a test and if the temp table is created with 'on commit delete rows' option, the select blocks until the transaction is committed. This seems a normal behavior to me. I don't think you can just ignore prepared temp relations in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. Do you mean that it will break the DROP CASCADE behavior in general, or that would break the behavior for master/child temp tables? For temp tables, I suppose. I confirm that doing a drop cascade on a master temp table after a prepared transaction committed from another backend will not drop the children for now. The hack in findDependentObjects still isn't enough, anyway. If you have a prepared transaction that created a temp table, the database doesn't shut down: $ bin/pg_ctl -D data start server starting $ LOG: database system was shut down at 2008-11-04 10:27:27 EST LOG: autovacuum launcher started LOG: database system is ready to accept connections $ bin/psql postgres -c begin; CREATE TEMPORARY TABLE temp (id integer); PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION [EMAIL PROTECTED]:~/pgsql.fsmfork$ bin/pg_ctl -D data stop LOG: received smart shutdown request LOG: autovacuum launcher shutting down waiting for server to shut down... failed pg_ctl: server does not shut down Interesting case, if the table is created but not accessed it is not enlisted and then the shutdown does not catch this dependency. The table should be enlisted at CREATE time as well. The bookkeeping of prepared commit tables is just for the shutdown case right now. If you think it is a bad idea altogether to have session temp tables (even with delete rows on commit) that can cross commit boundaries, then we can remove that second bookkeeping and only allow temp tables that have been created withing the scope of the transaction. I fixed the hash_freeze problem but this drop cascade on temp table seems to be an issue (if anyone uses that feature). Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/backend/access/heap/heapam.c === RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.268 diff -u -r1.268 heapam.c --- src/backend/access/heap/heapam.c31 Oct 2008 19:40:26 - 1.268 +++ src/backend/access/heap/heapam.c7 Nov 2008 23:09:11 - @@ -878,7 +878,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -926,7 +926,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -976,7 +976,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel =
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: What's the purpose of checking that a table is empty on prepare? I think I'd feel more comfortable with the approach of only accepting PREPARE TRANSACTIOn if the accessed temp tables have been created and destroyed in the same transaction, to avoid possibly surprising behavior when a temp table is kept locked by a prepared transaction and you try to drop it later in the sesssion, but the patch allows more than that. I guess accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. An empty temp table at PREPARE time would be similar to an ON COMMIT DELETE ROW table. I think you'll want to check explicitly that the table is defined with ON COMMIT DELETE ROWS, instead of checking that it's empty. but checking that there's no visible rows in the table doesn't achieve that. If the relation exist but contains no row, is it possible that the table is not empty? What would I need to do to ensure that the table is empty? Yeah, thanks to MVCC, it's possible that the table looks empty to the transaction being prepared, using SnapshotNow, but there's some tuples that are still visible to other transactions. For example: CREATE TEMPORARY TABLE foo (id int4); INSERT INTO foo VALUES (1); begin; DELETE FROM foo; PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is empty, according to SnapshotNow SELECT * FROM foo; -- Still shows the one row, because the deleting transaction hasn't committed yet. I don't think you can just ignore prepared temp relations in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. Do you mean that it will break the DROP CASCADE behavior in general, or that would break the behavior for master/child temp tables? For temp tables, I suppose. The hack in findDependentObjects still isn't enough, anyway. If you have a prepared transaction that created a temp table, the database doesn't shut down: $ bin/pg_ctl -D data start server starting $ LOG: database system was shut down at 2008-11-04 10:27:27 EST LOG: autovacuum launcher started LOG: database system is ready to accept connections $ bin/psql postgres -c begin; CREATE TEMPORARY TABLE temp (id integer); PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION [EMAIL PROTECTED]:~/pgsql.fsmfork$ bin/pg_ctl -D data stop LOG: received smart shutdown request LOG: autovacuum launcher shutting down waiting for server to shut down... failed pg_ctl: server does not shut down By the way, does Postgres support child temp tables? Yes. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Heikki Linnakangas wrote: Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. An empty temp table at PREPARE time would be similar to an ON COMMIT DELETE ROW table. I think you'll want to check explicitly that the table is defined with ON COMMIT DELETE ROWS, instead of checking that it's empty. Where can I find the field containing the CREATE options for the temp table? Yeah, thanks to MVCC, it's possible that the table looks empty to the transaction being prepared, using SnapshotNow, but there's some tuples that are still visible to other transactions. For example: CREATE TEMPORARY TABLE foo (id int4); INSERT INTO foo VALUES (1); begin; DELETE FROM foo; PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is empty, according to SnapshotNow SELECT * FROM foo; -- Still shows the one row, because the deleting transaction hasn't committed yet. Is that a problem? If your transaction isolation level is not serializable the SELECT will not block and return the current snapshot. From the transaction standpoint, it is fine that the transaction can prepare or am I missing something? Actually, I did a test and if the temp table is created with 'on commit delete rows' option, the select blocks until the transaction is committed. This seems a normal behavior to me. I don't think you can just ignore prepared temp relations in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. Do you mean that it will break the DROP CASCADE behavior in general, or that would break the behavior for master/child temp tables? For temp tables, I suppose. I confirm that doing a drop cascade on a master temp table after a prepared transaction committed from another backend will not drop the children for now. The hack in findDependentObjects still isn't enough, anyway. If you have a prepared transaction that created a temp table, the database doesn't shut down: $ bin/pg_ctl -D data start server starting $ LOG: database system was shut down at 2008-11-04 10:27:27 EST LOG: autovacuum launcher started LOG: database system is ready to accept connections $ bin/psql postgres -c begin; CREATE TEMPORARY TABLE temp (id integer); PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION [EMAIL PROTECTED]:~/pgsql.fsmfork$ bin/pg_ctl -D data stop LOG: received smart shutdown request LOG: autovacuum launcher shutting down waiting for server to shut down... failed pg_ctl: server does not shut down Interesting case, if the table is created but not accessed it is not enlisted and then the shutdown does not catch this dependency. The table should be enlisted at CREATE time as well. The bookkeeping of prepared commit tables is just for the shutdown case right now. If you think it is a bad idea altogether to have session temp tables (even with delete rows on commit) that can cross commit boundaries, then we can remove that second bookkeeping and only allow temp tables that have been created withing the scope of the transaction. I fixed the hash_freeze problem but this drop cascade on temp table seems to be an issue (if anyone uses that feature). Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Here is the latest patch and the regression tests for the temp tables and 2PC issue. This fails: postgres=# begin; BEGIN postgres=# CREATE TEMPORARY TABLE temp1 (id int4); CREATE TABLE postgres=# PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION postgres=# CREATE TEMPORARY TABLE temp2 (id int4); ERROR: cannot insert into frozen hashtable accessed temp tables I don't understand the bookkeeping of accessed and prepared temp tables in general. What's it for? The comments on preparedTempRel says that it keeps track of accessed temporary relations that have been prepared commit but not committed yet. That's never going to work as a backend-private hash table, because there's no way to remove entries from it when the prepared transaction is committed or rolled back from another backend. What's the purpose of checking that a table is empty on prepare? I think I'd feel more comfortable with the approach of only accepting PREPARE TRANSACTIOn if the accessed temp tables have been created and destroyed in the same transaction, to avoid possibly surprising behavior when a temp table is kept locked by a prepared transaction and you try to drop it later in the sesssion, but the patch allows more than that. I guess accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, but checking that there's no visible rows in the table doesn't achieve that. I don't think you can just ignore prepared temp relations in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi Heikki, Emmanuel Cecchet wrote: Here is the latest patch and the regression tests for the temp tables and 2PC issue. This fails: postgres=# begin; BEGIN postgres=# CREATE TEMPORARY TABLE temp1 (id int4); CREATE TABLE postgres=# PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION postgres=# CREATE TEMPORARY TABLE temp2 (id int4); ERROR: cannot insert into frozen hashtable accessed temp tables I will address that. I don't understand the bookkeeping of accessed and prepared temp tables in general. What's it for? Right now (in 8.3) the bookkeeping prevents a transaction that has used a temp table to prepare commit. As you mentioned earlier (http://archives.postgresql.org/pgsql-hackers/2008-02/msg01277.php) we should be able to allow CREATE+DROP in the same transaction. The comments on preparedTempRel says that it keeps track of accessed temporary relations that have been prepared commit but not committed yet. That's never going to work as a backend-private hash table, because there's no way to remove entries from it when the prepared transaction is committed or rolled back from another backend. It does not really matter since we only allow empty temp tables at prepared time. And the transaction can only be prepared locally. If the transaction is committed or rolled back from another backend, the only thing that can happen is that tables that were created in the transaction will remain in the list. They will be ignored at the next prepare since the relation will not exist anymore. Once again, the tables remaining in the list after prepare are empty. What's the purpose of checking that a table is empty on prepare? I think I'd feel more comfortable with the approach of only accepting PREPARE TRANSACTIOn if the accessed temp tables have been created and destroyed in the same transaction, to avoid possibly surprising behavior when a temp table is kept locked by a prepared transaction and you try to drop it later in the sesssion, but the patch allows more than that. I guess accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. An empty temp table at PREPARE time would be similar to an ON COMMIT DELETE ROW table. but checking that there's no visible rows in the table doesn't achieve that. If the relation exist but contains no row, is it possible that the table is not empty? What would I need to do to ensure that the table is empty? I don't think you can just ignore prepared temp relations in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. Do you mean that it will break the DROP CASCADE behavior in general, or that would break the behavior for master/child temp tables? By the way, does Postgres support child temp tables? Thanks for the feedback. I will address the problem of the frozen hash list but let me know what you think of the other potential issues. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Here is the latest patch and the regression tests for the temp tables and 2PC issue. Is there a way to stop and restart postmaster between 2 tests? Thanks for your feedback, Emmanuel I did not get any comment on that one. How should I proceed so that the patch integration can be considered for 8.4? Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Emmanuel Cecchet wrote: Here is the latest patch and the regression tests for the temp tables and 2PC issue. Is there a way to stop and restart postmaster between 2 tests? Thanks for your feedback, Emmanuel I did not get any comment on that one. How should I proceed so that the patch integration can be considered for 8.4? http://wiki.postgresql.org/wiki/Submitting_a_Patch -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi all, Here is the latest patch and the regression tests for the temp tables and 2PC issue. Is there a way to stop and restart postmaster between 2 tests? Thanks for your feedback, Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/test/regress/parallel_schedule === RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v retrieving revision 1.49 diff -u -r1.49 parallel_schedule --- src/test/regress/parallel_schedule 4 Oct 2008 21:56:55 - 1.49 +++ src/test/regress/parallel_schedule 10 Oct 2008 18:33:01 - @@ -90,3 +90,19 @@ # run tablespace by itself test: tablespace + +# +# 2PC related tests +# +test: 2pc_persistent_table 2pc_on_delete_rows_transaction 2pc_on_commit_drop 2pc_temp_table_transaction 2pc_temp_table_rollback 2pc_temp_table_savepoint 2pc_temp_table_failure +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed + \ No newline at end of file Index: src/test/regress/serial_schedule === RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v retrieving revision 1.46 diff -u -r1.46 serial_schedule --- src/test/regress/serial_schedule4 Oct 2008 21:56:55 - 1.46 +++ src/test/regress/serial_schedule10 Oct 2008 18:33:01 - @@ -118,3 +118,20 @@ test: xml test: stats test: tablespace +test: 2pc_persistent_table +test: 2pc_on_delete_rows_transaction +test: 2pc_on_commit_drop +test: 2pc_temp_table_transaction +test: 2pc_temp_table_rollback +test: 2pc_temp_table_savepoint +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +test: 2pc_temp_table_failure +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed Index: src/test/regress/expected/2pc_temp_table_failure.out === RCS file: src/test/regress/expected/2pc_temp_table_failure.out diff -N src/test/regress/expected/2pc_temp_table_failure.out --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/expected/2pc_temp_table_failure.out1 Jan 1970 00:00:00 - @@ -0,0 +1,8 @@ +-- Existing non-empty temp table at commit time should still fail +begin; +create temp table foo(x int); +insert into foo values(1); +prepare transaction 'existingTempTableShouldFail'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not empty at commit time +commit prepared 'existingTempTableShouldFail'; +ERROR: prepared transaction with identifier existingTempTableShouldFail does not exist Index: src/test/regress/expected/2pc_temp_table_commit_prepared.out === RCS file: src/test/regress/expected/2pc_temp_table_commit_prepared.out diff -N src/test/regress/expected/2pc_temp_table_commit_prepared.out --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/expected/2pc_temp_table_commit_prepared.out1 Jan 1970 00:00:00 - @@ -0,0 +1,8 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- This test has to be called in sequence after 2pc_temp_table_prepared_not_committed.sql +commit prepared 'preparedNonCommittedFoo'; +-- The table should not exist anymore in this session +drop table foo; +ERROR: table foo does not exist Index: src/test/regress/expected/2pc_on_delete_rows_session.out === RCS file: src/test/regress/expected/2pc_on_delete_rows_session.out diff -N src/test/regress/expected/2pc_on_delete_rows_session.out --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/expected/2pc_on_delete_rows_session.out1 Jan 1970 00:00:00 - @@ -0,0 +1,7 @@ +-- Temp table with ON DELETE ROWS option (session scope) +create temp table foo(x int) on commit delete rows; +begin; +insert into foo values(1); +prepare transaction 'onCommitDeleteRowsTempTableShouldSucceed'; +commit prepared
Re: [HACKERS] Transactions and temp tables
Hi, I am attaching a new patch that deals with the issue of the locks on temporary tables that have been accessed in transactions that have been prepared but not committed. I have added another list that keeps track of temp tables accessed by transactions that are prepared but not committed. The RemoveTempTable callback does not try to acquire locks on these tables. Now postmaster can terminate even with transactions in the prepared state that accessed temp tables. Let me know what you think. manu Tom Lane wrote: Emmanuel Cecchet [EMAIL PROTECTED] writes: Ok, so actually I don't see any different behavior between a temp table or a regular table. The locking happens the same way and as long as the commit prepared happens (potentially in another session), the lock is released at commit time which seems to make sense. Right, the problem is that you can't shut down the original backend because it'll try to drop the temp table at exit, and then block on the lock that the prepared xact is holding. From a database management standpoint that is unacceptable --- it means for instance that you can't shut down the database cleanly while such a prepared transaction is pending. The difference from a regular table is that no such automatic action is taken at backend exit for regular tables. The whole business of having restrictions on temp table access is annoying; I wish we could get rid of them not add complexity to enforcing them. The local-buffer-management end of the issue seems readily solvable: we need only decree that PREPARE has to flush out any dirty local buffers (and maybe discard local buffers altogether, not sure). But I've not been able to see a decent solution to the lock behavior. regards, tom lane ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/backend/access/heap/heapam.c === RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.264 diff -u -r1.264 heapam.c --- src/backend/access/heap/heapam.c30 Sep 2008 10:52:10 - 1.264 +++ src/backend/access/heap/heapam.c9 Oct 2008 21:44:04 - @@ -878,7 +878,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -926,7 +926,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -976,7 +976,7 @@ /* Make note that we've accessed a temporary relation */ if (r-rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); Index: src/backend/access/transam/xact.c === RCS file: /root/cvsrepo/pgsql/src/backend/access/transam/xact.c,v retrieving revision 1.265 diff -u -r1.265 xact.c --- src/backend/access/transam/xact.c 11 Aug 2008 11:05:10 - 1.265 +++ src/backend/access/transam/xact.c 9 Oct 2008 21:44:04 - @@ -47,6 +47,7 @@ #include utils/memutils.h #include utils/relcache.h #include utils/snapmgr.h +#include utils/tqual.h #include utils/xml.h #include pg_trace.h @@ -65,13 +66,6 @@ intCommitDelay = 0;/* precommit delay in microseconds */ intCommitSiblings = 5; /* # concurrent xacts needed to sleep */ -/* - * MyXactAccessedTempRel is set when a temporary relation is accessed. - * We don't allow PREPARE TRANSACTION in that case. (This is global - * so that it can be set from heapam.c.) - */ -bool MyXactAccessedTempRel = false; - /* * transaction states - transaction state from server perspective @@ -209,6 +203,13 @@ */ static MemoryContext TransactionAbortContext = NULL; +/* Hash table containing Oids of accessed temporary relations */ +HTAB *accessedTempRel; +/* Hash table containing Oids of accessed temporary relations that have been + * prepared commit but not committed yet + */ +HTAB *preparedTempRel; + /* * List of add-on start- and end-of-xact callbacks */ @@ -250,6 +251,7 @@ SubTransactionId mySubid, SubTransactionId parentSubid); static void CleanupTransaction(void); +static void CleanupAccessedTempRel(void); static void CommitTransaction(void); static TransactionId RecordTransactionAbort(bool isSubXact); static void StartTransaction(void); @@ -624,7 +626,7 @@ /* Propagate new command ID into static snapshots */ SnapshotSetCommandId(currentCommandId); - + /* * Make any catalog changes
Re: [HACKERS] Transactions and temp tables
Heikki Linnakangas wrote: I was thinking of a transaction that's just prepared (1st phase), but not committed or rolled back: postgres=# CREATE TEMP TABLE foo (bar int); CREATE TABLE postgres=# BEGIN; BEGIN postgres=# DROP TABLE foo; DROP TABLE postgres=# PREPARE TRANSACTION '2pc'; PREPARE TRANSACTION postgres=# SELECT * FROM foo; blocks Furthermore, it looks like the backend refuses to shut down, even if you end the psql session, because RemoveTempRelations() is called on backend shutdown and it gets blocked on that lock. Thanks for the example, I get it now. Does it make sense to allow any request execution between PREPARE TRANSACTION and the subsequent COMMIT or ROLLBACK? I did the same experiment with a regular table (not a temp table) and it blocks exactly the same way, so I don't think that the problem is specific to temp tables. Once PREPARE has been executed, the transaction state is restored to TRANS_DEFAULT, but I wonder if we should not have a specific TRANS_PREPARED state in which we can only authorize a COMMIT or a ROLLBACK. Is there any reasonable use case where someone would have to execute requests between PREPARE and COMMIT/ROLLBACK? Let me know what you think of the additional TRANS_PREPARED transaction state. It looks like the behavior of what happens between PREPARE and COMMIT/ROLLBACK is pretty much undefined. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet [EMAIL PROTECTED] writes: Thanks for the example, I get it now. Does it make sense to allow any request execution between PREPARE TRANSACTION and the subsequent COMMIT or ROLLBACK? Yes. Don't even think of trying to disallow that. The COMMIT doesn't even have to happen in the same session, anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Also, even if the table is created and dropped in the same transaction, a subsequent transaction that tries to create and drop the table gets blocked on the lock. I suppose we could just say that that's the way it works, but I'm afraid it will come as a nasty surprise, making the feature a lot less useful. I do not get that one, if the table is dropped in the transaction the lock is released. Why would another transaction be blocked when trying to create/drop another temp table? I was thinking of a transaction that's just prepared (1st phase), but not committed or rolled back: postgres=# CREATE TEMP TABLE foo (bar int); CREATE TABLE postgres=# BEGIN; BEGIN postgres=# DROP TABLE foo; DROP TABLE postgres=# PREPARE TRANSACTION '2pc'; PREPARE TRANSACTION postgres=# SELECT * FROM foo; blocks Furthermore, it looks like the backend refuses to shut down, even if you end the psql session, because RemoveTempRelations() is called on backend shutdown and it gets blocked on that lock. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Tom Lane wrote: Emmanuel Cecchet [EMAIL PROTECTED] writes: Thanks for the example, I get it now. Does it make sense to allow any request execution between PREPARE TRANSACTION and the subsequent COMMIT or ROLLBACK? Yes. Don't even think of trying to disallow that. The COMMIT doesn't even have to happen in the same session, anyway. Ok, so actually I don't see any different behavior between a temp table or a regular table. The locking happens the same way and as long as the commit prepared happens (potentially in another session), the lock is released at commit time which seems to make sense. The issue that Heikki was mentioning about the server not shutting down seems to happen as soon as you have a single transaction that has prepared commit but not commit/rollback yet. This seems also independent of whether you are using a temp table or not. It seems that the patch did not alter the behavior of PG in that regard. What do you think? Emmanuel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet [EMAIL PROTECTED] writes: Ok, so actually I don't see any different behavior between a temp table or a regular table. The locking happens the same way and as long as the commit prepared happens (potentially in another session), the lock is released at commit time which seems to make sense. Right, the problem is that you can't shut down the original backend because it'll try to drop the temp table at exit, and then block on the lock that the prepared xact is holding. From a database management standpoint that is unacceptable --- it means for instance that you can't shut down the database cleanly while such a prepared transaction is pending. The difference from a regular table is that no such automatic action is taken at backend exit for regular tables. The whole business of having restrictions on temp table access is annoying; I wish we could get rid of them not add complexity to enforcing them. The local-buffer-management end of the issue seems readily solvable: we need only decree that PREPARE has to flush out any dirty local buffers (and maybe discard local buffers altogether, not sure). But I've not been able to see a decent solution to the lock behavior. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Emmanuel Cecchet wrote: Instead of relying on a boolean that tells if a temp table was accessed, I keep a list of the Oid for the temp tables accessed in the transaction and at prepare commit time, I check if the relations are still valid. I also added a check to allow empty temp tables at prepare commit time (this allows to use temp tables with 'on commit delete rows' options. I am attaching the patch and the use cases I have been using to test it. The test cases try to compile the various use cases that I have seen reported on the list. Thanks for looking into this. The patch allows preparing any transaction that has dropped the temp table, even if it wasn't created in the same transaction. Is that sane? Also, even if the table is created and dropped in the same transaction, a subsequent transaction that tries to create and drop the table gets blocked on the lock. I suppose we could just say that that's the way it works, but I'm afraid it will come as a nasty surprise, making the feature a lot less useful. The fixed-size array of temp table oids is an unnecessary limitation. A list or hash table would be better. Let me know what you think of the patch and if it could be applied to 8.3 and 8.4? Not to 8.3. We only back-patch bug-fixes, and this isn't one. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi Heikki, The patch allows preparing any transaction that has dropped the temp table, even if it wasn't created in the same transaction. Is that sane? If you have a temp table created with an 'on commit delete rows' option in another transaction, it would be fine to drop it in another transaction. If the temp table was created without any on commit option, it could only cross prepare commit if it is empty and then it could be safely dropped in another transaction. That does not seem to insane for me if you need temp tables for a session. Also, even if the table is created and dropped in the same transaction, a subsequent transaction that tries to create and drop the table gets blocked on the lock. I suppose we could just say that that's the way it works, but I'm afraid it will come as a nasty surprise, making the feature a lot less useful. I do not get that one, if the table is dropped in the transaction the lock is released. Why would another transaction be blocked when trying to create/drop another temp table? When I run my test cases (see attached file in previous mail), I create/drop multiple times the same temp table in different transactions and I do not experience any blocking. The fixed-size array of temp table oids is an unnecessary limitation. A list or hash table would be better. You are right, I will fix that. Let me know what you think of the patch and if it could be applied to 8.3 and 8.4? Not to 8.3. We only back-patch bug-fixes, and this isn't one. Ok understood. Thanks for your feedback and don't hesitate to enlighten me on the potential locking issue I did not understand. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Heikki, Here is a new version of the patch using a hash table as you suggested. I also include the tests that I have added to the regression test suite to test the various scenarios. All patches are based on Postgres 8.3.3, let me know if you want me to generate patch for 8.4. Thanks in advance for your feedback, Emmanuel Emmanuel Cecchet wrote: Hi Heikki, The patch allows preparing any transaction that has dropped the temp table, even if it wasn't created in the same transaction. Is that sane? If you have a temp table created with an 'on commit delete rows' option in another transaction, it would be fine to drop it in another transaction. If the temp table was created without any on commit option, it could only cross prepare commit if it is empty and then it could be safely dropped in another transaction. That does not seem to insane for me if you need temp tables for a session. Also, even if the table is created and dropped in the same transaction, a subsequent transaction that tries to create and drop the table gets blocked on the lock. I suppose we could just say that that's the way it works, but I'm afraid it will come as a nasty surprise, making the feature a lot less useful. I do not get that one, if the table is dropped in the transaction the lock is released. Why would another transaction be blocked when trying to create/drop another temp table? When I run my test cases (see attached file in previous mail), I create/drop multiple times the same temp table in different transactions and I do not experience any blocking. The fixed-size array of temp table oids is an unnecessary limitation. A list or hash table would be better. You are right, I will fix that. Let me know what you think of the patch and if it could be applied to 8.3 and 8.4? Not to 8.3. We only back-patch bug-fixes, and this isn't one. Ok understood. Thanks for your feedback and don't hesitate to enlighten me on the potential locking issue I did not understand. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote: Heikki, Here is a new version of the patch using a hash table as you suggested. I also include the tests that I have added to the regression test suite to test the various scenarios. All patches are based on Postgres 8.3.3, let me know if you want me to generate patch for 8.4. CVS TIP is the only place where new features, like this, go :) I didn't see the attachment anyhow... Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi,
On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
Heikki,
Here is a new version of the patch using a hash table as you
suggested. I also include the tests that I have added to the
regression test suite to test the various scenarios. All patches
are based on Postgres 8.3.3, let me know if you want me to generate
patch for 8.4.
CVS TIP is the only place where new features, like this, go :)
I looked at the Wiki and it looks like I should add en entry (assuming I
get a patch for the current CVS HEAD) to CommitFest 2008-11. Is that
correct?
I didn't see the attachment anyhow...
Good point! The same with the attachments now!
Thanks,
manu
### Eclipse Workspace Patch 1.0
#P Postgres-8.3.3
Index: src/include/access/xact.h
===
RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v
retrieving revision 1.93.2.1
diff -u -r1.93.2.1 xact.h
--- src/include/access/xact.h 4 Mar 2008 19:54:13 - 1.93.2.1
+++ src/include/access/xact.h 7 Oct 2008 20:29:47 -
@@ -18,6 +18,7 @@
#include nodes/pg_list.h
#include storage/relfilenode.h
#include utils/timestamp.h
+#include postgres_ext.h
/*
@@ -44,8 +45,8 @@
/* Asynchronous commits */
extern bool XactSyncCommit;
-/* Kluge for 2PC support */
-extern bool MyXactAccessedTempRel;
+/* List of temp tables accessed during a transaction for 2PC support */
+extern void enlistRelationIdFor2PCChecks(Oid relationId);
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
Index: src/backend/access/heap/heapam.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.249.2.2
diff -u -r1.249.2.2 heapam.c
--- src/backend/access/heap/heapam.c8 Mar 2008 21:58:07 -
1.249.2.2
+++ src/backend/access/heap/heapam.c7 Oct 2008 20:29:47 -
@@ -870,7 +870,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -918,7 +918,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -968,7 +968,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
Index: src/backend/access/transam/xact.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257.2.2
diff -u -r1.257.2.2 xact.c
--- src/backend/access/transam/xact.c 26 Apr 2008 23:35:33 -
1.257.2.2
+++ src/backend/access/transam/xact.c 7 Oct 2008 20:29:47 -
@@ -62,13 +62,6 @@
intCommitDelay = 0;/* precommit delay in
microseconds */
intCommitSiblings = 5; /* # concurrent xacts needed to
sleep */
-/*
- * MyXactAccessedTempRel is set when a temporary relation is accessed.
- * We don't allow PREPARE TRANSACTION in that case. (This is global
- * so that it can be set from heapam.c.)
- */
-bool MyXactAccessedTempRel = false;
-
/*
* transaction states - transaction state from server perspective
@@ -206,6 +199,9 @@
*/
static MemoryContext TransactionAbortContext = NULL;
+/* Hash table containing Oids of accessed temporary relations */
+HTAB *accessedTempRel;
+
/*
* List of add-on start- and end-of-xact callbacks
*/
@@ -1512,7 +1508,6 @@
XactIsoLevel = DefaultXactIsoLevel;
XactReadOnly = DefaultXactReadOnly;
forceSyncCommit = false;
- MyXactAccessedTempRel = false;
/*
* reinitialize within-transaction counters
@@ -1777,6 +1772,35 @@
RESUME_INTERRUPTS();
}
+/*
+ * enlistRelationIdFor2PCChecks - enlist a relation in the list of
+ * resources to check at PREPARE COMMIT time if we are part of
+ * a 2PC transaction. The resource will be removed from the list
+ * if the table is dropped before commit.
+ *
+ */
+void
+enlistRelationIdFor2PCChecks(Oid relationId)
+{
+ /*
+* Each time a temporary relation is accessed, it is added to the
+* accessedTempRel list. PREPARE TRANSACTION will fail if any
+* of the accessed relation is still valid (not dropped). (This is
+* called from from heapam.c.)
+*/
+ if (accessedTempRel == NULL)
+ { // Allocate the list on-demand
+ HASHCTL ctl;
+
+ ctl.keysize = sizeof(Oid);
+ ctl.entrysize = sizeof(Oid);
+
Re: [HACKERS] Transactions and temp tables
On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote: Hi, On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote: Heikki, Here is a new version of the patch using a hash table as you suggested. I also include the tests that I have added to the regression test suite to test the various scenarios. All patches are based on Postgres 8.3.3, let me know if you want me to generate patch for 8.4. CVS TIP is the only place where new features, like this, go :) I looked at the Wiki and it looks like I should add en entry (assuming I get a patch for the current CVS HEAD) to CommitFest 2008-11. Is that correct? I didn't see the attachment anyhow... Good point! The same with the attachments now! Perhaps we did not make this clear. The patch is a new feature. New features are not going into 8.3, as it has already been released. Make a patch against CVS TIP aka 8.4, and re-submit. Cheers, David -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Transactions and temp tables
Hi, Here are the patches for 8.4 (1 patch for the code and 1 patch for the regression tests). Thanks in advance for your feedback, Emmanuel David Fetter wrote: On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote: Hi, On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote: Heikki, Here is a new version of the patch using a hash table as you suggested. I also include the tests that I have added to the regression test suite to test the various scenarios. All patches are based on Postgres 8.3.3, let me know if you want me to generate patch for 8.4. CVS TIP is the only place where new features, like this, go :) I looked at the Wiki and it looks like I should add en entry (assuming I get a patch for the current CVS HEAD) to CommitFest 2008-11. Is that correct? I didn't see the attachment anyhow... Good point! The same with the attachments now! Perhaps we did not make this clear. The patch is a new feature. New features are not going into 8.3, as it has already been released. Make a patch against CVS TIP aka 8.4, and re-submit. Cheers, David -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development Consulting -- Web: http://www.frogthinker.org email: [EMAIL PROTECTED] Skype: emmanuel_cecchet ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/test/regress/parallel_schedule === RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v retrieving revision 1.46 diff -u -r1.46 parallel_schedule --- src/test/regress/parallel_schedule 24 Nov 2007 19:49:23 - 1.46 +++ src/test/regress/parallel_schedule 7 Oct 2008 23:41:14 - @@ -90,3 +90,11 @@ # run tablespace by itself test: tablespace + +# +# 2PC related tests +# +test: 2pc_persistent_table 2pc_on_delete_rows_transaction 2pc_on_commit_drop 2pc_temp_table_transaction 2pc_temp_table_rollback 2pc_temp_table_savepoint 2pc_temp_table_failure +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session \ No newline at end of file Index: src/test/regress/serial_schedule === RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v retrieving revision 1.43 diff -u -r1.43 serial_schedule --- src/test/regress/serial_schedule24 Nov 2007 20:41:35 - 1.43 +++ src/test/regress/serial_schedule7 Oct 2008 23:41:14 - @@ -115,3 +115,13 @@ test: xml test: stats test: tablespace +test: 2pc_persistent_table +test: 2pc_on_delete_rows_transaction +test: 2pc_on_commit_drop +test: 2pc_temp_table_transaction +test: 2pc_temp_table_rollback +test: 2pc_temp_table_savepoint +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +test: 2pc_temp_table_failure \ No newline at end of file Index: src/test/regress/sql/2pc_on_delete_rows_transaction.sql === RCS file: src/test/regress/sql/2pc_on_delete_rows_transaction.sql diff -N src/test/regress/sql/2pc_on_delete_rows_transaction.sql --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/sql/2pc_on_delete_rows_transaction.sql 1 Jan 1970 00:00:00 - @@ -0,0 +1,7 @@ +-- Temp table with ON DELETE ROWS option (transaction scope) +begin; +create temp table foo(x int) on commit delete rows; +insert into foo values(1); +prepare transaction 'onCommitDeleteRowsTempTableShouldSucceed'; +commit prepared 'onCommitDeleteRowsTempTableShouldSucceed'; +drop table foo; Index: src/test/regress/expected/2pc_on_commit_drop.out === RCS file: src/test/regress/expected/2pc_on_commit_drop.out diff -N src/test/regress/expected/2pc_on_commit_drop.out --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/expected/2pc_on_commit_drop.out1 Jan 1970 00:00:00 - @@ -0,0 +1,6 @@ +-- Temp table with ON COMMIT DROP option +begin; +create temp table foo(x int) on commit drop; +insert into foo values(1); +prepare transaction 'onCommitDropTempTableShouldSucceed'; +commit prepared 'onCommitDropTempTableShouldSucceed'; Index: src/test/regress/expected/2pc_temp_table_transaction.out === RCS file: src/test/regress/expected/2pc_temp_table_transaction.out diff -N src/test/regress/expected/2pc_temp_table_transaction.out --- /dev/null 1 Jan 1970 00:00:00 - +++ src/test/regress/expected/2pc_temp_table_transaction.out1 Jan 1970 00:00:00 - @@ -0,0 +1,7 @@ +-- Transaction-scope dropped temp table use case +begin; +create temp table foo(x int); +insert into foo values(1); +drop table foo; +prepare transaction 'dropTempTableShouldSucceed'; +commit prepared 'dropTempTableShouldSucceed'; Index: src/test/regress/sql/2pc_temp_table_failure.sql
[HACKERS] Transactions and temp tables
Hi,
I had the same problem as John with could not open relation
1663/16384/16584: No such file or directory in a specific combination
of transactions with temp tables
(http://archives.postgresql.org/pgsql-hackers/2008-02/msg01260.php). As
Heikki mentioned
(http://archives.postgresql.org/pgsql-hackers/2008-02/msg01277.php) we
should be able to allow CREATE+DROP in the same transaction.
I came up with a patch (currently based on 8.3.3) to address that issue.
Instead of relying on a boolean that tells if a temp table was accessed,
I keep a list of the Oid for the temp tables accessed in the transaction
and at prepare commit time, I check if the relations are still valid. I
also added a check to allow empty temp tables at prepare commit time
(this allows to use temp tables with 'on commit delete rows' options.
I am attaching the patch and the use cases I have been using to test it.
The test cases try to compile the various use cases that I have seen
reported on the list. Let me know what you think of the patch and if it
could be applied to 8.3 and 8.4?
Thanks in advance for your feedback,
manu
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development Consulting
--
Web: http://www.frogthinker.org
email: [EMAIL PROTECTED]
Skype: emmanuel_cecchet
### Eclipse Workspace Patch 1.0
#P Postgres-8.3.3
Index: src/include/access/xact.h
===
RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v
retrieving revision 1.93.2.1
diff -u -r1.93.2.1 xact.h
--- src/include/access/xact.h 4 Mar 2008 19:54:13 - 1.93.2.1
+++ src/include/access/xact.h 6 Oct 2008 20:39:46 -
@@ -18,6 +18,7 @@
#include nodes/pg_list.h
#include storage/relfilenode.h
#include utils/timestamp.h
+#include postgres_ext.h
/*
@@ -44,8 +45,8 @@
/* Asynchronous commits */
extern bool XactSyncCommit;
-/* Kluge for 2PC support */
-extern bool MyXactAccessedTempRel;
+/* List of temp tables accessed during a transaction for 2PC support */
+extern void enlistRelationIdFor2PCChecks(Oid relationId);
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
Index: src/backend/access/heap/heapam.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.249.2.2
diff -u -r1.249.2.2 heapam.c
--- src/backend/access/heap/heapam.c8 Mar 2008 21:58:07 -
1.249.2.2
+++ src/backend/access/heap/heapam.c6 Oct 2008 20:39:46 -
@@ -870,7 +870,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -918,7 +918,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -968,7 +968,7 @@
/* Make note that we've accessed a temporary relation */
if (r-rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
Index: src/backend/access/transam/xact.c
===
RCS file: /root/cvsrepo/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257.2.2
diff -u -r1.257.2.2 xact.c
--- src/backend/access/transam/xact.c 26 Apr 2008 23:35:33 -
1.257.2.2
+++ src/backend/access/transam/xact.c 6 Oct 2008 20:39:46 -
@@ -62,13 +62,6 @@
intCommitDelay = 0;/* precommit delay in
microseconds */
intCommitSiblings = 5; /* # concurrent xacts needed to
sleep */
-/*
- * MyXactAccessedTempRel is set when a temporary relation is accessed.
- * We don't allow PREPARE TRANSACTION in that case. (This is global
- * so that it can be set from heapam.c.)
- */
-bool MyXactAccessedTempRel = false;
-
/*
* transaction states - transaction state from server perspective
@@ -206,6 +199,10 @@
*/
static MemoryContext TransactionAbortContext = NULL;
+#defineMAX_TEMP_TABLES_IN_A_TX 10
+#defineUNUSED_OID 0
+OidaccessedTempRel[MAX_TEMP_TABLES_IN_A_TX]; /* Oids of accessed
temporary relations */
+
/*
* List of add-on start- and end-of-xact callbacks
*/
@@ -1512,7 +1509,11 @@
XactIsoLevel = DefaultXactIsoLevel;
XactReadOnly = DefaultXactReadOnly;
forceSyncCommit = false;
- MyXactAccessedTempRel = false;
+ {
+ int i;
+ for (i = 0 ; i MAX_TEMP_TABLES_IN_A_TX ; i++)
+ accessedTempRel[i] = UNUSED_OID;
+ }
/*
* reinitialize within-transaction counters
@@ -1777,6 +1778,38 @@
