Re: Error code for "terminating connection due to conflict with recovery" - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: Error code for "terminating connection due to conflict with recovery" |
Date | |
Msg-id | AANLkTimDNebaoJB-5S0zk1H2RjiB3o7R8BOqL5--XCDF@mail.gmail.com Whole thread Raw |
In response to | Re: Error code for "terminating connection due to conflict with recovery" (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: Error code for "terminating connection due to
conflict with recovery"
(Tatsuo Ishii <ishii@postgresql.org>)
|
List | pgsql-hackers |
On Fri, Jan 14, 2011 at 1:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Jan 14, 2011 at 12:29 PM, Simon Riggs <simon@2ndquadrant.com> wrote: >> This whole thing is confused. No change is appropriate here at all. >> >> We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for >> recovery conflicts. >> >> We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable, >> which occurs if someone drops the database that the user was connected >> to when they get kicked off. That code exists specifically to inform the >> user that they *cannot* reconnect. So pgpool should not be trying to >> trap that error and reconnect. > > CheckRecoveryConflictDeadlock() uses ERRCODE_QUERY_CANCELLED. > ProcessInterrupts() sometimes uses ERRCODE_T_R_SERIALIZATION_FAILURE > and sometimes uses ERRCODE_ADMIN_SHUTDOWN. It seems to me that it > wouldn't be a bad thing to be a bit more consistent, and perhaps to > have dedicated error codes for recovery conflicts. This bit strikes > me as particularly strange: > > else if (RecoveryConflictPending && RecoveryConflictRetryable) > { > pgstat_report_recovery_conflict(RecoveryConflictReason); > ereport(FATAL, > > (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), > errmsg("terminating connection due to > conflict with recovery"), > errdetail_recovery_conflict())); > } > else if (RecoveryConflictPending) > { > pgstat_report_recovery_conflict(RecoveryConflictReason); > ereport(FATAL, > (errcode(ERRCODE_ADMIN_SHUTDOWN), > errmsg("terminating connection due to > conflict with recovery"), > errdetail_recovery_conflict())); > } > > That's the same error message at the same severity level with two > different SQLSTATEs depending on RecoveryConflictRetryable. Seems a > bit cryptic. So what we do want to do about this? I'm pretty well convinced that we should NOT be issuing ERRCODE_ADMIN_SHUTDOWN for a recovery conflict, but that could be fixed by a trivial simplification of the code posted above, without introducing any new error code. I'd also be in favor of changing the one that uses ERRCODE_QUERY_CANCELLED to use ERRCODE_T_R_SERIALIZATION_FAILURE, as the former might be taken to imply active user intervention, and for consistency. It's no longer clear to me that we actually need a new error code for this - using the same one everywhere seems like it might be sufficient, unless someone wants to make an argument why it isn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
pgsql-hackers by date: