Re: File truncation within PostgresNode::issues_sql_like() wrong on Windows

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: File truncation within PostgresNode::issues_sql_like() wrong on Windows
Date: 2021-04-15 01:26:19
Message-ID: d5162219-15a8-a86c-61e4-ba06e0cff1ee@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 4/14/21 8:10 PM, Michael Paquier wrote:
> On Wed, Apr 14, 2021 at 05:10:41PM -0400, Andrew Dunstan wrote:
>> That seems rather heavy-handed. The buildfarm's approach is a bit
>> different. Essentially it seeks to the previous position of the log file
>> before reading contents. Here is its equivalent of slurp_file:
>>
>> use Fcntl qw(:seek);
>> sub file_lines
>> {
>>     my $filename = shift;
>>     my $filepos  = shift;
>>     my $handle;
>>     open($handle, '<', $filename) || croak "opening $filename: $!";
>>     seek($handle, $filepos, SEEK_SET) if $filepos;
>>     my @lines = <$handle>;
>>     close $handle;
>>     return @lines;
>> }
> That's a bit surprising to see that you can safely open a file handle
> with perl like that without using Win32API::File, and I would have
> assumed that this would have conflicted with the backend redirecting
> its output to stderr the same way as a truncation on Windows.
>
>> A client wanting what's done in PostgresNode would do something like:
>>
>> my $logpos  = -s $logfile;
>> do_some_stuff();
>> my @lines = file_lines($logfile, $logpos);
>>
>> This has the benefit of working the same on all platforms, and no
>> truncation, rotation, or restart is required.
> Jacob has suggested something like that a couple of days ago, but all
> this code was not centralized yet in a single place.
>
> For this code, the cleanest approach would be to extend slurp_file()
> with an extra argument to seek the file before fetching its contents
> based on a location given by the caller? Looking at the docs of
> Win32API::File, we'd need to use SetFilePointer() instead of seek().

Well, let me try it on fairywren tomorrow. Since we manage this on the
buildfarm without any use at all of Win32API::File it might not be
necessary in TAP code either, particularly if we're not truncating the file.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-04-15 01:28:43 Re: Proposal: Save user's original authenticated identity for logging
Previous Message Michael Paquier 2021-04-15 01:21:32 Re: psql - add SHOW_ALL_RESULTS option