Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbc77be7

Browse files
committed
Harden postgres_fdw tests against unexpected cache flushes.
postgres_fdw will close its remote session if an sinval cache resetoccurs, since it's possible that that means some FDW parameterschanged. We had two tests that were trying to ensure that thesession remains alive by setting debug_discard_caches = 0; butthat's not sufficient. Even though the tests seem stable enoughin the buildfarm, they flap a lot under CI.In the first test, which is checking the ability to recover froma lost connection, we can stabilize the results by just notcaring whether pg_terminate_backend() finds a victim backend.If a reset did happen, there won't be a session to terminateanymore, but the test can proceed anyway. (Arguably, we arethen not testing the unintentional-disconnect case, but as longas that scenario is exercised in most runs I think it's fine;testing the reset-driven case is of value too.)In the second test, which is trying to verify the application_namedisplayed in pg_stat_activity by a remote session, we had a racecondition in that the remote session might go away before we canfetch its pg_stat_activity entry. We can close that race and makethe test more certainly test what it intends to by arranging thingsso that the remote session itself fetches its pg_stat_activity entry(based on PID rather than a somewhat-circular assumption about theapplication name).Both tests now demonstrably pass under debug_discard_caches = 1,so we can remove that hack.Back-patch into relevant back branches.Discussion:https://postgr.es/m/20230226194340.u44bkfgyz64c67i6@awork3.anarazel.de
1 parent696fa47 commitbc77be7

File tree

2 files changed

+64
-81
lines changed

2 files changed

+64
-81
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 38 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -9762,11 +9762,6 @@ WARNING: there is no transaction in progress
97629762
-- Change application_name of remote connection to special one
97639763
-- so that we can easily terminate the connection later.
97649764
ALTER SERVER loopback OPTIONS (application_name 'fdw_retry_check');
9765-
-- If debug_discard_caches is active, it results in
9766-
-- dropping remote connections after every transaction, making it
9767-
-- impossible to test termination meaningfully. So turn that off
9768-
-- for this test.
9769-
SET debug_discard_caches = 0;
97709765
-- Make sure we have a remote connection.
97719766
SELECT 1 FROM ft1 LIMIT 1;
97729767
?column?
@@ -9775,13 +9770,12 @@ SELECT 1 FROM ft1 LIMIT 1;
97759770
(1 row)
97769771

