On 10/17/24 06:19, Fujii Masao wrote:
>
>
> On 2024/10/17 1:59, Fujii Masao wrote:
>>
>>
>> On 2024/07/05 7:03, Andres Freund wrote:
>>> Calls to pgstat_report_activity() aren't exactly free. This
>>> substantially
>>> increases the number of calls to it for common workloads. There got
>>> to be a
>>> more targeted way of dealing with this.
>>
>> Yes. So, as an alternative approach, how about setting xact_start and
>> query_start to NULL whenever the state is idle, since non-NULL values
>> aren't expected in that case? This would prevent the weired
>> pg_stat_activity
>> entries I mentioned earlier, with minimal performance impact.
>
> Patch attached.
>
I was going through the CF looking for long-running patches that might
need more attention ... I was not expecting a thread started in 2014.
It seems a bit strange to "fix" this while querying the data, which I
think is what the patch is doing. Wouldn't it be better to make sure we
don't have such "incorrect" data in the first place?
I mean, couldn't we simply reset the timestamp whenever the state gets
set to IDLE? Of maybe if there are legitimate cases where we can't do
that, maybe we could add a flag to pgstat_report_activity() to trigger
this timestamp reset?
Alternatively, why don't we simply add pgstat_report_xact_timestamp(0)
to the two places that can cause this, after CommitTransactionCommand()?
FWIW it seems a bit confusing that CommitTransactionCommand() doesn't
already do this. If StartTransactionCommand() can set the start
timestamp, wouldn't it be reasonable for CommitTransactionCommand() to
reset it?
Of course, it's entirely possible I miss something and there are reasons
why none of this would work.
regards
--
Tomas Vondra
