On Wed, Dec 3, 2025 at 5:59 PM Mircea Cadariu <[email protected]> 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

Attachment: v3-0003-Add-test-for-pg_recvlogical-reconnection-behavior.patch
Description: Binary data

Attachment: v3-0001-pg_recvlogical-Prevent-flushed-data-from-being-re.patch
Description: Binary data

Attachment: v3-0002-Add-a-new-helper-function-wait_for_file-to-Utils..patch
Description: Binary data

Reply via email to