From 9dca62e9ff3db2f482c829884b24a425692a5a18 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 26 Aug 2016 10:07:28 -0400 Subject: [PATCH] Fix logic for adding "parallel worker" context line to worker errors. The previous coding here was capable of adding a "parallel worker" context line to errors that were not, in fact, returned from a parallel worker. Instead of using an errcontext callback to add that annotation, just paste it onto the message by hand; this looks uglier but is more reliable. Discussion: <19757.1472151987@sss.pgh.pa.us> --- src/backend/access/transam/parallel.c | 54 +++++++++++++-------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index a47eba647b..ec6e1c5e6d 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -108,7 +108,6 @@ static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list); /* Private functions. */ static void HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg); -static void ParallelErrorContext(void *arg); static void ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc); static void ParallelWorkerMain(Datum main_arg); static void WaitForParallelWorkersToExit(ParallelContext *pcxt); @@ -788,30 +787,43 @@ HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg) case 'N': /* NoticeResponse */ { ErrorData edata; - ErrorContextCallback errctx; ErrorContextCallback *save_error_context_stack; - /* - * Rethrow the error using the error context callbacks that - * were in effect when the context was created, not the - * current ones. - */ - save_error_context_stack = error_context_stack; - errctx.callback = ParallelErrorContext; - errctx.arg = NULL; - errctx.previous = pcxt->error_context_stack; - error_context_stack = &errctx; - /* Parse ErrorResponse or NoticeResponse. */ pq_parse_errornotice(msg, &edata); /* Death of a worker isn't enough justification for suicide. */ edata.elevel = Min(edata.elevel, ERROR); - /* Rethrow error or notice. */ + /* + * If desired, add a context line to show that this is a + * message propagated from a parallel worker. Otherwise, it + * can sometimes be confusing to understand what actually + * happened. (We don't do this in FORCE_PARALLEL_REGRESS mode + * because it causes test-result instability depending on + * whether a parallel worker is actually used or not.) + */ + if (force_parallel_mode != FORCE_PARALLEL_REGRESS) + { + if (edata.context) + edata.context = psprintf("%s\n%s", edata.context, + _("parallel worker")); + else + edata.context = pstrdup(_("parallel worker")); + } + + /* + * Context beyond that should use the error context callbacks + * that were in effect when the ParallelContext was created, + * not the current ones. + */ + save_error_context_stack = error_context_stack; + error_context_stack = pcxt->error_context_stack; + + /* Rethrow error or print notice. */ ThrowErrorData(&edata); - /* Restore previous context. */ + /* Not an error, so restore previous context stack. */ error_context_stack = save_error_context_stack; break; @@ -1112,18 +1124,6 @@ ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc) entrypt(seg, toc); } -/* - * Give the user a hint that this is a message propagated from a parallel - * worker. Otherwise, it can sometimes be confusing to understand what - * actually happened. - */ -static void -ParallelErrorContext(void *arg) -{ - if (force_parallel_mode != FORCE_PARALLEL_REGRESS) - errcontext("parallel worker"); -} - /* * Update shared memory with the ending location of the last WAL record we * wrote, if it's greater than the value already stored there.