public inbox for [email protected]  
help / color / mirror / Atom feed
From: Koshino Taiki <[email protected]>
To: Tatsuo Ishii <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Proposal: Add lifecheck started status to pcp_watchdog_info.
Date: Wed, 15 Apr 2026 02:45:18 +0000
Message-ID: <OS9PR01MB17364604EE50807847ABB328394222@OS9PR01MB17364.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <TY4PR01MB173744EDFBDCE68DDECAE746B9456A@TY4PR01MB17374.jpnprd01.prod.outlook.com>
	<[email protected]>

Hi,
Thank you for your review.
I have made the changes based on your review.
The final commit has been pushed to the master branch:
https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commitdiff;h=366e602bca9ecc70f91fb52afcad74facbce...
Best regards,

Taiki Koshino<[email protected]>
SRA OSS K.K.
TEL: 03-5979-2701 FAX: 03-5979-2702
URL: https://www.sraoss.co.jp/



________________________________
差出人: Tatsuo Ishii <[email protected]>
送信: 2026 年 4 月 15 日 (水曜日) 8:16
宛先: Koshino Taiki <[email protected]>
Cc: [email protected] <[email protected]>
件名: Re: Proposal: Add lifecheck started status to pcp_watchdog_info.

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 <[email protected]>
> 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


reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Proposal: Add lifecheck started status to pcp_watchdog_info.
  In-Reply-To: <OS9PR01MB17364604EE50807847ABB328394222@OS9PR01MB17364.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox