Hi,
On Wed, Jan 29, 2025 at 02:57:21PM +0300, Nazir Bilal Yavuz wrote:
> I agree with you but it was discussed before in this thread [2]. It
> was decided to use both track_wal_io_timing and track_io_timing
> because of the overhead that track_wal_io_timing creates but we can
> still re-discuss it. Do you think that this discussion needs its own
> thread?
>
> If we continue to discuss it in this thread, I am in favor of removing
> track_wal_io_timing and using track_io_timing for all types of I/Os.
> Like you said, this cross-dependency makes things more complex than
> they used to be. Downside of removing track_wal_io_timing is affecting
> people who:
>
> 1- Want to track timings of only WAL I/Os.
> 2- Want to track timings of all IOs except WAL I/Os.
>
> I think the first group is more important than the second because
> track_io_timing already creates overhead.
I'm -1 of removing track_wal_io_timing. I think that this code path is very
sensible to performance to not add extra overhead when not necessary asked for.
I think that's the main reason why ff99918c625 added this new GUC (looking at
the commit message). I'd feel more comfortable if we keep it.
That said, in:
+static bool
+pgstat_should_track_io_time(IOObject io_object)
+{
+ if (io_object == IOOBJECT_WAL)
+ return track_wal_io_timing;
+
+ return track_io_timing;
+}
I think it would make sense to return "track_io_timing && track_wal_io_timing"
for the IOOBJECT_WAL case. That way it maintains track_io_timing as the master
switch for all I/O timing in pg_stat_io.
> One additional thing is that I think track_io_timing is a general
> word. When it exists, I do not expect there to be another GUC like
> track_wal_io_timing to track WAL I/Os' timings.
That's true but OTOH track_wal_io_timing is already there since years (it's not
like we are adding it).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
