public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andrei Lepikhov <[email protected]>
To: Imseih (AWS), Sami <[email protected]>
To: Michael Paquier <[email protected]>
To: kaido vaikla <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: query_id, pg_stat_activity, extended query protocol
Date: Tue, 23 Apr 2024 11:42:41 +0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+427g8DiW3aZ6pOpVgkPbqK97ouBdf18VLiHFesea2jUk3XoQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>

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

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: query_id, pg_stat_activity, extended query protocol
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox