Hi Michael, Brandon and Ben,
Thank you for looking into this!
On Thu, 4 Dec 2025 at 10:42, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Dec 03, 2025 at 11:01:31PM -0800, Brandon Tat wrote:
> > Regarding the function regression_log_helper(), this function reads
> > all the lines in the logs at line 219 of
> > src/test/recovery/t/027_stream_regress.pl. It seems wasteful to read
> > the file again twice in read_file_ends(). Alternatively, we could
> > read the file once within regression_log_helper() and index lines to
> > emit the lines that we want.
>
> It seems to me that you are looking at v4-0001 and v4-0002 posted at
> [1], which I did not author. So your suggestion would be to call
> read_file_ends() once with the file opened once, with three modes
> instead of the two presented in the patch: fetch the head, the tail,
> or both at the same time. Yes, that would be more efficient.
>
> While looking at the patch with fresher eyes (didn't look at this
> thread for a couple of months, sorry), it looks like there is no point
> in having regression_log_helper() at all. We could just return the
> tail and the head in a single call of read_file_ends() with two output
> variables. Then we could embed in read_file_ends() the knowledge that
> if we are dealing with a file that has less lines than twice
> PG_TEST_FILE_READ_LINES, we can just print the whole file, returning
> only the full contents as in a variable for what would have been the
> head content, leaving the tail content empty.
>
> If somebody would like to send a patch among these lines, feel free..
I applied these feedbacks in v5. I wanted to cover all possible cases
so I think 0002 might be a bit more complicated than it needs to be.
What do you think about the current implementation?
--
Regards,
Nazir Bilal Yavuz
Microsoft
