On Fri, Jan 21, 2011 at 7:48 AM, Simon Riggs <si...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
