Re: Display individual query in pg_stat_activity

On 8/17/20 7:49 AM, Drouvot, Bertrand wrote:
>
> Hi,
>
> On 8/6/20 12:24 PM, Magnus Hagander wrote:
>>
>> On Thu, Aug 6, 2020 at 12:17 PM Drouvot, Bertrand
>> <bdrouvot(at)amazon(dot)com <mailto:bdrouvot(at)amazon(dot)com>> wrote:
>>
>> Hi,
>>
>> On 7/27/20 4:57 PM, Dave Page wrote:
>>>
>>> Hi
>>>
>>> On Mon, Jul 27, 2020 at 3:40 PM Drouvot, Bertrand
>>> <bdrouvot(at)amazon(dot)com <mailto:bdrouvot(at)amazon(dot)com>> wrote:
>>>
>>> Hi hackers,
>>>
>>> I've attached a patch to display individual query in the
>>> pg_stat_activity query field when multiple SQL statements
>>> are currently displayed.
>>>
>>> _Motivation:_
>>>
>>> When multiple statements are displayed then we don’t know
>>> which one is currently running.
>>>
>>>
>>> I'm not sure I'd want that to happen, as it could make it much
>>> harder to track the activity back to a query in the application
>>> layer or server logs.
>>>
>>> Perhaps a separate field could be added for the current
>>> statement, or a value to indicate what the current statement
>>> number in the query is?
>>
>> Thanks for he feedback.
>>
>> I like the idea of adding extra information without changing the
>> current behavior.
>>
>> A value to indicate what the current statement number is, would
>> need parsing the query field by the user to get the individual
>> statement.
>>
>> I think the separate field makes sense (though it come with an
>> extra memory price) as it will not change the existing behavior
>> and would just provide extra information (without any extra
>> parsing needed for the user).
>>
>>
>>
>> Idle though without having considered it too much -- you might reduce
>> the memory overhead by just storing a start/end offset into the
>> combined query string instead of a copy of the query.
>
> Good point, thanks for the feedback.
>
> The new attached patch is making use of stmt_len and stmt_location
> (instead of a copy of the query).
>
>> That way the cost would only be paid when doing the reading of
>> pg_stat_activity (by extracting the piece of the string), which I'd
>> argue is done orders of magnitude fewer times than the query changes
>> at least on busy systems.
>
> The individual query extraction (making use of stmt_len and
> stmt_location) has been moved to pg_stat_get_activity() in the new
> attached patch (as opposed to pgstat_report_activity() in the previous
> patch version).
>
>> Care would have to be taken for the case of the current executing
>> query actually being entirely past the end of the query string buffer
>> of course, but I don't think that's too hard to define a useful
>> behaviour for. (The user interface would stay the same, showing the
>> actual string and thus not requiring the user to do any parsing)
>
> As a proposal the new attached patch does not display the individual
> query if length + location is greater than
> pgstat_track_activity_query_size (anyway it could not, as the query
> field that might contain multiple statements is already <=
> pgstat_track_activity_query_size in pg_stat_get_activity()).
>
> Bertrand
>
Attaching a new version as the previous one was not passing the Patch
Tester anymore.

Bertrand

>> --
>>  Magnus Hagander
>>  Me: https://www.hagander.net/ <http://www.hagander.net/>
>>  Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>