Re: query_id, pg_stat_activity, extended query protocol

From: Andrei Lepikhov <lepihov(at)gmail(dot)com>
To: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, kaido vaikla <kaido(dot)vaikla(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: query_id, pg_stat_activity, extended query protocol
Date: 2024-04-23 04:42:41
Message-ID: d1d3629e-d1f2-404a-b4c8-3695c881a071@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 23/4/2024 11:16, Imseih (AWS), Sami wrote:
>> FWIW, I'd like to think that we could improve the situation, requiring
>> a mix of calling pgstat_report_query_id() while feeding on some query
>> IDs retrieved from CachedPlanSource->query_list. I have not in
>> details looked at how much could be achieved, TBH.
>
> I was dealing with this today and found this thread. I spent some time
> looking at possible solutions.
>
> In the flow of extended query protocol, the exec_parse_message
> reports the queryId, but subsequent calls to exec_bind_message
> and exec_execute_message reset the queryId when calling
> pgstat_report_activity(STATE_RUNNING,..) as you can see below.
>
> /*
> * If a new query is started, we reset the query identifier as it'll only
> * be known after parse analysis, to avoid reporting last query's
> * identifier.
> */
> if (state == STATE_RUNNING)
> beentry->st_query_id = UINT64CONST(0);
>
>
> So, I think the simple answer is something like the below.
> Inside exec_bind_message and exec_execute_message,
> the query_id should be reported after pg_report_activity.
>
> diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
> index 76f48b13d2..7ec2df91d5 100644
> --- a/src/backend/tcop/postgres.c
> +++ b/src/backend/tcop/postgres.c
> @@ -1678,6 +1678,7 @@ exec_bind_message(StringInfo input_message)
> debug_query_string = psrc->query_string;
>
> pgstat_report_activity(STATE_RUNNING, psrc->query_string);
> + pgstat_report_query_id(linitial_node(Query, psrc->query_list)->queryId, true);
>
> set_ps_display("BIND");
>
> @@ -2146,6 +2147,7 @@ exec_execute_message(const char *portal_name, long max_rows)
> debug_query_string = sourceText;
>
> pgstat_report_activity(STATE_RUNNING, sourceText);
> + pgstat_report_query_id(portal->queryDesc->plannedstmt->queryId, true);
>
> cmdtagname = GetCommandTagNameAndLen(portal->commandTag, &cmdtaglen);
>
>
> thoughts?
In exec_bind_message, how can you be sure that queryId exists in
query_list before the call of GetCachedPlan(), which will validate and
lock the plan? What if some OIDs were altered in the middle?

--
regards, Andrei Lepikhov

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2024-04-23 04:54:48 Internal error codes triggered by tests
Previous Message Imseih (AWS), Sami 2024-04-23 04:16:29 Re: query_id, pg_stat_activity, extended query protocol