From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter_e@gmx.net>
Cc: pgsql-committers@postgresql.org
Subject: Re: pgsql: Add support for temporary replication slots
Date: Mon, 12 Dec 2016 19:47:53 +0100
Message-ID: <62935e6f-4f1b-c433-e0fa-7f936a38b3e5@2ndquadrant.com> (raw)
In-Reply-To: <27833.1481558144@sss.pgh.pa.us>
References: <E1cGRP8-0003EH-So@gemulon.postgresql.org>
<27274.1481557380@sss.pgh.pa.us>
<27833.1481558144@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-committers>
On 12/12/16 16:55, Tom Lane wrote:
> I wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> Add support for temporary replication slots
>
>> Some of the slower buildfarm members are failing test-decoding-check since
>> this went in. At least on prairiedog, it looks like a race condition in
>> the test:
>
> Having now looked a bit more closely, that's almost certainly what it is:
> the test is assuming that the old backend exits instantaneously, which
> it doesn't. You might want to borrow the wait-for-previous-session-to-die
> loop I put into src/test/modules/test_extensions/sql/test_extensions.sql
> recently. (Make sure you get the fixed version ;-))
>
Yes you are correct, we tried naively to first drop another slot in
hopes that it will give the other backend enough time to close, but
apparently that wasn't robust enough for slower machines.
Attached is the fixed test using the loop from test_extensions (and yes
I would have missed the pg_stat_clear_snapshot() call without the hint,
thanks :) ).
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committersAttachments:0001-Fix-the-test_decoding-slot-test-race-condition.patch (application/x-patch, 3.0K) download
From f551cfe440d5ce086824cdb3be7ab494f5cdef1d Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Mon, 12 Dec 2016 19:41:40 +0100
Subject: [PATCH] Fix the test_decoding slot test race condition
---
contrib/test_decoding/expected/slot.out | 19 ++++++++++++++-----
contrib/test_decoding/sql/slot.sql | 16 ++++++++++++----
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.outindex 5e6b70b..c9171ff 100644--- a/contrib/test_decoding/expected/slot.out+++ b/contrib/test_decoding/expected/slot.out@@ -22,17 +22,26 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_p', 'test
init
(1 row)
--- reconnect to clean temp slots-\c+-- here we want to start a new session and wait till old one is gone+select pg_backend_pid() as oldpid \gset+\c -+do 'declare c int = 0;+begin+ while (select count(*) from pg_stat_activity where pid = '+ :'oldpid'+ ') > 0 loop c := c + 1; perform pg_stat_clear_snapshot(); end loop;+ raise log ''slot test looped % times'', c;+end';+-- should fail because the temporary slot was dropped automatically+SELECT pg_drop_replication_slot('regression_slot_t');+ERROR: replication slot "regression_slot_t" does not exist+-- permanent slot has survived
SELECT pg_drop_replication_slot('regression_slot_p');
pg_drop_replication_slot
--------------------------
(1 row)
--- should fail because the temporary slot was dropped automatically-SELECT pg_drop_replication_slot('regression_slot_t');-ERROR: replication slot "regression_slot_t" does not exist
-- test switching between slots in a session
SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot1', 'test_decoding', true);
?column?
diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sqlindex 3b0aecd..5d6d97a 100644--- a/contrib/test_decoding/sql/slot.sql+++ b/contrib/test_decoding/sql/slot.sql@@ -4,14 +4,22 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_t', 'test
SELECT pg_drop_replication_slot('regression_slot_p');
SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_p', 'test_decoding', false);
--- reconnect to clean temp slots-\c--SELECT pg_drop_replication_slot('regression_slot_p');+-- here we want to start a new session and wait till old one is gone+select pg_backend_pid() as oldpid \gset+\c -+do 'declare c int = 0;+begin+ while (select count(*) from pg_stat_activity where pid = '+ :'oldpid'+ ') > 0 loop c := c + 1; perform pg_stat_clear_snapshot(); end loop;+ raise log ''slot test looped % times'', c;+end';
-- should fail because the temporary slot was dropped automatically
SELECT pg_drop_replication_slot('regression_slot_t');
+-- permanent slot has survived+SELECT pg_drop_replication_slot('regression_slot_p');
-- test switching between slots in a session
SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot1', 'test_decoding', true);
--
2.7.4
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: pgsql-committers@postgresql.org
Cc: petr.jelinek@2ndquadrant.com, tgl@sss.pgh.pa.us, peter_e@gmx.net
Subject: Re: pgsql: Add support for temporary replication slots
In-Reply-To: <62935e6f-4f1b-c433-e0fa-7f936a38b3e5@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox