Connection Pools and DISCARD ALL
There's been a lively discussion on JDBC list recently about how we
handle connection pooling. This has connected a few thoughts in my head.
That's made me think about the PHP interface, which issues a
BEGIN; ROLLBACK;
pair every time somebody connects to the pool.
We should have a single/consistent way of starting a new connection to
PostgreSQL when using a session pool.
As committed, DISCARD ALL does everything but cannot be issued inside a
transaction block.
I'd like to propose that DISCARD ALL also issue a ROLLBACK command if it
is issued from within a transaction block. That way whenever we reassign
a session pool connection to another agent we can just issue a single
command from all interfaces, without needing to test what the state of
the connection is beforehand.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
Simon Riggs <simon@2ndquadrant.com> writes:
As committed, DISCARD ALL does everything but cannot be issued inside a
transaction block.
I'd like to propose that DISCARD ALL also issue a ROLLBACK command if it
is issued from within a transaction block.
That was *intentional* to prevent mistakes. Somebody who wants the
above behavior can send "ROLLBACK; DISCARD ALL".
regards, tom lane
On Thu, 2007-10-04 at 10:29 -0400, Tom Lane wrote:
Simon Riggs <simon@2ndquadrant.com> writes:
As committed, DISCARD ALL does everything but cannot be issued inside a
transaction block.I'd like to propose that DISCARD ALL also issue a ROLLBACK command if it
is issued from within a transaction block.That was *intentional* to prevent mistakes.
I understand; I'm challenging that intention. Neil's original commit
message said that was "intended to catch programmer mistakes" and that
such use is "probably unintended".
If the developer has attempted to issue it in the wrong place, he's
probably also forgot to handle errors correctly, i.e. ROLLBACK then
reissue.
If we care about helping the developer we should make the command end
the transaction block if one exists then issue it. Less code for the
developer, less mistakes.
Somebody who wants the
above behavior can send "ROLLBACK; DISCARD ALL".
...which generates an ERROR if no transaction is in progress and fills
the log needlessly.
http://svr5.postgresql.org/pgsql-interfaces/2001-02/msg00116.php
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
On Thu, 2007-10-04 at 15:50 +0100, Simon Riggs wrote:
On Thu, 2007-10-04 at 10:29 -0400, Tom Lane wrote:
Somebody who wants the
above behavior can send "ROLLBACK; DISCARD ALL"....which generates an ERROR if no transaction is in progress and fills
the log needlessly.
Well, it's a WARNING, but your point is taken. Can't a clueful interface
just check what the transaction status of the connection is, rather than
unconditionally issuing a ROLLBACK?
-Neil
On Thu, 2007-10-04 at 13:03 -0700, Neil Conway wrote:
On Thu, 2007-10-04 at 15:50 +0100, Simon Riggs wrote:
On Thu, 2007-10-04 at 10:29 -0400, Tom Lane wrote:
Somebody who wants the
above behavior can send "ROLLBACK; DISCARD ALL"....which generates an ERROR if no transaction is in progress and fills
the log needlessly.Well, it's a WARNING, but your point is taken. Can't a clueful interface
just check what the transaction status of the connection is, rather than
unconditionally issuing a ROLLBACK?
I think it can, but can't a clueful server do this and avoid the problem
of non-clueful interfaces?
This is making me think that we should just embed the session pool
inside the server as well and have done with it.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
Simon Riggs wrote:
This is making me think that we should just embed the session pool
inside the server as well and have done with it.
You mean prefork? That would be neat. I don't think it's all that
impossible.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Simon Riggs wrote:
On Thu, 2007-10-04 at 13:03 -0700, Neil Conway wrote:
On Thu, 2007-10-04 at 15:50 +0100, Simon Riggs wrote:
On Thu, 2007-10-04 at 10:29 -0400, Tom Lane wrote:
Somebody who wants the
above behavior can send "ROLLBACK; DISCARD ALL"....which generates an ERROR if no transaction is in progress and fills
the log needlessly.Well, it's a WARNING, but your point is taken. Can't a clueful interface
just check what the transaction status of the connection is, rather than
unconditionally issuing a ROLLBACK?I think it can, but can't a clueful server do this and avoid the problem
of non-clueful interfaces?This is making me think that we should just embed the session pool
inside the server as well and have done with it.
Could we maybe have some flavor of ROLLBACK that doesn't issue a warning
if no transaction is in progress? There is precedent for this sort of
facility - DROP ... IF EXISTS.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
Could we maybe have some flavor of ROLLBACK that doesn't issue a warning
if no transaction is in progress? There is precedent for this sort of
facility - DROP ... IF EXISTS.
Something that would actually be doable for 8.3 would be to downgrade
this particular WARNING to a NOTICE. A DBA who hasn't got
log_min_messages set higher than NOTICE hasn't really got a lot of room
to whine about bulky logs.
regards, tom lane
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Could we maybe have some flavor of ROLLBACK that doesn't issue a warning
if no transaction is in progress? There is precedent for this sort of
facility - DROP ... IF EXISTS.Something that would actually be doable for 8.3 would be to downgrade
this particular WARNING to a NOTICE. A DBA who hasn't got
log_min_messages set higher than NOTICE hasn't really got a lot of room
to whine about bulky logs.
I have developed the attached patch to implement this. I assume we want
to change ABORT outside a transaction from WARNING to NOTICE, but not
COMMIT.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachments:
/pgpatches/aborttext/x-diffDownload
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.251
diff -c -c -r1.251 xact.c
*** src/backend/access/transam/xact.c 11 Sep 2007 03:28:05 -0000 1.251
--- src/backend/access/transam/xact.c 9 Nov 2007 02:45:30 -0000
***************
*** 3025,3031 ****
* default state.
*/
case TBLOCK_STARTED:
! ereport(WARNING,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
errmsg("there is no transaction in progress")));
s->blockState = TBLOCK_ABORT_PENDING;
--- 3025,3031 ----
* default state.
*/
case TBLOCK_STARTED:
! ereport(NOTICE,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
errmsg("there is no transaction in progress")));
s->blockState = TBLOCK_ABORT_PENDING;
Index: src/test/regress/expected/errors.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/errors.out,v
retrieving revision 1.52
diff -c -c -r1.52 errors.out
*** src/test/regress/expected/errors.out 15 Apr 2006 17:45:46 -0000 1.52
--- src/test/regress/expected/errors.out 9 Nov 2007 02:45:31 -0000
***************
*** 116,122 ****
-- not in a xact
abort;
! WARNING: there is no transaction in progress
-- not in a xact
end;
WARNING: there is no transaction in progress
--- 116,122 ----
-- not in a xact
abort;
! NOTICE: there is no transaction in progress
-- not in a xact
end;
WARNING: there is no transaction in progress
On Thu, 2007-11-08 at 21:50 -0500, Bruce Momjian wrote:
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Could we maybe have some flavor of ROLLBACK that doesn't issue a warning
if no transaction is in progress? There is precedent for this sort of
facility - DROP ... IF EXISTS.Something that would actually be doable for 8.3 would be to downgrade
this particular WARNING to a NOTICE. A DBA who hasn't got
log_min_messages set higher than NOTICE hasn't really got a lot of room
to whine about bulky logs.I have developed the attached patch to implement this. I assume we want
to change ABORT outside a transaction from WARNING to NOTICE, but not
COMMIT.
Patch looks fine to me.
I've been trying to track down the code in the PHP interface that issues
it and see if we can make a version specific change. The code I'm
looking at now has already got that change, so I'm trying to understand
whether or not it works correctly.
But either way, I think the change is appropriate.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
Bruce Momjian wrote:
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Could we maybe have some flavor of ROLLBACK that doesn't issue a warning
if no transaction is in progress? There is precedent for this sort of
facility - DROP ... IF EXISTS.Something that would actually be doable for 8.3 would be to downgrade
this particular WARNING to a NOTICE. A DBA who hasn't got
log_min_messages set higher than NOTICE hasn't really got a lot of room
to whine about bulky logs.I have developed the attached patch to implement this. I assume we want
to change ABORT outside a transaction from WARNING to NOTICE, but not
COMMIT.
Patch applied:
test=> ROLLBACK;
NOTICE: there is no transaction in progress
ROLLBACK
Commit is unchanged:
test=> COMMIT;
WARNING: there is no transaction in progress
COMMIT
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +