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

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