On Fri, 2011-01-14 at 12:04 -0500, Tom Lane wrote: > Tatsuo Ishii <is...@postgresql.org> writes: > >> Review: > >> The only possible point of concern I see here is the naming of the C > >> identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, > >> with T_R standing for transaction rollback. It's not obvious to me > >> that that convention has any real value, but perhaps we ought to > >> follow it here for the sake of consistency? > > > Yeah. Actually at first I used "T_R" convention. After a few seconds > > thought, I realized that "T_R" is not appropreate by the same reason > > you feel. Possible other argument might be "Terminating connection > > always involves transaction rollback. So using T_R is ok". I'm not > > sure this argument is reasonable enough though. > > This is not only a matter of some macro name or other. According to the > SQL standard, class 40 itself is defined as "transaction rollback". > If the error condition can't reasonably be regarded as a subcase of > that, you're making a bad choice of SQLSTATE code.
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. So the whole basis for the existence of this patch is moot. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers