On Thu, Mar 19, 2015 at 07:54:02AM -0400, Robert Haas wrote: > On Wed, Mar 18, 2015 at 10:56 PM, Bruce Momjian <br...@momjian.us> wrote: > > I have researched this issue originally reported in June of 2014 and > > implemented a patch to ignore cancel while we are completing a commit. > > I am not clear if this is the proper place for this code, though a > > disable_timeout() call on the line above suggests I am close. :-) > > This would also disable cancel interrupts while running AFTER > triggers, which seems almost certain to be wrong. TBH, I'm not sure > why the existing HOLD_INTERRUPTS() in CommitTransaction() isn't > already preventing this problem. Did you investigate that at all?
Yes, the situation is complex, and was suggested by the original poster. The issue with CommitTransaction() is that it only _holds_ the signal --- it doesn't clear it. Now, since there are very few CHECK_FOR_INTERRUPTS() calls in the typical commit process flow, the signal is normally erased. However, if log_duration or log_min_duration_statement are set, they call ereport, which calls errfinish(), which has a call to CHECK_FOR_INTERRUPTS(). First attached patch is more surgical and clears a possible cancel request before we report the query duration in the logs --- this doesn't affect any after triggers that might include CHECK_FOR_INTERRUPTS() calls we want to honor. Another approach would be to have CommitTransaction() clear any pending cancel before it calls RESUME_INTERRUPTS(). The second attached patch takes that approach, and also works. -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c new file mode 100644 index 33720e8..9521c48 *** a/src/backend/tcop/postgres.c --- b/src/backend/tcop/postgres.c *************** exec_simple_query(const char *query_stri *** 1163,1168 **** --- 1163,1174 ---- NullCommand(dest); /* + * We have already committed, so clear any cancel requests + * that might be processed by the ereport() calls below. + */ + QueryCancelPending = false; + + /* * Emit duration logging if appropriate. */ switch (check_log_duration(msec_str, was_logged))
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c new file mode 100644 index 1495bb4..9b6da95 *** a/src/backend/access/transam/xact.c --- b/src/backend/access/transam/xact.c *************** CommitTransaction(void) *** 1958,1963 **** --- 1958,1969 ---- */ s->state = TRANS_DEFAULT; + /* + * We have already committed, so clear any cancel requests + * that might be processed later. + */ + QueryCancelPending = false; + RESUME_INTERRUPTS(); }
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
