From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, 'vignesh C' <vignesh21(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: Introduce XID age and inactive timeout based replication slot invalidation |
Date: | 2025-02-04 12:16:28 |
Message-ID: | OSCPR01MB14966A918EBB0674E5423EDE0F5F42@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2025-02-04 12:46:24 | Re: Restrict publishing of partitioned table with a foreign table as partition |
Previous Message | Shlok Kyal | 2025-02-04 12:09:05 | Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility. |