On 06.12.2019 4:57, Michael Paquier wrote:
> On Thu, Dec 05, 2019 at 12:23:40PM +0300, Konstantin Knizhnik wrote:
>> Concerning keeping PGPROC size as small as possible, I agree that it is
>> reasonable argument.
>> But even now it is very large (816 bytes) and adding extra 8 bytes will
>> increase it on less than 1%.
> It does not mean that we should add all kind of things to PGPROC as
> that's a structure sensitive enough already. By the way, why do you
> assume that 8-byte reads are always safe and atomic in the patch?
I never assumed it - in the previous mail I wrote:
Certainly update of 64 bit field is not atomic at 32-but architectures.
>> Right now pg_stat_activity also accessing PGPROC to obtain wait event
>> information and also not taking any locks.
>> So it can wrongly report backend status. But I never heard that somebody
>> complains about it.
> Please see pgstat.h, close to pgstat_report_wait_start().
Sorry, I do not understand what should I look for?
Do you mean this comment:
/*
* Since this is a four-byte field which is always read and written as
* four-bytes, updates are atomic.
*/
Yes, I already have noticed that as far as walWritten is 64-bit, its
update is not atomic at 32-bit platforms and so it is possible to see
sometimes incorrect values.
So monotone observe of walWritten can be violated. From my point of view
it is not so critical to enforce update of this fields under lock or
accumulating result in local variable with later write it to backend
status at commit time as Kyotaro proposed. Monitoring of WAL activity is
especially interested for long living transactions and from my point of
view it is much more
important to be able to see up-to-date but may be not always correct
information then do not see any information at all before commit.
Please also take in account the percent of 32-bit Postgres installations
and probability of observing non-atomic update of 64-bit walWritten
field (I think that you will have no chances to see it even if you will
run Postgres for a years).
But what I mean by "wrongly report backend wait event status" is that
pg_stat_activity may report wait event status for wrong backend.
I.e. if backend is already terminated and its PGPROC entry is reused by
some other backend, than you can see incorrect wait event information:
backend with such PID actually never sleep on this event.
In my reply
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
