Skip to content

Commit

Permalink
PG-261: Fix regression tests on distribution packages.
Browse files Browse the repository at this point in the history
The code added in pgss_store() to handle an assertion failure when
GetUserId() was being called introduced a problem with regression tests
in some builds, specifically our PG11 and PG12 distributions for Ubuntu.

The problem was detected when calling some json functions that would
trigger parallel workers, to solve the problem now we check if our hooks
are being called by parallel workers, in this case we can avoid doing
work, it also fixes the issue mentioned above as we won't call
GetUserId() anymore in an invalid context.
  • Loading branch information
darkfronza committed Oct 15, 2021
1 parent 52ea543 commit bf8c93b
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions pg_stat_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
*
*-------------------------------------------------------------------------
*/

#include "postgres.h"
#include "access/parallel.h"
#include <regex.h>
#ifdef BENCHMARK
#include <time.h> /* clock() */
Expand Down Expand Up @@ -367,6 +369,9 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, JumbleState *jstate)
if (!IsSystemInitialized())
return;

if (IsParallelWorker())
return;

/*
* Clear queryId for prepared statements related utility, as those will
* inherit from the underlying statement's one (except DEALLOCATE which is
Expand Down Expand Up @@ -423,6 +428,9 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query)
if (!IsSystemInitialized())
return;

if (IsParallelWorker())
return;

/*
* Utility statements get queryId zero. We do this even in cases where
* the statement contains an optimizable statement for which a queryId
Expand Down Expand Up @@ -485,6 +493,9 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
else
standard_ExecutorStart(queryDesc, eflags);

if (IsParallelWorker())
return;

/*
* If query has queryId zero, don't track it. This prevents double
* counting of optimizable statements that are directly contained in
Expand Down Expand Up @@ -655,7 +666,7 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
MemoryContextSwitchTo(mct);
}

if (queryId != UINT64CONST(0) && queryDesc->totaltime)
if (queryId != UINT64CONST(0) && queryDesc->totaltime && !IsParallelWorker())
{
/*
* Make sure stats accumulation is done. (Note: it's okay if several
Expand Down Expand Up @@ -770,7 +781,7 @@ pgss_planner_hook(Query *parse, const char *query_string, int cursorOptions, Par
{
PlannedStmt *result;

if (PGSM_TRACK_PLANNING && query_string && parse->queryId != UINT64CONST(0))
if (PGSM_TRACK_PLANNING && query_string && parse->queryId != UINT64CONST(0) && !IsParallelWorker())
{
PlanInfo plan_info;
instr_time start;
Expand Down Expand Up @@ -942,7 +953,7 @@ static void pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
if (PGSM_TRACK_UTILITY &&
!IsA(parsetree, ExecuteStmt) &&
!IsA(parsetree, PrepareStmt) &&
!IsA(parsetree, DeallocateStmt))
!IsA(parsetree, DeallocateStmt) && !IsParallelWorker())
{
instr_time start;
instr_time duration;
Expand Down Expand Up @@ -1453,7 +1464,6 @@ pgss_store(uint64 queryid,
uint64 bucketid;
uint64 prev_bucket_id;
uint64 userid;
int con;
uint64 planid;
uint64 appid;
char comments[512] = "";
Expand All @@ -1468,10 +1478,7 @@ pgss_store(uint64 queryid,
return;

Assert(query != NULL);
if (kind == PGSS_ERROR)
GetUserIdAndSecContext((unsigned int *)&userid, &con);
else
userid = GetUserId();
userid = GetUserId();

application_name_len = pg_get_application_name(application_name);
planid = plan_info ? plan_info->planid: 0;
Expand Down Expand Up @@ -3255,6 +3262,9 @@ pgsm_emit_log_hook(ErrorData *edata)
if (!IsSystemInitialized() || edata == NULL)
goto exit;

if (IsParallelWorker())
return;

if ((edata->elevel == ERROR || edata->elevel == WARNING || edata->elevel == INFO || edata->elevel == DEBUG1))
{
uint64 queryid = 0;
Expand Down

0 comments on commit bf8c93b

Please sign in to comment.