RE: Introduce XID age and inactive timeout based replication slot invalidation - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: Introduce XID age and inactive timeout based replication slot invalidation
Date
Msg-id OSCPR01MB14966A918EBB0674E5423EDE0F5F42@OSCPR01MB14966.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: Introduce XID age and inactive timeout based replication slot invalidation  (vignesh C <vignesh21@gmail.com>)
List pgsql-hackers
Dear Nisha,

Thanks for updating the patch! Here are my comments.

01.
```
+# Test for replication slots invalidation
```

Since the file tests only timeout invalidations, the comment seems too general.

02.
```
+       # Check that an invalidated slot cannot be acquired
+       my ($result, $stdout, $stderr);
+       ($result, $stdout, $stderr) = $node->psql(
+               'postgres', qq[
+                       SELECT pg_replication_slot_advance('$slot', '0/1');
+       ]);
+       ok( $stderr =~ /can no longer access replication slot "$slot"/,
+               "detected error upon trying to acquire invalidated slot $slot on node $node_name"
+         )
+         or die
+         "could not detect error upon trying to acquire invalidated slot $slot on node $node_name";
```

This part can be removal because this is not directly related with timeout invalidation.
If needed this can be outside the function and we can confirm only once.

03.
```
+# Initialize primary
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 'logical');
```

I think this node is not "primary" because there are no standby nodes. We can use new('node').
Also some comments which used "primary" can be removed.

04.
```
+$node->psql('postgres',
+       q{SELECT pg_create_logical_replication_slot('logical_slot', 'test_decoding');}
+);
```

Please use safe_psql() instead of psql().

05.
```
my $logstart = -s $node->logfile;
```

According to other tests, the variable name can be $log_offset.

Best regards,
Hayato Kuroda
FUJITSU LIMITED


pgsql-hackers by date:

Previous
From: Shlok Kyal
Date:
Subject: Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Next
From: vignesh C
Date:
Subject: Re: Restrict publishing of partitioned table with a foreign table as partition