Re: About to add WAL write/fsync statistics to pg_stat_wal view

On 2020-12-08 16:45, Li Japin wrote:
> Hi,
>
>> On Dec 8, 2020, at 1:06 PM, Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
>> wrote:
>>
>> Hi,
>>
>> I propose to add wal write/fsync statistics to pg_stat_wal view.
>> It's useful not only for developing/improving source code related to
>> WAL
>> but also for users to detect workload changes, HW failure, and so on.
>>
>> I introduce "track_wal_io_timing" parameter and provide the following
>> information on pg_stat_wal view.
>> I separate the parameter from "track_io_timing" to
>> "track_wal_io_timing"
>> because IIUC, WAL I/O activity may have a greater impact on query
>> performance than database I/O activity.
>>
>> ```
>> postgres=# SELECT wal_write, wal_write_time, wal_sync, wal_sync_time
>> FROM pg_stat_wal;
>> -[ RECORD 1 ]--+----
>> wal_write | 650 # Total number of times WAL data was written to
>> the disk
>>
>> wal_write_time | 43 # Total amount of time that has been spent in
>> the portion of WAL data was written to disk
>> # if track-wal-io-timing is enabled, otherwise
>> zero
>>
>> wal_sync | 78 # Total number of times WAL data was synced to
>> the disk
>>
>> wal_sync_time | 104 # Total amount of time that has been spent in
>> the portion of WAL data was synced to disk
>> # if track-wal-io-timing is enabled, otherwise
>> zero
>> ```
>>
>> What do you think?
>> Please let me know your comments.
>>
>> Regards
>> --
>> Masahiro Ikeda
>> NTT DATA
>> CORPORATION<0001_add_wal_io_activity_to_the_pg_stat_wal.patch>
>
> There is a no previous prototype warning for ‘fsyncMethodCalled’, and
> it now only used in xlog.c,
> should we declare with static? And this function wants a boolean as a
> return, should we use
> true/false other than 0/1?
>
> +/*
> + * Check if fsync mothod is called.
> + */
> +bool
> +fsyncMethodCalled()
> +{
> + if (!enableFsync)
> + return 0;
> +
> + switch (sync_method)
> + {
> + case SYNC_METHOD_FSYNC:
> + case SYNC_METHOD_FSYNC_WRITETHROUGH:
> + case SYNC_METHOD_FDATASYNC:
> + return 1;
> + default:
> + /* others don't have a specific fsync method */
> + return 0;
> + }
> +}
> +

Thanks for your review.
I agree with your comments. I fixed them.

Regards
--
Masahiro Ikeda
NTT DATA CORPORATION