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

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company