Horton

Re: pg_resetwal tests, logging, and docs update

Peter Eisentraut <peter@eisentraut.org>

View thread View on postgr.es
From: Peter Eisentraut <peter@eisentraut.org>
To: Aleksander Alekseev <aleksander@timescale.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-10-31T11:50:23Z
Lists: pgsql-hackers

Attachments

On 30.10.23 12:55, Aleksander Alekseev wrote:
> The patch LGTM. However, postgresql:pg_resetwal test suite doesn't
> pass on Windows according to cfbot. Seems to be a matter of picking a
> more generic regular expression:
> 
> ```
> at C:/cirrus/src/bin/pg_resetwal/t/001_basic.pl line 54.
>                  'pg_resetwal: error: could not change directory to
> "foo": No such file or directory
>    doesn't match '(?^:error: could not read permissions of directory)'
> ```
> 
> Should we simply use something like:
> 
> ```
> qr/error: could not (read|change).* directory/
> ```

Hmm.  I think maybe we should fix the behavior of 
GetDataDirectoryCreatePerm() to be more consistent between Windows and 
non-Windows.  This is usually the first function a program uses on the 
proposed data directory, so it's also responsible for reporting if the 
data directory does not exist.  But then on Windows, because the 
function does nothing, those error scenarios end up on quite different 
code paths, and I'm not sure if those are really checked that carefully. 
  I think we can make this more robust if we have 
GetDataDirectoryCreatePerm() still run the stat() call on the proposed 
data directory and report the error.  See attached patch.

Commits

  1. doc: pg_resetwal: Add comments how the multipliers are derived

  2. pg_resetwal: Add more tests and test coverage

  3. More consistent behavior of GetDataDirectoryCreatePerm on Windows

  4. doc: Improve documentation about pg_resetwal -f option

  5. pg_resetwal: Use frontend logging API

  6. pg_resetwal: Regroup --help output

  7. pg_resetwal: Improve error with wrong/missing data directory

  8. pg_resetwal: Update an obsolete comment