On Wed, Dec 30, 2015 at 10:18:35AM -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2015-12-29 11:07:26 -0500, Tom Lane wrote: > >> In passing, the patch gets rid of a vestigial CHECK_FOR_INTERRUPTS() > >> call; it was added by e710b65c and IMO should have been removed again > >> by 6647248e. There's certainly no very good reason to have one right > >> at that spot anymore. > > > Why? Doesn't seem like the worst place for an explicit interrupt check? > > I think we don't really have a problem with too many such checks... We > > surely could move it, but I don't really see how it's related to the > > topic at hand nor do I think it's really worth pondering about > > extensively.
Agreed. > The only reason there was one there at all was that e710b65c added > code like this: > > + /* > + * Disable immediate interrupts while doing database access. (Note > + * we don't bother to turn this back on if we hit one of the failure > + * conditions, since we can expect we'll just exit right away anyway.) > + */ > + ImmediateInterruptOK = false; > > ... some catalog access here ... > > + /* Re-enable immediate response to SIGTERM/SIGINT/timeout interrupts */ > + ImmediateInterruptOK = true; > + /* And don't forget to detect one that already arrived */ > + CHECK_FOR_INTERRUPTS(); > > In 6647248e you got rid of nine of these ten lines, leaving something > that's both pointless and undocumented. There are more than enough > CHECK_FOR_INTERRUPTS calls already in the auth code; there's not a > reason to expend code space on one here. (If MD5 ran long enough to > be worth interrupting, there would be an argument for a check inside > its hashing loop, but that still wouldn't be this check.) I see no general benefit from being parsimonious with CHECK_FOR_INTERRUPTS calls or documenting them. As you explain, it's probably fine to remove the two calls that commit e710b65 had added. However, the sole connection to $SUBJECT is one of those two calls sharing a screenful with lines $SUBJECT changed. The removal, if worthwhile, is worth a freestanding patch. Squashing the changes makes both topics harder to review. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
