Date: Fri, 04 Jul 2025 17:10:15 +0900 (JST)
Message-Id: <20250704.171015.770396187969679710.ishii@postgresql.org>
To: pengbo@sraoss.co.jp
Cc: koshino@sraoss.co.jp, pgpool-hackers@lists.postgresql.org
Subject: Re: Patch for fixing doc about some parameters.
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <TYWP286MB2633CAED55BC22ED763B4921F242A@TYWP286MB2633.JPNP286.PROD.OUTLOOK.COM>
References: <20250702.202713.220401050993902307.ishii@postgresql.org>
	<TYYP286MB29311B5F79DA03F39C1B551B9443A@TYYP286MB2931.JPNP286.PROD.OUTLOOK.COM>
	<TYWP286MB2633CAED55BC22ED763B4921F242A@TYWP286MB2633.JPNP286.PROD.OUTLOOK.COM>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20250704.171015.770396187969679710.ishii%40postgresql.org>
Precedence: bulk

> Koshino-san,
> 
> Thank you for your patch.
> I have reviewed your patch and have a few comments.
> 
> --------------------------------------
> v2-0001-Doc-fix-documentation-for-enum-parameters-reporte.patch.
> --------------------------------------
> 
> For the following parameters:
> 
>     log_standby_delay
>     wd_lifecheck_method
>     memqcache_method
>     disable_load_balance_on_write
> 
> In the documentation, their types have been correctly changed to enum, which is good.
> However, in both the configuration file examples and the parameter description sections,
> these values are still written as strings (enclosed in single quotes).
> 
> For example:
> 
> (config file)
> #wd_lifecheck_method = 'heartbeat'
> 
> (Document)
> wd_lifecheck_method (enum)
>     Specifies the method of life check. This can be either of 'heartbeat' (default), 'query' or 'external'. 
> 
> This is no big issue since Pgpool-II accepts string values (e.g. 'foo') even for enum parameters.
> However, it's a bit confusing, so I think it should be fixed.

+1.

> For example:
> 
> #wd_lifecheck_method = heartbeat  
>                                                  # Method of watchdog lifecheck (heartbeat or query or external)
> 
> wd_lifecheck_method (enum)
>     Specifies the method of life check. This can be either of <literal>heartbeat</literal> (default), <literal>query</literal> or <literal>external</literal>. 
> 
> ------------------------------------------
> v2-0001-Doc-fix-documentation-for-parameters-that-are-not.patch
> ------------------------------------------
> 
> I think we should also fix the configuration file to add the description such as "(change requires restart)".

Agreed.

> For example:
> 
> #authentication_timeout = 1min
>                                    # Delay in seconds to complete client authentication
>                                    # 0 means no timeout.
>                                    # (change requires restart)

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp