public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Drouvot, Bertrand <[email protected]>
Cc: Nitin Jadhav <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Date: Tue, 15 Nov 2022 12:18:11 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAMm1aWbuJLEnAjT4_1s8eArmZq=H7oi0Jv=J2GnRpBhkbC_PVw@mail.gmail.com>
<CAMm1aWYWmqys2O7UekQ0=ZSWSpG9h2qoEW7w46yiWM0eO1Lu7g@mail.gmail.com>
<CALj2ACXsmvEDsHtADoQeDE-NVyBh4A-D80NmNanTUc37g=FQ-Q@mail.gmail.com>
<CAMm1aWabDx3V9Hw5L2eB+6EOtr60QWDybZShQsYLCCMNF-es_A@mail.gmail.com>
<CAMm1aWaFmbc8QCbcPkG7ROV7gjshamS=sJRJd=oghLJM9uOnpQ@mail.gmail.com>
<[email protected]>
<CAMm1aWa_3N1Z2-_YwdNxfogs_rxzpu90VZeKjqwjerCVq9rQAw@mail.gmail.com>
<[email protected]>
<CAMm1aWbfFwT+Cf1eSrXtF2R8ffxXTbH=un8unQ8WbcpcbymnYw@mail.gmail.com>
<[email protected]>
Hi,
On 2022-11-04 09:25:52 +0100, Drouvot, Bertrand wrote:
>
> @@ -7023,29 +7048,63 @@ static void
> CheckPointGuts(XLogRecPtr checkPointRedo, int flags)
> {
> CheckPointRelationMap();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_REPLI_SLOTS);
> CheckPointReplicationSlots();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_SNAPSHOTS);
> CheckPointSnapBuild();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_LOGICAL_REWRITE_MAPPINGS);
> CheckPointLogicalRewriteHeap();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_REPLI_ORIGIN);
> CheckPointReplicationOrigin();
>
> /* Write out all dirty data in SLRUs and the main buffer pool */
> TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags);
> CheckpointStats.ckpt_write_t = GetCurrentTimestamp();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_CLOG_PAGES);
> CheckPointCLOG();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_COMMITTS_PAGES);
> CheckPointCommitTs();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_SUBTRANS_PAGES);
> CheckPointSUBTRANS();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_MULTIXACT_PAGES);
> CheckPointMultiXact();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_PREDICATE_LOCK_PAGES);
> CheckPointPredicate();
> +
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_BUFFERS);
> CheckPointBuffers(flags);
>
> /* Perform all queued up fsyncs */
> TRACE_POSTGRESQL_BUFFER_CHECKPOINT_SYNC_START();
> CheckpointStats.ckpt_sync_t = GetCurrentTimestamp();
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_SYNC_FILES);
> ProcessSyncRequests();
> CheckpointStats.ckpt_sync_end_t = GetCurrentTimestamp();
> TRACE_POSTGRESQL_BUFFER_CHECKPOINT_DONE();
>
> /* We deliberately delay 2PC checkpointing as long as possible */
> + pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
> + PROGRESS_CHECKPOINT_PHASE_TWO_PHASE);
> CheckPointTwoPhase(checkPointRedo);
> }
This is quite the code bloat. Can we make this less duplicative?
> +CREATE VIEW pg_stat_progress_checkpoint AS
> + SELECT
> + S.pid AS pid,
> + CASE S.param1 WHEN 1 THEN 'checkpoint'
> + WHEN 2 THEN 'restartpoint'
> + END AS type,
> + ( CASE WHEN (S.param2 & 4) > 0 THEN 'immediate ' ELSE '' END ||
> + CASE WHEN (S.param2 & 8) > 0 THEN 'force ' ELSE '' END ||
> + CASE WHEN (S.param2 & 16) > 0 THEN 'flush-all ' ELSE '' END ||
> + CASE WHEN (S.param2 & 32) > 0 THEN 'wait ' ELSE '' END ||
> + CASE WHEN (S.param2 & 128) > 0 THEN 'wal ' ELSE '' END ||
> + CASE WHEN (S.param2 & 256) > 0 THEN 'time ' ELSE '' END
> + ) AS flags,
> + ( '0/0'::pg_lsn +
> + ((CASE
> + WHEN S.param3 < 0 THEN pow(2::numeric, 64::numeric)::numeric
> + ELSE 0::numeric
> + END) +
> + S.param3::numeric)
> + ) AS start_lsn,
I don't think we should embed this much complexity in the view
defintions. It's hard to read, bloats the catalog, we can't fix them once
released. This stuff seems like it should be in a helper function.
I don't have any iea what that pow stuff is supposed to be doing.
> + to_timestamp(946684800 + (S.param4::float8 / 1000000)) AS start_time,
I don't think this is a reasonable path - embedding way too much low-level
details about the timestamp format in the view definition. Why do we need to
do this?
Greetings,
Andres Freund
view thread (85+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox