This is an automated email from the ASF dual-hosted git repository. avamingli pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 21af1e9d6205f7832d9e856771ec14d31a55c130 Author: 1mmortal <[email protected]> AuthorDate: Thu Apr 20 16:53:18 2023 +0800 Fix InterruptHoldoffCount not being reset issue (#15279) This commit addresses issue #15278, where the InterruptHoldoffCount value was not always reset to zero in certain abnormal scenarios. This issue had the potential to impact the proper functioning of the ProcessInterrupts process. We should use ThrowErrorData instead of ReThrowError to throw a constructed ErrorData from QE. --- src/backend/cdb/cdbcopy.c | 2 +- src/backend/cdb/cdbtm.c | 2 +- src/backend/cdb/dispatcher/cdbdisp.c | 2 +- src/backend/cdb/dispatcher/cdbdisp_query.c | 8 ++++---- src/backend/cdb/endpoint/cdbendpointutils.c | 2 +- src/backend/commands/explain.c | 18 ++++++++++++++++++ src/backend/executor/execUtils.c | 2 +- src/backend/executor/nodeSubplan.c | 2 +- src/backend/utils/error/elog.c | 20 -------------------- 9 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/backend/cdb/cdbcopy.c b/src/backend/cdb/cdbcopy.c index 88fd50e5c6..94a95e2337 100644 --- a/src/backend/cdb/cdbcopy.c +++ b/src/backend/cdb/cdbcopy.c @@ -726,7 +726,7 @@ cdbCopyEndInternal(CdbCopy *c, char *abort_msg, if (first_error) { FlushErrorState(); - ReThrowError(first_error); + ThrowErrorData(first_error); } /* errors that occurred in the COPY itself */ diff --git a/src/backend/cdb/cdbtm.c b/src/backend/cdb/cdbtm.c index 6de5509315..e22c5c27a0 100644 --- a/src/backend/cdb/cdbtm.c +++ b/src/backend/cdb/cdbtm.c @@ -1273,7 +1273,7 @@ doDispatchDtxProtocolCommand(DtxProtocolCommand dtxProtocolCommand, else { FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } return false; } diff --git a/src/backend/cdb/dispatcher/cdbdisp.c b/src/backend/cdb/dispatcher/cdbdisp.c index eb87e8b339..e319fdc65e 100644 --- a/src/backend/cdb/dispatcher/cdbdisp.c +++ b/src/backend/cdb/dispatcher/cdbdisp.c @@ -280,7 +280,7 @@ CdbDispatchHandleError(struct CdbDispatcherState *ds) if (error != NULL) { FlushErrorState(); - ReThrowError(error); + ThrowErrorData(error); } } PG_CATCH(); diff --git a/src/backend/cdb/dispatcher/cdbdisp_query.c b/src/backend/cdb/dispatcher/cdbdisp_query.c index c241404eb4..82d7fbe093 100644 --- a/src/backend/cdb/dispatcher/cdbdisp_query.c +++ b/src/backend/cdb/dispatcher/cdbdisp_query.c @@ -358,7 +358,7 @@ CdbDispatchSetCommand(const char *strCommand, bool cancelOnError) { FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } cdbdisp_destroyDispatcherState(ds); @@ -514,7 +514,7 @@ cdbdisp_dispatchCommandInternal(DispatchCommandQueryParms *pQueryParms, if (qeError) { FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } /* collect pgstat from QEs for current transaction level */ @@ -1217,7 +1217,7 @@ cdbdisp_dispatchX(QueryDesc* queryDesc, if (qeError) { FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } /* @@ -1368,7 +1368,7 @@ CdbDispatchCopyStart(struct CdbCopy *cdbCopy, Node *stmt, int flags) if (!cdbdisp_getDispatchResults(ds, &error)) { FlushErrorState(); - ReThrowError(error); + ThrowErrorData(error); } /* diff --git a/src/backend/cdb/endpoint/cdbendpointutils.c b/src/backend/cdb/endpoint/cdbendpointutils.c index 8731836a46..32250bc8f8 100644 --- a/src/backend/cdb/endpoint/cdbendpointutils.c +++ b/src/backend/cdb/endpoint/cdbendpointutils.c @@ -158,7 +158,7 @@ check_parallel_retrieve_cursor_errors(EState *estate) estate->dispatcherState = NULL; cdbdisp_cancelDispatch(ds); FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } } diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 33bc59ad91..8ba54dfa28 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -724,6 +724,24 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, /* run the plan */ ExecutorRun(queryDesc, dir, 0L, true); + /* Wait for completion of all qExec processes. */ + if (queryDesc->estate->dispatcherState && queryDesc->estate->dispatcherState->primaryResults) + { + cdbdisp_checkDispatchResult(queryDesc->estate->dispatcherState, DISPATCH_WAIT_NONE); + /* + * If some QE throw errors, we might not receive stats from QEs, + * In ExecutorEnd we will reThrow QE's error, In this situation, + * there is no need to execute ExplainPrintPlan. reThrow error in advance. + */ + ErrorData *qeError = NULL; + cdbdisp_getDispatchResults(queryDesc->estate->dispatcherState, &qeError); + if (qeError) + { + FlushErrorState(); + ThrowErrorData(qeError); + } + } + /* run cleanup too */ ExecutorFinish(queryDesc); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 952c5e8737..3a47f088d2 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -2054,7 +2054,7 @@ void mppExecutorFinishup(QueryDesc *queryDesc) { estate->dispatcherState = NULL; FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } if (ProcessDispatchResult_hook) diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 43adfd129c..5d63b939c7 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -1402,7 +1402,7 @@ PG_TRY(); { queryDesc->estate->dispatcherState = NULL; FlushErrorState(); - ReThrowError(qeError); + ThrowErrorData(qeError); } /* collect pgstat from QEs for current transaction level */ diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index cdfef978bc..e49fd4f9c7 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -838,8 +838,6 @@ errfinish_and_return(const char *filename, int lineno, const char *funcname) { ErrorData *edata = &errordata[errordata_stack_depth]; ErrorData *edata_copy; - ErrorContextCallback *econtext; - MemoryContext oldcontext; int saved_errno; /*CDB*/ recursion_depth++; @@ -861,24 +859,6 @@ errfinish_and_return(const char *filename, int lineno, const char *funcname) edata->lineno = lineno; edata->funcname = funcname; - /* - * Do processing in ErrorContext, which we hope has enough reserved space - * to report an error. - */ - oldcontext = MemoryContextSwitchTo(ErrorContext); - - /* - * Call any context callback functions. Errors occurring in callback - * functions will be treated as recursive errors --- this ensures we will - * avoid infinite recursion (see errstart). - */ - for (econtext = error_context_stack; - econtext != NULL; - econtext = econtext->previous) - (*econtext->callback) (econtext->arg); - - MemoryContextSwitchTo(oldcontext); - edata_copy = CopyErrorData(); /* Now free up subsidiary data attached to stack entry, and release it */ --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