97779772
-- Terminate the remote connection and wait for the termination to complete.
9778-
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
9773+
-- (If a cache flush happens, the remote connection might have already been
9774+
-- dropped; so code this step in a way that doesn't fail if no connection.)
9775+
DO $$ BEGIN
9776+
PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
97799777
WHERE application_name = 'fdw_retry_check';
9780-
pg_terminate_backend
9781-
----------------------
9782-
t
9783-
(1 row)
9784-
9778+
END $$;
97859779
-- This query should detect the broken connection when starting new remote
97869780
-- transaction, reestablish new connection, and then succeed.
97879781
BEGIN;
@@ -9794,21 +9788,17 @@ SELECT 1 FROM ft1 LIMIT 1;
97949788
-- If we detect the broken connection when starting a new remote
97959789
-- subtransaction, we should fail instead of establishing a new connection.
97969790
-- Terminate the remote connection and wait for the termination to complete.
9797-
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
9791+
DO $$ BEGIN
9792+
PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
97989793
WHERE application_name = 'fdw_retry_check';
9799-
pg_terminate_backend
9800-
----------------------
9801-
t
9802-
(1 row)
9803-
9794+
END $$;
98049795
SAVEPOINT s;
98059796
-- The text of the error might vary across platforms, so only show SQLSTATE.
98069797
\set VERBOSITY sqlstate
98079798
SELECT 1 FROM ft1 LIMIT 1; -- should fail
98089799
ERROR: 08006
98099800
\set VERBOSITY default
98109801
COMMIT;
9811-
RESET debug_discard_caches;
98129802
-- =============================================================================
98139803
-- test connection invalidation cases and postgres_fdw_get_connections function
98149804
-- =============================================================================
@@ -11423,74 +11413,66 @@ HINT: There are no valid options in this context.
1142311413
-- ===================================================================
1142411414
-- test postgres_fdw.application_name GUC
1142511415
-- ===================================================================
11426-
--- Turn debug_discard_caches off for this test to make sure that
11427-
--- the remote connection is alive when checking its application_name.
11428-
SET debug_discard_caches = 0;
11416+
-- To avoid race conditions in checking the remote session's application_name,
11417+
-- use this view to make the remote session itself read its application_name.
11418+
CREATE VIEW my_application_name AS
11419+
SELECT application_name FROM pg_stat_activity WHERE pid = pg_backend_pid();
11420+
CREATE FOREIGN TABLE remote_application_name (application_name text)
11421+
SERVER loopback2
11422+
OPTIONS (schema_name 'public', table_name 'my_application_name');
11423+
SELECT count(*) FROM remote_application_name;
11424+
count
11425+
-------
11426+
1
11427+
(1 row)
11428+
1142911429
-- Specify escape sequences in application_name option of a server
1143011430
-- object so as to test that they are replaced with status information
11431-
-- expectedly.
11431+
-- expectedly. Note that we are also relying on ALTER SERVER to force
11432+
-- the remote session to be restarted with its new application name.
1143211433
--
1143311434
-- Since pg_stat_activity.application_name may be truncated to less than
1143411435
-- NAMEDATALEN characters, note that substring() needs to be used
1143511436
-- at the condition of test query to make sure that the string consisting
1143611437
-- of database name and process ID is also less than that.
1143711438
ALTER SERVER loopback2 OPTIONS (application_name 'fdw_%d%p');
11438-
SELECT 1 FROM ft6 LIMIT 1;
11439-
?column?
11440-
----------
11441-
1
11442-
(1 row)
11443-
11444-
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
11439+
SELECT count(*) FROM remote_application_name
1144511440
WHERE application_name =
1144611441
substring('fdw_' || current_database() || pg_backend_pid() for
1144711442
current_setting('max_identifier_length')::int);
11448-
pg_terminate_backend
11449-
----------------------
11450-
t
11443+
count
11444+
-------
11445+
1
1145111446
(1 row)
1145211447

1145311448
-- postgres_fdw.application_name overrides application_name option
1145411449
-- of a server object if both settings are present.
11450+
ALTER SERVER loopback2 OPTIONS (SET application_name 'fdw_wrong');
1145511451
SET postgres_fdw.application_name TO 'fdw_%a%u%%';
11456-
SELECT 1 FROM ft6 LIMIT 1;
11457-
?column?
11458-
----------
11459-
1
11460-
(1 row)
11461-
11462-
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
11452+
SELECT count(*) FROM remote_application_name
1146311453
WHERE application_name =
1146411454
substring('fdw_' || current_setting('application_name') ||
1146511455
CURRENT_USER || '%' for current_setting('max_identifier_length')::int);
11466-
pg_terminate_backend
11467-
----------------------
11468-
t
11456+
count
11457+
-------
11458+
1
1146911459
(1 row)
1147011460

11461+
RESET postgres_fdw.application_name;
1147111462
-- Test %c (session ID) and %C (cluster name) escape sequences.
11472-
SET postgres_fdw.application_name TO 'fdw_%C%c';
11473-
SELECT 1 FROM ft6 LIMIT 1;
11474-
?column?
11475-
----------
11476-
1
11477-
(1 row)
11478-
11479-
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
11463+
ALTER SERVER loopback2 OPTIONS (SET application_name 'fdw_%C%c');
11464+
SELECT count(*) FROM remote_application_name
1148011465
WHERE application_name =
1148111466
substring('fdw_' || current_setting('cluster_name') ||
1148211467
to_hex(trunc(EXTRACT(EPOCH FROM (SELECT backend_start FROM
1148311468
pg_stat_get_activity(pg_backend_pid()))))::integer) || '.' ||
1148411469
to_hex(pg_backend_pid())
1148511470
for current_setting('max_identifier_length')::int);
11486-
pg_terminate_backend
11487-
----------------------
11488-
t
11471+
count
11472+
-------
11473+
1
1148911474
(1 row)
1149011475

11491-
--Clean up
11492-
RESET postgres_fdw.application_name;
11493-
RESET debug_discard_caches;
1149411476
-- ===================================================================
1149511477
-- test parallel commit
1149611478
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2964,18 +2964,16 @@ ROLLBACK;
29642964
-- so that we can easily terminate the connection later.
29652965
ALTER SERVER loopback OPTIONS (application_name'fdw_retry_check');
29662966

2967-
-- If debug_discard_caches is active, it results in
2968-
-- dropping remote connections after every transaction, making it
2969-
-- impossible to test termination meaningfully. So turn that off
2970-
-- for this test.
2971-
SET debug_discard_caches=0;
2972-
29732967
-- Make sure we have a remote connection.
29742968
SELECT1FROM ft1LIMIT1;
29752969

29762970
-- Terminate the remote connection and wait for the termination to complete.
2977-
SELECT pg_terminate_backend(pid,180000)FROM pg_stat_activity
2971+
-- (If a cache flush happens, the remote connection might have already been
2972+
-- dropped; so code this step in a way that doesn't fail if no connection.)
2973+
DO $$BEGIN
2974+
PERFORM pg_terminate_backend(pid,180000)FROM pg_stat_activity
29782975
WHERE application_name='fdw_retry_check';
2976+
END $$;
29792977

29802978
-- This query should detect the broken connection when starting new remote
29812979
-- transaction, reestablish new connection, and then succeed.
@@ -2985,17 +2983,17 @@ SELECT 1 FROM ft1 LIMIT 1;
29852983
-- If we detect the broken connection when starting a new remote
29862984
-- subtransaction, we should fail instead of establishing a new connection.
29872985
-- Terminate the remote connection and wait for the termination to complete.
2988-
SELECT pg_terminate_backend(pid,180000)FROM pg_stat_activity
2986+
DO $$BEGIN
2987+
PERFORM pg_terminate_backend(pid,180000)FROM pg_stat_activity
29892988
WHERE application_name='fdw_retry_check';
2989+
END $$;
29902990
SAVEPOINT s;
29912991
-- The text of the error might vary across platforms, so only show SQLSTATE.
29922992
\set VERBOSITY sqlstate
29932993
SELECT1FROM ft1LIMIT1;-- should fail
29942994
\set VERBOSITY default
29952995
COMMIT;
29962996

2997-
RESET debug_discard_caches;
2998-
29992997
-- =============================================================================
30002998
-- test connection invalidation cases and postgres_fdw_get_connections function
30012999
-- =============================================================================
@@ -3675,49 +3673,52 @@ ALTER FOREIGN DATA WRAPPER postgres_fdw OPTIONS (nonexistent 'fdw');
36753673
-- ===================================================================
36763674
-- test postgres_fdw.application_name GUC
36773675
-- ===================================================================
3678-
--- Turn debug_discard_caches off for this test to make sure that
3679-
--- the remote connection is alive when checking its application_name.
3680-
SET debug_discard_caches=0;
3676+
-- To avoid race conditions in checking the remote session's application_name,
3677+
-- use this view to make the remote session itself read its application_name.
3678+
CREATEVIEWmy_application_nameAS
3679+
SELECT application_nameFROM pg_stat_activityWHERE pid= pg_backend_pid();
3680+
3681+
CREATE FOREIGN TABLE remote_application_name (application_nametext)
3682+
SERVER loopback2
3683+
OPTIONS (schema_name'public', table_name'my_application_name');
3684+
3685+
SELECTcount(*)FROM remote_application_name;
36813686

36823687
-- Specify escape sequences in application_name option of a server
36833688
-- object so as to test that they are replaced with status information
3684-
-- expectedly.
3689+
-- expectedly. Note that we are also relying on ALTER SERVER to force
3690+
-- the remote session to be restarted with its new application name.
36853691
--
36863692
-- Since pg_stat_activity.application_name may be truncated to less than
36873693
-- NAMEDATALEN characters, note that substring() needs to be used
36883694
-- at the condition of test query to make sure that the string consisting
36893695
-- of database name and process ID is also less than that.
36903696
ALTER SERVER loopback2 OPTIONS (application_name'fdw_%d%p');
3691-
SELECT1FROM ft6LIMIT1;
3692-
SELECT pg_terminate_backend(pid,180000)FROM pg_stat_activity
3697+
SELECTcount(*)FROM remote_application_name
36933698
WHERE application_name=
36943699
substring('fdw_'|| current_database()|| pg_backend_pid() for
36953700
current_setting('max_identifier_length')::int);
36963701

36973702
-- postgres_fdw.application_name overrides application_name option
36983703
-- of a server object if both settings are present.
3704+
ALTER SERVER loopback2 OPTIONS (SET application_name'fdw_wrong');
36993705
SETpostgres_fdw.application_name TO'fdw_%a%u%%';
3700-
SELECT1FROM ft6LIMIT1;
3701-
SELECT pg_terminate_backend(pid,180000)FROM pg_stat_activity
3706+
SELECTcount(*)FROM remote_application_name
37023707
WHERE application_name=
37033708
substring('fdw_'|| current_setting('application_name')||
37043709
CURRENT_USER||'%' for current_setting('max_identifier_length')::int);
3710+
RESETpostgres_fdw.application_name;
37053711

37063712
-- Test %c (session ID) and %C (cluster name) escape sequences.
3707-
SETpostgres_fdw.application_name TO'fdw_%C%c';
3708-
SELECT1FROM ft6LIMIT1;
3709-
SELECT pg_terminate_backend(pid,180000)FROM pg_stat_activity
3713+
ALTER SERVER loopback2 OPTIONS (SET application_name'fdw_%C%c');
3714+
SELECTcount(*)FROM remote_application_name
37103715
WHERE application_name=
37113716
substring('fdw_'|| current_setting('cluster_name')||
37123717
to_hex(trunc(EXTRACT(EPOCHFROM (SELECT backend_startFROM
37133718
pg_stat_get_activity(pg_backend_pid()))))::integer)||'.'||
37143719
to_hex(pg_backend_pid())
37153720
for current_setting('max_identifier_length')::int);
37163721

3717-
--Clean up
3718-
RESETpostgres_fdw.application_name;
3719-
RESET debug_discard_caches;
3720-
37213722
-- ===================================================================
37223723
-- test parallel commit
37233724
-- ===================================================================

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp