Skip site navigation (1) Skip section navigation (2)

Re: Error code for "terminating connection due to conflict with recovery"

From: Tatsuo Ishii <ishii(at)postgresql(dot)org>
To: robertmhaas(at)gmail(dot)com
Cc: simon(at)2ndquadrant(dot)com, fgp(at)phlo(dot)org, ishii(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Error code for "terminating connection due to conflict with recovery"
Date: 2011-01-31 00:02:40
Message-ID: 20110131.090240.311359837544450614.t-ishii@sraoss.co.jp (view raw or flat)
Thread:
Lists: pgsql-hackers
Robert,

> On Fri, Jan 21, 2011 at 8:28 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Jan 21, 2011 at 7:48 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>> Ah, thanks Florian. Now I understand. There are two related issues here.
>>>
>>> 1. The discussion around ERRCODE_ADMIN_SHUTDOWN is incorrect and the
>>> specific patch should be rejected as is. No changes are required in
>>> ProcessInterrupts(), nor new errcodes.
>>
>> Can you please justify that statement instead of simply asserting it?
>> Tatsuo-san and I both seem to agree that it looks wrong.
>> ERRCODE_ADMIN_SHUTDOWN is in class 57, operator intervention, and it's
>> used elsewhere when a SIGTERM is received and the database is shutting
>> down.  That's a world away from what's actually happening here.
>> Wanting to have a different error code for this type of failure may
>> make sense, but that doesn't mean that this is the right one.
>>
>>> 2. Robert is correct that CheckRecoveryConflictDeadlock() returns
>>> ERRCODE_QUERY_CANCELED. Thanks to Florian for noting that we had
>>> switched away from the original discussion onto another part of the
>>> code, which confused me. I agree the use of ERRCODE_QUERY_CANCELED is a
>>> mistake; CheckRecoveryConflictDeadlock() should return
>>> ERRCODE_T_R_DEADLOCK_DETECTED. This was an omission from my commit of 12
>>> May 2010.
>>
>> This part sounds good.
>>
>>> This should be backpatched to 9.0.
>>
>> Hmm, I don't necessarily agree.  The standard for changing behavior in
>> an existing release is fairly high.
> 
> It seems like we have consensus to change
> CheckRecoveryConflictDetected() to return
> ERRCODE_T_R_DEADLOCK_DETECTED in 9.1, but not on whether to also
> change that in 9.0 (votes: Robert - for, Simon - against)

Please note that I'm with you.

> and arguably
> not on whether to change the case that returns ERROR_ADMIN_SHUTDOWN
> for a recovery conflict (votes: Robert, Tatsuo-san - for, Simon -
> against).
> 
> Anyone else want to weigh in?
> 
> -- 
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2011-01-31 01:11:47
Subject: Re: Error code for "terminating connection due to conflict with recovery"
Previous:From: Tom LaneDate: 2011-01-30 23:27:57
Subject: Re: autogenerating error code lists (was Re: [COMMITTERS] pgsql: Add foreign data wrapper error code values for SQL/MED.)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group