On Wed, Jun 18, 2014 at 8:01 PM, Josh Berkus [via PostgreSQL] <[hidden email]> wrote:
On 06/18/2014 04:54 PM, Marko Tiikkaja wrote: > On 2014-06-19 1:46 AM, Josh Berkus wrote: >> Robert's right, not killing the "BEGIN;" only transactions is liable to >> result in user confusion unless we label those sessions differently in >> pg_stat_activity. > > Wouldn't they be labeled differently already? i.e. the last query would > be "BEGIN". Unless your app tries to unsuccessfully use nested > transactions, you would know why it hasn't been killed.
That's pretty darned obscure for a casual user. *you* would know, and *I* would know, but 99.5% of our users would be very confused.
Plus, if a session which has only issued a "BEGIN;" doesn't have a snapshot and isn't holding any locks, then I'd argue we shouldn't lable it IIT in the first place because it's not doing any of the bad stuff we want to resolve by killing IITs. Effectively, it's just "idle".
+1
Since the BEGIN is not meaningfully interpreted until the first subsequent command (SET excepting apparently - are there others?) is issued no transaction has begun at this point so "idle" and not "IIT" should be the reported state on pg_stat_activity.
Though I can understand some level of surprise if someone sees "idle" with a "BEGIN" (or SET TIMEZONE) as the last executed command - so maybe "idle before transaction" instead of "idle in transaction" - which hopefully will not be assumed to be controlled by the "idle_in_transaction_timeout" GUC. I presume that we have some way to distinguish this particular case and can report it accordingly. Even if that state endures after a SET TIMEZONE or similar session configuration command explaining that all those are "pre-transaction" shouldn't be too hard - though as a third option "idle initialized transaction" could be the state indicator.
Depending on how "idle in transaction (aborted)" gets resolved we could also consider "idle in transaction (initializing)" - but since the former, IMO, should be dropped (and thus matches the "in" specification of the GUC) naming the later - which should not be dropped (I'll go with the stated opinion here for now) - with "in" is not advisable.
The current behavior is transparent to the casual user so sticking with "idle" does seem like the best choice to maintain the undocumented nature of the behavior.
