From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, "bharath(dot)rupireddyforpostgres(at)gmail(dot)com" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Subject: | Re: Show WAL write and fsync stats in pg_stat_io |
Date: | 2023-12-31 00:58:33 |
Message-ID: | ZZC8uR1JEn1_g_Ij@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Dec 26, 2023 at 03:35:52PM +0300, Nazir Bilal Yavuz wrote:
> On Tue, 26 Dec 2023 at 13:10, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> I am not sure while the whole point of the exercise is to have all the
>> I/O related data in a single view. Something that I've also found a
>> bit disturbing yesterday while looking at your patch is the fact that
>> the operation size is guessed from the context and object type when
>> querying the view because now everything is tied to BLCKSZ. This
>> patch extends it with two more operation sizes, and there are even
>> cases where it may be a variable. Could it be a better option to
>> extend pgstat_count_io_op_time() so as callers can themselves give the
>> size of the operation?
>
> Do you mean removing the op_bytes column and tracking the number of
> bytes in reads, writes, and extends? If so, that makes sense to me but
> I don't want to remove the number of operations; I believe that has a
> value too. We can extend the pgstat_count_io_op_time() so it can both
> track the number of bytes and the number of operations.
Apologies if my previous wording sounded confusing. The idea I had in
mind was to keep op_bytes in pg_stat_io, and extend it so as a value
of NULL (or 0, or -1) is a synonym as "writes", "extends" and "reads"
as a number of bytes.
> Also, it is not directly related to this patch but vectored IO [1] is
> coming soon; so the number of operations could be wrong since vectored
> IO could merge a couple of operations.
Hmm. I have not checked this patch series so I cannot say for sure,
but we'd likely just want to track the number of bytes if a single
operation has a non-equal size rather than registering in pg_stat_io N
rows with different op_bytes, no? I am looping in Thomas Munro in CC
for comments.
>> The whole patch is kind of itself complicated enough, so I'd be OK to
>> discard the case of the WAL receiver for now. Now, if we do so, the
>> code stack of pgstat_io.c should handle WAL receivers as something
>> entirely disabled until all the known issues are solved. There is
>> still a lot of value in tracking WAL data associated to the WAL
>> writer, normal backends and WAL senders.
>
> Why can't we add comments and leave it as it is? Is it because this
> could cause misunderstandings?
>
> If we want to entirely disable it, we can add
>
> if (MyBackendType == B_WAL_RECEIVER && io_object == IOOBJECT_WAL)
> return;
>
> to the top of the pgstat_count_io_op_time() since all IOOBJECT_WAL
> calls are done by this function, then we can disable it at
> pgstat_tracks_io_bktype().
Yeah, a limitation like that may be acceptable for now. Tracking the
WAL writer and WAL sender activities can be relevant in a lot of cases
even if we don't have the full picture for the WAL receiver yet.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2023-12-31 01:08:41 | Re: pg_class.reltuples of brin indexes |
Previous Message | Michael Paquier | 2023-12-31 00:49:13 | Re: Commitfest 2024-01 starting in 3 days! |