[pgpool-hackers: 4217] Re: Proposal: Add trusted_server_command parameter

[Tatsuo Ishii]

> Hi,
> 
> On 2022/10/27 16:38, Takuma Hoshiai wrote:
>> Hi Ishii-san,
>> On 2022/10/27 16:20, Tatsuo Ishii wrote:
>>> Hoshiai-san,
>>>
>>>>> Hi all,
>>>>>
>>>>> I create a patch about trusted_servers for v4.4.
>>>>> Add new parameter 'trusted_server_command'.
>>>>>
>>>>> Currently, only 'ping' command is used by trusted_servers for checking
>>>>> up stream
>>>>> connection, and number of sending packets is hard coded. In some
>>>>> cases, user maybe
>>>>> want to use other command.
>>>>>
>>>>> The trusted_server_command allow users to any specify command.　
>>>>> Default is
>>>>> 'ping -q -c %h' which means same as before.　Pgpool-II replaces the
>>>>> special
>>>>> characters(%h) with the host name in trusted_servers.
>>>>
>>>> I have tested your patch and encountered an issue. Can you please
>>>> help?
>>>>
>>>> I ran watchdog_setup to create a 3-pgpool-node cluster. And added
>>>> following two lined to each pgpool.conf then ran ./startall.
>>>>
>>>> trusted_servers = 'localhost'
>>>> trusted_server_command = 'ping -q -c3 %h'
>>>>
>>>> However the cluster did not start. I found following in the
>>>> pgpool.log.
>>>>
>>>> ping: write error: Bad file descriptor
>>>> 2022-10-27 13:35:29.855: watchdog_utility pid 326297: FATAL: watchdog
>>>> failed to ping host"localhost"
>>>> 2022-10-27 13:35:29.855: watchdog_utility pid 326297: DETAIL:
>>>> system(ping -q -c3 localhost) failed. reason: Success
>>>> 2022-10-27 13:35:29.856: life_check pid 326157: WARNING:  watchdog
>>>> lifecheck, failed to connect to any trusted servers
>>>> 2022-10-27 13:35:29.856: life_check pid 326157: LOG:  informing the
>>>> node status change to watchdog
>>>> 2022-10-27 13:35:29.856: life_check pid 326157: DETAIL:  node id :0
>>>> status = "NODE DEAD" message:"trusted server is unreachable"
>>>>
>>>> If I ran "ping -q -c3 localhost" manually, it works fine.
>>>>
>>>> $ ping -q -c3 localhost
>>>> PING localhost (127.0.0.1) 56(84) bytes of data.
>>>>
>>>> --- localhost ping statistics ---
>>>> 3 packets transmitted, 3 received, 0% packet loss, time 2034ms
>>>> rtt min/avg/max/mdev = 0.082/0.087/0.095/0.005 ms
>>>
>>> I think wd_trusted_server_command() needs to assign appropreate fd
>>> (/dev/null) to stdin, stdout and stderr. Attached is the revised
>>> patch. Can you please check?
> 
> I think it's good!
> But pgpool log messeges are suppressed, I fix debug and error messages
> to your patch.
> In addition, I add a regression test for trusted_server.

Great!

Very minor points:

+		{"trusted_server_command", CFGCXT_RELOAD, WATCHDOG_CONFIG,
+			"Command to excute when communicate trusted server.",

communicate --> communicate with

+			CONFIG_VAR_TYPE_STRING, false, 0
+		},

+					ereport(WARNING,
+							(errmsg("watchdog failed to ping host\"%s\"", server->hostname)));

ping host --> ping host:

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