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

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) 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