Subject: Re: Allow escape in application_name (was: [postgres_fdw] add local
 pid to fallback_application_name)
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>,
 'Masahiro Ikeda' <ikedamsh@oss.nttdata.com>,
 "'pgsql-hackers@lists.postgresql.org'" <pgsql-hackers@lists.postgresql.org>
References: 
 <TYCPR01MB5870D1E8B949DAF6D3B84E02F5F29@TYCPR01MB5870.jpnprd01.prod.outlook.com>
 <TYAPR01MB5866EF9624D2DFA8A74E5E94F5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <8f985ba0-2e79-ab67-4fa4-d35024188047@oss.nttdata.com>
 <TYAPR01MB5866B697888D5568E6D1546BF5CD9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <0d5df322-c9ad-308e-982b-a3d3275bcdc0@oss.nttdata.com>
 <TYAPR01MB5866A3CAE041F7B05E2EE3A9F5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <2dde164d-bc49-b3ca-0cc1-1c5fe40f0886@oss.nttdata.com>
 <TYAPR01MB58669D67A505AD8995BBD2D6F5CF9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <21aa7786-fce9-5dbf-6b75-519015319724@oss.nttdata.com>
 <TYAPR01MB58666157D4FF274822A76B3BF5D29@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <42ea526c-aa4f-bfe2-5547-6fcf8ac6469c@oss.nttdata.com>
 <TYAPR01MB5866772D4732E9CD7C040D62F5D39@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <6da5713e-9adf-5301-0d9d-eb6cce16ea40@oss.nttdata.com>
 <3220909.1631054766@sss.pgh.pa.us>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <848ff477-effd-42b9-8b25-3f7cfe289398@oss.nttdata.com>
Date: Wed, 8 Sep 2021 09:34:14 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <3220909.1631054766@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/848ff477-effd-42b9-8b25-3f7cfe289398%40oss.nttdata.com>
Precedence: bulk


On 2021/09/08 7:46, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> Pushed 0001 patch. Thanks!
> 
> The buildfarm shows that this test case isn't terribly reliable.

Yes... Thanks for reporting this!


> TBH, I think you should just remove the test case altogether.
> It does not look useful enough to justify a permanent expenditure of
> testing cycles, never mind the amount of effort that would be needed to
> stabilize it.

Ok, I will remove the tests.

Kuroda-san is proposing the additional feature which allows
postgres_fdw.application_name to include escape characters.
If we commit the feature, it's worth adding the similar tests
checking that the escape characters there are replaced with
expected values. So it's better to investigate what made
the tests fail, not to cause the same tests failure later.

The tests use postgres_fdw_disconnect_all() to close the existing
remote connections before establishing new remote connection
with new application_name setting. Then they check that
the expected application_name appears in pg_stat_activity.
The cause of the issue seems to be that it can take a bit time for
the existing remote connection (i.e., its corresponding backend)
to exit after postgres_fdw_disconect_all() is called.
So application_name of the existing remote connection also could
appear in pg_stat_activity when it's checked next time.

If this analysis is right, the tests that the upcoming patch adds
should handle the case where the existing remote connection
can appear in pg_stat_activity after postgres_fdw_disconect_all().

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION