Re: pg_recvlogical: Prevent flushed data from being re-sent after restarting replication

Fujii Masao <masao.fujii@gmail.com>

From: Fujii Masao <masao.fujii@gmail.com>
To: Mircea Cadariu <cadariu.mircea@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-12-26T10:28:26Z
Lists: pgsql-hackers

Attachments

On Wed, Dec 3, 2025 at 5:59 PM Mircea Cadariu <cadariu.mircea@gmail.com> wrote:
>
> Hi,
>
> On 28/11/2025 02:15, Fujii Masao wrote:
>
> I've made a few minor adjustments to the test patch.
> The updated version is attached.
>
> Thanks for the updated patch! Nice improvements.
>
> Two futher proposals for the current version of the test.
>
> Shall we use slurp_file then everywhere we need file reads? (instead of pg_read_file)

Maybe it's better to use slurp_file(). We already have wait_for_log() to
wait for a message in the cluster's log file, but there's no helper function
to wait for specific content to appear in an arbitrary file.

To support waiting for output in pg_recvlogical's output file,
I added a new helper that uses slurp_file() (see the attached 0002 patch).
I also updated the 0003 patch (the pg_recvlogical reconnection test) to
use this helper instead of pg_read_file(). Thoughts?


> The following can be seen as nits for your consideration.
>
> We can consider making the string provided in the "or die" to be consistent with the comment. We can pick one of the options below and specify the same for each.
>
> * receive and write the first INSERT / receive first INSERT
>
> * establish a new connection / to reconnect
>
> * receive and write  / receive

As a result of removing poll_query_until() with pg_read_file(), the patch
now contains only one "or die" code. In that case, I chose "to reconnect"
rather than "establish a new connection".


> If we are mentioning multiple INSERTs instead of just one, might read better if we add the "s" at the end. This might be just my preference though, I leave it up to you.

Agreed,  I've applied that change.

Regards,

-- 
Fujii Masao