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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: add checkpoint stats of snapshot and mapping files of pg_logical dir
Date: 2022-03-16 09:32:41
Message-ID: CALj2ACVdN=wbp_nSzv=9k6k4sjHPO3hQ+h-y3+69MaZHgYh3iw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 14, 2022 at 10:34 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> > 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.

Thanks.

> 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.

Yeah. I will remove them.

> 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.

Below are the ways that I can think of. Thoughts?

1)
if (restartpoint)
{
if (snap_files_rmvd > 0 && map_files_rmvd > 0)
existing "restartpoint complete" message + "snapshot files
removed cnt, time, cutoff lsn" + "mapping files removed cnt, time,
cutoff lsn"
else if (snap_files_rmvd > 0)
existing "restartpoint complete" message + "snapshot files
removed cnt, time, cutoff lsn"
else if (map_files_rmvd > 0)
existing "restartpoint complete" message + "mapping files removed
cnt, time, cutoff lsn"
else
existing "restartpoint complete" message
}
else
{
same as above but with "checkpoint complete" instead of "restart complete"
}

2) Use StringInfoData to prepare the message dynamically but this will
create message translation problems.

3) Emit the "snapshot files removed cnt, time, cutoff lsn" and
"mapping files removed cnt, time, cutoff lsn" at LOG level if
(log_checkpoints) in CheckPointSnapBuild and
CheckPointLogicalRewriteHeap respectively.

4) Have separate log messages in LogCheckpointEnd:
if (snap_files_rmvd > 0)
"snapshot files removed cnt, time, cutoff lsn"
if (map_files_rmvd > 0)
"mapping files removed cnt, time, cutoff lsn"

if (restartpoint)
existing "restartpoint complete" message
else
existing "checkpoint complete" message

Regards,
Bharath Rupireddy.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2022-03-16 10:12:06 Re: pgcrypto: Remove internal padding implementation
Previous Message Bharath Rupireddy 2022-03-16 09:32:26 Re: add checkpoint stats of snapshot and mapping files of pg_logical dir