Date: Wed, 15 Apr 2026 08:16:01 +0900 (JST)
Message-Id: <20260415.081601.1744698268968717621.ishii@postgresql.org>
To: koshino@sraoss.co.jp
Cc: pgpool-hackers@lists.postgresql.org
Subject: Re: Proposal: Add lifecheck started status to pcp_watchdog_info.
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <TY4PR01MB173744EDFBDCE68DDECAE746B9456A@TY4PR01MB17374.jpnprd01.prod.outlook.com>
References: 
 <TY4PR01MB173744EDFBDCE68DDECAE746B9456A@TY4PR01MB17374.jpnprd01.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/20260415.081601.1744698268968717621.ishii%40postgresql.org>
Precedence: bulk

Thank you for the patch!

> Hi,
> This patch adds a new field, "Lifecheck Started", to the pcp_watchdog_info output.
> Currently, users need to inspect logs to verify whether the lifecheck has started on each node.
> This change allows the status to be checked directly from a single command, making it easier to verify behavior and perform regression testing.
> For example:
> 
> $ pcp_watchdog_info -h localhost -p 9898 -U pgpool -v
> Password:
> Watchdog Cluster Information
> Total Nodes              : 3
> Remote Nodes             : 2
> Member Remote Nodes      : 2
> Alive Remote Nodes       : 2
> Nodes required for quorum: 2
> Quorum state             : QUORUM EXIST
> Local node escalation    : YES
> Leader Node Name         : server1:9999 Linux server1.localdomain
> Leader Host Name         : server1
> 
> Watchdog Node Information
> Node Name         : server1:9999 Linux server1.localdomain
> Host Name         : server1
> Delegate IP       : 192.168.56.150
> Pgpool port       : 9999
> Watchdog port     : 9000
> Node priority     : 1
> Status            : 4
> Status Name       : LEADER
> Membership Status : MEMBER
> Lifecheck Started : YES

This will make admin's lifer easier.
So I took a look at the patch.

> From f353f922222f70fad1d104f76adca1010578d40a Mon Sep 17 00:00:00 2001
> From: Taiki Koshino <koshino@sraoss.co.jp>
> Date: Thu, 26 Mar 2026 17:04:37 +0900
> Subject: [PATCH v9] Add Lifecheck Started status to pcp_watchdog_info output.
> 
> This commit enhances the pcp_watchdog_info command by adding a new field, Lifecheck Started,
> which indicates whether lifecheck has been started on each watchdog node (NO: not started, YES: started).
> 
> This allows users to check the lifecheck status directly from the command output without inspecting logs.
> 
> Add a lifecheck_started member to WatchdogNode. When the lifecheck process
> detects that lifecheck has started, it notifies the watchdog process, which
> sets lifecheck_started to true. When set to true, the status is propagated
> across the cluster.
> 
> Add a lifecheck_status field to pcp_watchdog_info so that the latest
> lifecheck_started status is displayed when the command is called.

Commit message itself looks good except some lines are too long. It
would be better to fold lines so that they are no longer 78 chars or
so.

diff --git a/src/include/pcp/pcp.h b/src/include/pcp/pcp.h
index e40b96bdc..15a4abb01 100644
--- a/src/include/pcp/pcp.h
+++ b/src/include/pcp/pcp.h
@@ -48,6 +48,8 @@ typedef struct PCPWDNodeInfo
 	int			wd_priority;	/* node priority in leader election */
 	int			pgpool_port;	/* pgpool port */
 	char		delegate_ip[WD_MAX_HOST_NAMELEN];	/* delegate IP */
+	bool		lifecheck_started; /* True means lifecheck is started,
+									* false means lifecheck is not started */

The comment for lifecheck_started is a little bit redundant. What
about something like /* True if lifecheck has started */"?

Rest of the patch looks good to me.

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