From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | rjuju123(at)gmail(dot)com |
Cc: | sergey(dot)dudoladov(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, tomas(dot)vondra(at)enterprisedb(dot)com, bharath(dot)rupireddyforpostgres(at)gmail(dot)com |
Subject: | Re: Is there a way (except from server logs) to know the kind of on-going/last checkpoint? |
Date: | 2022-01-28 02:10:36 |
Message-ID: | 20220128.111036.1993944051750632523.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
At Fri, 28 Jan 2022 10:41:28 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> Sorry, the last message lacks one citation.
>
> At Thu, 27 Jan 2022 19:09:29 +0800, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote in
> > On Thu, Jan 27, 2022 at 06:56:57PM +0800, Julien Rouhaud wrote:
> > >
> > > What it's showing is the "currently ongoing checkpoint or last completed
> > > checkpoint" kind.
> >
> > Ah after double checking I see it's storing the information *after* the
> > checkpoint completion, so it's indeed the last completed checkpoint. I'm not
> > sure how useful it can be, but ok.
>
> I don't see it useful (but don't oppose) to record checkpoint kind in
> control file. It is a kind of realtime noncritical info and in the
> first place retrievable from server log if needed. And I'm skeptical
> that it is needed such frequently. Checkpoint kind is useful to check
> max_wal_size's appropriateness if it is in a summarized form as in
> pg_stat_bgwriter. On the other hand showing the same in a stats view
> or the output of pg_control_checkpoint() is fine by me.
>
> > > Also, it's only showing the initial triggering conditions of checkpoints.
> > > For instance, if a timed checkpoint is started and then a backend executes a
> > > "CHECKPOINT;", it will upgrade the ongoing checkpoint with additional flags but
> > > AFAICS those new flags won't be saved to the control file.
> >
> > This one is still valid I think, it's only storing the initial flags and not
> > the possibly upgraded one in shmem.
>
> Agreed.
>
> + snprintf(ckpt_kind, 2 * MAXPGPATH, "%s%s%s%s%s%s%s%s%s",
> + (flags == 0) ? "unknown" : "",
> + (flags & CHECKPOINT_IS_SHUTDOWN) ? "shutdown " : "",
> + (flags & CHECKPOINT_END_OF_RECOVERY) ? "end-of-recovery " : "",
> + (flags & CHECKPOINT_IMMEDIATE) ? "immediate " : "",
> + (flags & CHECKPOINT_FORCE) ? "force " : "",
> + (flags & CHECKPOINT_WAIT) ? "wait " : "",
> + (flags & CHECKPOINT_CAUSE_XLOG) ? "wal " : "",
> + (flags & CHECKPOINT_CAUSE_TIME) ? "time " : "",
> + (flags & CHECKPOINT_FLUSH_ALL) ? "flush-all" : "");
>
>
> I don't like to add this complex-but-need-in-sync blob twice. If we
> need to do that twice, I want them consolidated in any shape.
>
> + Datum values[18];
> + bool nulls[18];
>
> You forgot to expand these arrays.
>
> This breaks checkpoint file format. Need to bump PG_CONTROL_VERSION,
> and pg_upgrade need to treat the change.
>
> Even if we add checkpoint kind to control file, it would look a bit
> strange that the "checkpoint kind" shows first among all
> checkpoint-related lines. And at least the "wait" in the line is
> really useless since it is not a property of a checkpoint. Instead, it
> doesn't show "requested" which is one of the checkpoint properties
> like "xlog" and "time". I'm not sure we need all of the properties to
> be shown but I don't have a clear criteria for each property of it
> ought to be shown or not.
>
> > pg_control last modified: Fri 28 Jan 2022 09:49:46 AM JST
> > Latest checkpoint kind: immediate force wait
> > Latest checkpoint location: 0/172B2C8
>
> I'd like to see the PID of the triggering process, but it is really
> not a information suitable in the control file...
>
>
> - proallargtypes => '{pg_lsn,pg_lsn,text,int4,int4,bool,text,oid,xid,xid,xid,oid,xid,xid,oid,xid,xid,timestamptz}',
> + proallargtypes => '{pg_lsn,pg_lsn,text,int4,int4,bool,text,oid,xid,xid,xid,oid,xid,xid,oid,xid,xid,timestamptz,text}',
>
> I think the additional column should be text[] instead of text, but
> not sure.
>
> And you need to edit the corresponding part of the doc.
I have an additional comment.
+ char ckpt_kind[2 * MAXPGPATH];
..
+ snprintf(ckpt_kind, 2 * MAXPGPATH, "%s%s%s%s%s%s%s%s%s",
+ (flags == 0) ? "unknown" : "",
+ (flags & CHECKPOINT_IS_SHUTDOWN) ? "shutdown " : "",
The value "2 * MAXPGPATH" is utterly nonsense about bouth "2" and
"MAXPGPATH", and the product of them is apparently too large.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Regina Obe | 2022-01-28 02:22:14 | substring odd behavior |
Previous Message | Julien Rouhaud | 2022-01-28 02:00:46 | Re: Is there a way (except from server logs) to know the kind of on-going/last checkpoint? |