Lists: | pgsql-hackerspgsql-patches |
---|
From: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Connection Pools and DISCARD ALL |
Date: | 2007-10-04 14:11:22 |
Message-ID: | 1191507082.4223.115.camel@ebony.site |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 14:29:00 |
Message-ID: | 20312.1191508140@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Simon Riggs <simon(at)2ndquadrant(dot)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
From: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 14:50:16 |
Message-ID: | 1191509416.4223.150.camel@ebony.site |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
On Thu, 2007-10-04 at 10:29 -0400, Tom Lane wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)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
From: | Neil Conway <neilc(at)samurai(dot)com> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 20:03:22 |
Message-ID: | 1191528202.6191.6.camel@dell.linuxdev.us.dell.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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
From: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
---|---|
To: | Neil Conway <neilc(at)samurai(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 20:14:51 |
Message-ID: | 1191528891.4223.225.camel@ebony.site |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Neil Conway <neilc(at)samurai(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 20:24:43 |
Message-ID: | 20071004202443.GD28896@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Neil Conway <neilc(at)samurai(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 20:27:06 |
Message-ID: | 47054C9A.2080005@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Simon Riggs <simon(at)2ndquadrant(dot)com>, Neil Conway <neilc(at)samurai(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Connection Pools and DISCARD ALL |
Date: | 2007-10-04 21:08:29 |
Message-ID: | 21198.1191532109@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Andrew Dunstan <andrew(at)dunslane(dot)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
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Neil Conway <neilc(at)samurai(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Connection Pools and DISCARD ALL |
Date: | 2007-11-09 02:50:45 |
Message-ID: | 200711090250.lA92ojX11076@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/pgpatches/abort | text/x-diff | 1.6 KB |
From: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Neil Conway <neilc(at)samurai(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Connection Pools and DISCARD ALL |
Date: | 2007-11-09 08:22:05 |
Message-ID: | 1194596525.4251.344.camel@ebony.site |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
On Thu, 2007-11-08 at 21:50 -0500, Bruce Momjian wrote:
> Tom Lane wrote:
> > Andrew Dunstan <andrew(at)dunslane(dot)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
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Neil Conway <neilc(at)samurai(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Connection Pools and DISCARD ALL |
Date: | 2007-11-10 14:37:38 |
Message-ID: | 200711101437.lAAEbcA19090@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Bruce Momjian wrote:
> Tom Lane wrote:
> > Andrew Dunstan <andrew(at)dunslane(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +