Re: add checkpoint stats of snapshot and mapping files of pg_logical dir

On Mon, Mar 14, 2022 at 05:03:59PM +0530, Bharath Rupireddy wrote:
> On Mon, Mar 14, 2022 at 1:33 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> On Mon, Mar 14, 2022 at 10:54:56AM +0530, Bharath Rupireddy wrote:
>> > Yes, this is a concern. Also, when there were no logical replication
>> > slots on a plain server or the server removed or cleaned up all the
>> > snapshot/mappings files, why would anyone want to have these messages
>> > with all 0s in the server log?
>>
>> The default settings don't enable that, so making things conditional
>> roughly as you are suggesting with two different LOG messages sounds
>> rather fine.
>
> Attaching v3 patch which emits logs only when necessary and doesn't
> clutter the existing "checkpoint/restartpoint completed" message, see
> some sample logs at [1]. Please review it further.

I'm okay with not adding these statistics when the number of files removed
and sync'd is zero.

IMO we shouldn't include the size of the files since it will prevent us
from removing lstat() calls. As noted upthread, I suspect the time spent
in these tasks is more closely related to the number of files anyway.

I'm -1 on splitting these new statistics to separate LOGs. In addition to
making it more difficult to discover statistics for a given checkpoint, I
think it actually adds even more bloat to the server log. If we are
concerned about the amount of information in these LOGs, perhaps we should
adjust the format to make it more human-readable.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com