On Mon, Jun 26, 2017 at 12:12 PM, Craig Ringer <cr...@2ndquadrant.com> wrote:
> On 26 June 2017 at 11:06, Michael Paquier <michael.paqu...@gmail.com> wrote:
>
>> As long as we are on it, there is this code block in the test:
>> my ($xmin, $catalog_xmin) = get_slot_xmins($node_master, $slotname_1);
>> is($xmin, '', 'non-cascaded slot xmin null with no hs_feedback');
>> is($catalog_xmin, '', 'non-cascaded slot xmin null with no hs_feedback');
>>
>> ($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
>> is($xmin, '', 'cascaded slot xmin null with no hs_feedback');
>> is($catalog_xmin, '', 'cascaded slot xmin null with no hs_feedback');
>>
>> This should be more verbose as the 2nd and 4th test should say
>> "catalog xmin" instead of xmin.
>
> Agree, should. Mind posting a revision?
Sure.
>> Also, wouldn't it be better to poll as well node_standby_1's
>> pg_replication_slot on slotname_2? It would really seem better to make
>> the nullness check conditional in get_slot_xmins instead. Sorry for
>> changing opinion here.
>
> I'm not sure I understand this.
The patch attached may explain that better. Your patch added 3 poll
phases. I think that a 4th is needed. At the same time I have found
cleaner to put the poll calls into a small wrapper.
--
Michael
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 266d27c8a2..d97db659cb 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -146,6 +146,17 @@ $node_standby_2->append_conf('postgresql.conf',
"wal_receiver_status_interval = 1");
$node_standby_2->restart;
+sub wait_slot_xmins_update
+{
+ my ($node, $slot_name, $check_expr) = @_;
+
+ $node->poll_query_until('postgres', qq[
+ SELECT $check_expr
+ FROM pg_replication_slots
+ WHERE slot_name = '$slot_name';
+]);
+}
+
sub get_slot_xmins
{
my ($node, $slotname) = @_;
@@ -156,12 +167,12 @@ sub get_slot_xmins
# There's no hot standby feedback and there are no logical slots on either peer
# so xmin and catalog_xmin should be null on both slots.
my ($xmin, $catalog_xmin) = get_slot_xmins($node_master, $slotname_1);
-is($xmin, '', 'non-cascaded slot xmin null with no hs_feedback');
-is($catalog_xmin, '', 'non-cascaded slot xmin null with no hs_feedback');
+is($xmin, '', 'xmin of non-cascaded slot null with no hs_feedback');
+is($catalog_xmin, '', 'catalog xmin of non-cascaded slot null with no hs_feedback');
($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
-is($xmin, '', 'cascaded slot xmin null with no hs_feedback');
-is($catalog_xmin, '', 'cascaded slot xmin null with no hs_feedback');
+is($xmin, '', 'xmin of cascaded slot null with no hs_feedback');
+is($catalog_xmin, '', 'catalog xmin of cascaded slot null with no hs_feedback');
# Replication still works?
$node_master->safe_psql('postgres', 'CREATE TABLE replayed(val integer);');
@@ -196,15 +207,17 @@ $node_standby_2->safe_psql('postgres',
'ALTER SYSTEM SET hot_standby_feedback = on;');
$node_standby_2->reload;
replay_check();
-sleep(2);
+wait_slot_xmins_update($node_master, $slotname_1,
+ "xmin IS NOT NULL AND catalog_xmin IS NULL");
($xmin, $catalog_xmin) = get_slot_xmins($node_master, $slotname_1);
-isnt($xmin, '', 'non-cascaded slot xmin non-null with hs feedback');
-is($catalog_xmin, '', 'non-cascaded slot xmin still null with hs_feedback');
+isnt($xmin, '', 'xmin of non-cascaded slot non-null with hs feedback');
+is($catalog_xmin, '',
+ 'catalog xmin of non-cascaded slot still null with hs_feedback');
($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
-isnt($xmin, '', 'cascaded slot xmin non-null with hs feedback');
-is($catalog_xmin, '', 'cascaded slot xmin still null with hs_feedback');
+isnt($xmin, '', 'xmin of cascaded slot non-null with hs feedback');
+is($catalog_xmin, '', 'catalog xmin cascaded slot still null with hs_feedback');
note "doing some work to advance xmin";
for my $i (10000 .. 11000)
@@ -216,15 +229,15 @@ $node_master->safe_psql('postgres', 'CHECKPOINT;');
my ($xmin2, $catalog_xmin2) = get_slot_xmins($node_master, $slotname_1);
note "new xmin $xmin2, old xmin $xmin";
-isnt($xmin2, $xmin, 'non-cascaded slot xmin with hs feedback has changed');
+isnt($xmin2, $xmin, 'xmin of non-cascaded slot with hs feedback has changed');
is($catalog_xmin2, '',
- 'non-cascaded slot xmin still null with hs_feedback unchanged');
+ 'catalog xmin of non-cascaded slot still null with hs_feedback unchanged');
($xmin2, $catalog_xmin2) = get_slot_xmins($node_standby_1, $slotname_2);
note "new xmin $xmin2, old xmin $xmin";
-isnt($xmin2, $xmin, 'cascaded slot xmin with hs feedback has changed');
+isnt($xmin2, $xmin, 'xmin of cascaded slot with hs feedback has changed');
is($catalog_xmin2, '',
- 'cascaded slot xmin still null with hs_feedback unchanged');
+ 'catalog xmin of cascaded slot still null with hs_feedback unchanged');
note "disabling hot_standby_feedback";
@@ -236,16 +249,21 @@ $node_standby_2->safe_psql('postgres',
'ALTER SYSTEM SET hot_standby_feedback = off;');
$node_standby_2->reload;
replay_check();
-sleep(2);
+wait_slot_xmins_update($node_master, $slotname_1,
+ "xmin IS NULL AND catalog_xmin IS NULL");
($xmin, $catalog_xmin) = get_slot_xmins($node_master, $slotname_1);
-is($xmin, '', 'non-cascaded slot xmin null with hs feedback reset');
+is($xmin, '', 'xmin of non-cascaded slot null with hs feedback reset');
is($catalog_xmin, '',
- 'non-cascaded slot xmin still null with hs_feedback reset');
+ 'catalog xmin of non-cascaded slot still null with hs_feedback reset');
+
+wait_slot_xmins_update($node_standby_1, $slotname_2,
+ "xmin IS NULL AND catalog_xmin IS NULL");
($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
-is($xmin, '', 'cascaded slot xmin null with hs feedback reset');
-is($catalog_xmin, '', 'cascaded slot xmin still null with hs_feedback reset');
+is($xmin, '', 'xmin of cascaded slot null with hs feedback reset');
+is($catalog_xmin, '',
+ 'catalog xmin of cascaded slot still null with hs_feedback reset');
note "re-enabling hot_standby_feedback and disabling while stopped";
$node_standby_2->safe_psql('postgres',
@@ -260,11 +278,12 @@ $node_standby_2->safe_psql('postgres',
$node_standby_2->stop;
($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
-isnt($xmin, '', 'cascaded slot xmin non-null with postgres shut down');
+isnt($xmin, '', 'xmin of cascaded slot non-null with postgres shut down');
# Xmin from a previous run should be cleared on startup.
$node_standby_2->start;
+wait_slot_xmins_update($node_standby_1, $slotname_2, "xmin IS NULL");
($xmin, $catalog_xmin) = get_slot_xmins($node_standby_1, $slotname_2);
is($xmin, '',
- 'cascaded slot xmin reset after startup with hs feedback reset');
+ 'xmin of cascaded slot reset after startup with hs feedback reset');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
