Re: A few new options for CHECKPOINT - Mailing list pgsql-hackers

From Bossart, Nathan
Subject Re: A few new options for CHECKPOINT
Date
Msg-id 4B516D8A-0891-44A3-BBAD-39A46185C15D@amazon.com
Whole thread Raw
In response to Re: A few new options for CHECKPOINT  (Michael Paquier <michael@paquier.xyz>)
Responses Re: A few new options for CHECKPOINT
List pgsql-hackers
Thanks for reviewing.

On 12/4/20, 5:44 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> On Sat, Dec 05, 2020 at 12:11:13AM +0000, Bossart, Nathan wrote:
>> On 12/4/20, 3:33 PM, "Alvaro Herrera" <alvherre@alvh.no-ip.org> wrote:
>>> Instead of adding checkpt_option_list, how about utility_option_list?
>>> It seems intended for reuse.
>
> +1.  It is intended for reuse.
>
>> Ah, good call.  That simplifies the grammar changes quite a bit.
>
> +CHECKPOINT;
> +CHECKPOINT (SPREAD);
> +CHECKPOINT (SPREAD FALSE);
> +CHECKPOINT (SPREAD ON);
> +CHECKPOINT (SPREAD 0);
> +CHECKPOINT (SPREAD 2);
> +ERROR:  spread requires a Boolean value
> +CHECKPOINT (NONEXISTENT);
> +ERROR:  unrecognized CHECKPOINT option "nonexistent"
> +LINE 1: CHECKPOINT (NONEXISTENT);
> Testing for negative cases like those two last ones is fine by me, but
> I don't like much the idea of running 5 checkpoints as part of the
> main regression test suite (think installcheck with a large shared
> buffer pool for example).
>
> --- a/src/include/postmaster/bgwriter.h
> +++ b/src/include/postmaster/bgwriter.h
> @@ -15,6 +15,8 @@
>  #ifndef _BGWRITER_H
>  #define _BGWRITER_H
>
> +#include "nodes/parsenodes.h"
> +#include "parser/parse_node.h"
> I don't think you need to include parsenodes.h here.
>
> +void
> +ExecCheckPointStmt(ParseState *pstate, CheckPointStmt *stmt)
> +{
> Nit: perhaps this could just be ExecCheckPoint()?  See the existing
> ExecVacuum().
>
> +   flags = CHECKPOINT_WAIT |
> +           (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE) |
> +           (spread ? 0 : CHECKPOINT_IMMEDIATE);
> The handling done for CHECKPOINT_FORCE and CHECKPOINT_WAIT deserve
> a comment.

This all seems reasonable to me.  I've attached a new version of the
patch.

Nathan


Attachment

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Proposed patch for key managment
Next
From: Amit Kapila
Date:
Subject: Re: POC: Cleaning up orphaned files using undo logs