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

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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

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: manu@frogthinker.org
Skype: emmanuel_cecchet

Emmanuel Cecchet <manu@frogthinker.org> 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

Tom Lane wrote:

Emmanuel Cecchet <manu@frogthinker.org> 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

Emmanuel Cecchet <manu@frogthinker.org> 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

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:

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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 Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

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: manu@frogthinker.org
Skype: emmanuel_cecchet

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
hlinnaka@heikkilaptop:~/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