Thaks for looking this, Robert and Tom.
At Fri, 24 Sep 2021 16:22:28 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Fri, Sep 24, 2021 at 3:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I think the basic idea is about right, but I'm not happy with the
> >> three-way delayChkpt business; that seems too cute by three-quarters.
>
> > Nobody, but the version of the patch that I was looking at uses a
> > separate bit for each one:
>
> > +/* symbols for PGPROC.delayChkpt */
> > +#define DELAY_CHKPT_START (1<<0)
> > +#define DELAY_CHKPT_COMPLETE (1<<1)
>
> Hm, that's not in the patch version that the CF app claims to be
> latest [1]. It does this:
>
> +/* type for PGPROC.delayChkpt */
> +typedef enum DelayChkptType
> +{
> + DELAY_CHKPT_NONE = 0,
> + DELAY_CHKPT_START,
> + DELAY_CHKPT_COMPLETE
> +} DelayChkptType;
>
> which seems like a distinct disimprovement over what you're quoting.
Yeah, that is because the latest patch is not attached as *.patch/diff
but *.txt. I didn't name it as *.patch in order to avoid noise patch
in that thread although it was too late. On the contrary that seems to
have lead in another trouble..
Tom's concern is right. Actually both the two events can happen
simultaneously but the latest *.patch.txt treats that case as Robert
said.
One advantage of having the two flags as one bitmap integer is it
slightly simplifies the logic in GetVirtualXIDsDelayingChkpt and
HaveVirtualXIDsDelayingChkpt. On the other hand it very slightly
complexifies how to set/reset the flags.
GetVirtualXIDsDelayingChkpt:
+ if ((proc->delayChkpt & type) != 0)
vs
+ if (delayStart)
+ delayflag = proc->delayChkptStart;
+ else
+ delayflag = proc->delayChkptEnd;
+ if (delayflag != 0)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center